Conversation
4a87153 to
bb55f8a
Compare
|
Force-pushed after rebasing onto latest |
steel-bucket
left a comment
There was a problem hiding this comment.
Test it in a setup which tests for the Console output being same as in C, test across 20-30 samples, make a simple test script which tests both for Console Output and subtitle output. I'll give you one you can salvage something from
https://gist.github.com/steel-bucket/11cfe6fa76318256d974f286b8e6cd30
src/lib_ccx/ccx_decoders_xds.c
Outdated
| // declare rust implementation of process_xds_bytes (function) | ||
| extern void ccxr_process_xds_bytes( |
src/lib_ccx/ccx_decoders_xds.c
Outdated
| #ifndef DISABLE_RUST | ||
| ccxr_process_xds_bytes(ctx, hi, lo); // use the rust implementation |
There was a problem hiding this comment.
remove DISABLE_RUST, we deprecated that
src/rust/src/xds/handlers.rs
Outdated
| use libc::free; | ||
| use libc::malloc; | ||
|
|
||
| use crate::xds::types::*; |
src/rust/src/xds/handlers.rs
Outdated
| pub enum XDSError { | ||
| Err, | ||
| } | ||
|
|
There was a problem hiding this comment.
was introduced to remove clippy warning :
warning: this returns a `Result<_, ()>`
--> src/xds/handlers.rs:100:1
|
100 | / pub unsafe fn write_xds_string(
101 | | sub: &mut cc_subtitle,
102 | | ctx: &mut CcxDecodersXdsContext,
103 | | p: String,
104 | | ts_start_of_xds: i64,
105 | | ) -> Result<(), ()> {
| |___________________^
|
= help: use a custom `Error` type instead
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#result_unit_err
= note: `#[warn(clippy::result_unit_err)]` on by default
warning: this returns a `Result<_, ()>`
--> src/xds/handlers.rs:149:1
|
149 | / pub unsafe fn xdsprint(
150 | | sub: &mut cc_subtitle,
151 | | ctx: &mut CcxDecodersXdsContext,
152 | | message: String,
153 | | ) -> Result<(), ()> {
| |___________________^
|
= help: use a custom `Error` type instead
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#result_unit_err
src/rust/src/xds/handlers.rs
Outdated
| log::debug!( | ||
| "XDS Start: {}.{} Is new: {} | Class: {} ({}), Used buffers: {}", |
There was a problem hiding this comment.
Import debug standalong instead of log::
src/rust/src/xds/handlers.rs
Outdated
| const US_TV_AGE_TEXT: [&str; 8] = [ | ||
| "None", | ||
| "TV-Y (All Children)", | ||
| "TV-Y7 (Older Children)", | ||
| "TV-G (General Audience)", | ||
| "TV-PG (Parental Guidance Suggested)", | ||
| "TV-14 (Parents Strongly Cautioned)", | ||
| "TV-MA (Mature Audience Only)", | ||
| "None", | ||
| ]; | ||
|
|
||
| /// MPA rating text | ||
| const MPA_RATING_TEXT: [&str; 8] = ["N/A", "G", "PG", "PG-13", "R", "NC-17", "X", "Not Rated"]; | ||
|
|
||
| /// Canadian English Language Rating | ||
| const CANADIAN_ENGLISH_RATING_TEXT: [&str; 8] = [ | ||
| "Exempt", | ||
| "Children", | ||
| "Children eight years and older", | ||
| "General programming suitable for all audiences", | ||
| "Parental Guidance", | ||
| "Viewers 14 years and older", | ||
| "Adult Programming", | ||
| "[undefined]", | ||
| ]; | ||
|
|
||
| /// Canadian French Language Rating | ||
| const CANADIAN_FRENCH_RATING_TEXT: [&str; 8] = [ | ||
| "Exempt?es", | ||
| "G?n?ral", | ||
| "G?n?ral - D?conseill? aux jeunes enfants", | ||
| "Cette ?mission peut ne pas convenir aux enfants de moins de 13 ans", | ||
| "Cette ?mission ne convient pas aux moins de 16 ans", | ||
| "Cette ?mission est r?serv?e aux adultes", | ||
| "[invalid]", | ||
| "[invalid]", | ||
| ]; |
src/rust/src/xds/handlers.rs
Outdated
| // XDS_TYPE_PIN_START_TIME = 1 | ||
| 1 => { |
| debug!( | ||
| msg_type = DebugMessageFlag::DECODER_XDS; | ||
| ":{:02}", el_sec |
There was a problem hiding this comment.
correct type of debug, use this in other impls
src/rust/src/xds/types.rs
Outdated
| pub static XDS_PROGRAM_TYPES: [&str; 96] = [ | ||
| "Education", | ||
| "Entertainment", | ||
| "Movie", |
There was a problem hiding this comment.
Move all constants, enums, and statics in a different file
| impl CType<xds_buffer> for XdsBuffer { | ||
| unsafe fn to_ctype(&self) -> xds_buffer { | ||
| xds_buffer { | ||
| in_use: self.in_use, | ||
| xds_class: self.xds_class.map(|c| c.to_c_int()).unwrap_or(-1), | ||
| xds_type: self.xds_type.map(|t| t.to_c_int()).unwrap_or(-1), | ||
| bytes: self.bytes, | ||
| used_bytes: self.used_bytes, | ||
| } | ||
| } | ||
| } | ||
|
|
trait impls
…tType enum, removed XDSerror enum
…s.rs, handlers.rs
…ith lib_ccxr::info
|
I've comitted the required changes from the review |
…arate file from types.rs, handlers.rs
|
Hi, Regression ID Sample Command Runtime (ms) Exit code Result 98 725a49f871... --autoprogram --out=ttxt --latin1 --ucla --xds 674 0 Pass |
src/rust/Cargo.toml
Outdated
| libc = "0.2.178" | ||
|
|
| pub extern "C" fn ccxr_set_ts_start_of_xds(value: i64) { | ||
| TS_START_OF_XDS.store(value, Ordering::SeqCst); | ||
| } |
There was a problem hiding this comment.
needed to sync TS_START_OF_XDS between C and Rust
|
Other than these and the format failure, PR looks good if there are no regressions. |
i;ve pushed the required fixes |
CCExtractor CI platform finished running the test files on linux. Below is a summary of the test results, when compared to test for commit d56a6be...:
Your PR breaks these cases:
NOTE: The following tests have been failing on the master branch as well as the PR:
Congratulations: Merging this PR would fix the following tests:
It seems that not all tests were passed completely. This is an indication that the output of some files is not as expected (but might be according to you). Check the result page for more info. |
CCExtractor CI platform finished running the test files on windows. Below is a summary of the test results, when compared to test for commit d56a6be...:
Your PR breaks these cases:
NOTE: The following tests have been failing on the master branch as well as the PR:
Congratulations: Merging this PR would fix the following tests:
It seems that not all tests were passed completely. This is an indication that the output of some files is not as expected (but might be according to you). Check the result page for more info. |
In raising this pull request, I confirm the following (please check boxes):
My familiarity with the project is as follows (check one):
(This PR Supersedes #1890; rebased on latest
masterwith fixes from review.)This pull request migrates
ccx_decoders_xds.candccx_decoders_xds.hto/src/rust/xdsrust module.mod.rs: exposes the Rust implementation to the C codebasetypes.rs: has types and structures for decoding extended datahandlers.rs: has handler functions for processing extended data packetsTested against stream : https://sampleplatform.ccextractor.org/sample/b22260d065ab537899baaf34e78a5184671f4bcb2df0414d05e6345adfd7812f