-
Notifications
You must be signed in to change notification settings - Fork 505
feat(rust): Add persistent DtvccRust context for CEA-708 decoder (Pha… #1782
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
src/rust/src/decoder/mod.rs
Outdated
| const CCX_DTVCC_MAX_COLUMNS: u8 = 32 * 2; | ||
|
|
||
| /// Maximum number of CEA-708 services | ||
| pub const CCX_DTVCC_MAX_SERVICES: usize = 63; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already have this: pub const DTVCC_MAX_SERVICES: usize = 63;
CCExtractor/ccextractor/src/rust/lib_ccxr/src/common/constants.rs
18e2e32 to
eb241d2
Compare
The cb_708 counter was being incremented twice for each CEA-708 data block: 1. In do_cb_dtvcc_rust() in Rust (src/rust/src/lib.rs) 2. In do_cb() in C (src/lib_ccx/ccx_decoders_common.c) Since FTS calculation uses cb_708 (fts = fts_now + fts_global + cb_708 * 1001 / 30), the double-increment caused timestamps to advance ~2x as fast as expected, resulting in incorrect milliseconds in start timestamps. This fix removes the increment from the Rust code since the C code already handles it in do_cb(). Fixes timestamp issues reported in PR CCExtractor#1782 tests where start times like 00:00:20,688 were incorrectly output as 00:00:20,737. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The cb_708 counter was being incremented twice for each CEA-708 data block: 1. In do_cb_dtvcc_rust() in Rust (src/rust/src/lib.rs) 2. In do_cb() in C (src/lib_ccx/ccx_decoders_common.c) Since FTS calculation uses cb_708 (fts = fts_now + fts_global + cb_708 * 1001 / 30), the double-increment caused timestamps to advance ~2x as fast as expected, resulting in incorrect milliseconds in start timestamps. This fix removes the increment from the Rust code since the C code already handles it in do_cb(). Fixes timestamp issues reported in PR CCExtractor#1782 tests where start times like 00:00:20,688 were incorrectly output as 00:00:20,737. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
abcfafe to
8ed6cc3
Compare
The cb_708 counter was being incremented twice for each CEA-708 data block: 1. In do_cb_dtvcc_rust() in Rust (src/rust/src/lib.rs) 2. In do_cb() in C (src/lib_ccx/ccx_decoders_common.c) Since FTS calculation uses cb_708 (fts = fts_now + fts_global + cb_708 * 1001 / 30), the double-increment caused timestamps to advance ~2x as fast as expected, resulting in incorrect milliseconds in start timestamps. This fix removes the increment from the Rust code since the C code already handles it in do_cb(). Fixes timestamp issues reported in PR CCExtractor#1782 tests where start times like 00:00:20,688 were incorrectly output as 00:00:20,737. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
8ed6cc3 to
4ff33fc
Compare
CCExtractor CI platform finished running the test files on linux. Below is a summary of the test results, when compared to test for commit 77e1dff...:
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:
All tests passing on the master branch were passed completely. Check the result page for more info. |
…se 1) This is Phase 1 of the fix for issue CCExtractor#1499. It adds the Rust-side infrastructure for a persistent CEA-708 decoder context without modifying any C code, ensuring backward compatibility. Problem: The current Rust CEA-708 decoder creates a new Dtvcc struct on every call to ccxr_process_cc_data(), causing all state to be reset. This breaks stateful caption processing. Solution: Add a new DtvccRust struct that: - Owns its decoder state (rather than borrowing from C) - Persists across processing calls - Is managed via FFI functions callable from C Changes: - Add DtvccRust struct in decoder/mod.rs with owned decoders - Add CCX_DTVCC_MAX_SERVICES constant (63) - Add FFI functions in lib.rs: - ccxr_dtvcc_init(): Create persistent context - ccxr_dtvcc_free(): Free context and all owned memory - ccxr_dtvcc_set_encoder(): Set encoder (not available at init) - ccxr_dtvcc_process_data(): Process CC data - ccxr_flush_active_decoders(): Flush all active decoders - ccxr_dtvcc_is_active(): Check if context is active - Add unit tests for DtvccRust - Use heap allocation for large structs to avoid stack overflow The existing Dtvcc struct and ccxr_process_cc_data() remain unchanged for backward compatibility. Phase 2-3 will add C header declarations and modify C code to use the new functions. Fixes: CCExtractor#1499 (partial) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove duplicate CCX_DTVCC_MAX_SERVICES constant from decoder/mod.rs - Import existing DTVCC_MAX_SERVICES from lib_ccxr::common - Fix clippy uninlined_format_args warnings in avc/core.rs and decoder/mod.rs 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add void *dtvcc_rust field to lib_cc_decode struct - Declare ccxr_dtvcc_init, ccxr_dtvcc_free, ccxr_dtvcc_process_data in ccx_dtvcc.h - Declare ccxr_dtvcc_set_encoder in lib_ccx.h - Declare ccxr_flush_active_decoders in ccx_decoders_common.h - All declarations guarded with #ifndef DISABLE_RUST - Update implementation plan to mark Phase 2 complete 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- init_cc_decode(): Initialize dtvcc_rust via ccxr_dtvcc_init() - dinit_cc_decode(): Free dtvcc_rust via ccxr_dtvcc_free() - flush_cc_decode(): Flush via ccxr_flush_active_decoders() - general_loop.c: Set encoder via ccxr_dtvcc_set_encoder() (3 locations) - mp4.c: Use ccxr_dtvcc_set_encoder() and ccxr_dtvcc_process_data() - Add ccxr_dtvcc_is_active() declaration to ccx_dtvcc.h - Fix clippy warnings in tv_screen.rs (unused assignments) - All changes guarded with #ifndef DISABLE_RUST - Update implementation plan to mark Phase 3 complete 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove extra space before comment in ccx_decoders_common.c - Fix comment indentation in mp4.c 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The ccxr_process_cc_data function was still accessing dec_ctx.dtvcc (which is NULL when Rust is enabled), causing a null pointer panic. Changed to use dec_ctx.dtvcc_rust (the persistent DtvccRust context) instead, which fixes the crash when processing CEA-708 data. Added do_cb_dtvcc_rust() function that works with DtvccRust instead of the old Dtvcc struct. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Move PLAN_PR1618_REIMPLEMENTATION.md to local plans/ folder - Add plans/ to .gitignore to keep plans local 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The cb_708 counter was being incremented twice for each CEA-708 data block: 1. In do_cb_dtvcc_rust() in Rust (src/rust/src/lib.rs) 2. In do_cb() in C (src/lib_ccx/ccx_decoders_common.c) Since FTS calculation uses cb_708 (fts = fts_now + fts_global + cb_708 * 1001 / 30), the double-increment caused timestamps to advance ~2x as fast as expected, resulting in incorrect milliseconds in start timestamps. This fix removes the increment from the Rust code since the C code already handles it in do_cb(). Fixes timestamp issues reported in PR CCExtractor#1782 tests where start times like 00:00:20,688 were incorrectly output as 00:00:20,737. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
4ff33fc to
cf74762
Compare
CCExtractor CI platform finished running the test files on windows. Below is a summary of the test results, when compared to test for commit 134cd75...:
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. |
Fix CEA-708 Decoder State Persistence (Issue #1499)
This PR fully implements the fix for issue #1499. The Rust CEA-708 decoder was creating a new
Dtvccstruct on every call toccxr_process_cc_data(), causing all state to be reset and breaking stateful caption processing.Based on #1618, which doesn't merge cleanly and contains implementations that differ from current code.
Changes by Phase
Phase 1: Rust Core
DtvccRuststruct indecoder/mod.rsthat owns its decoder stateCCX_DTVCC_MAX_SERVICESconstant (63)lib.rs:ccxr_dtvcc_init(): Create persistent contextccxr_dtvcc_free(): Free context and all owned memoryccxr_dtvcc_set_encoder(): Set encoder (not available at init)ccxr_dtvcc_process_data(): Process CC data on persistent contextccxr_flush_active_decoders(): Flush all active decodersccxr_dtvcc_is_active(): Check if context is activeDtvccRustPhase 2: C Headers
void *dtvcc_rustfield tolib_cc_decodestruct inccx_decoders_structs.hccx_dtvcc.hfor init/free/process_data/is_activelib_ccx.hforccxr_dtvcc_set_encoderccx_decoders_common.hforccxr_flush_active_decodersPhase 3: C Implementation
ccx_decoders_common.c:init_cc_decode(): Useccxr_dtvcc_init()when Rust enableddinit_cc_decode(): Useccxr_dtvcc_free()when Rust enabledflush_cc_decode(): Useccxr_flush_active_decoders()when Rust enabledgeneral_loop.c: Set encoder viaccxr_dtvcc_set_encoder()at 3 locationsmp4.c: Useccxr_dtvcc_set_encoder()andccxr_dtvcc_process_data()#ifndef DISABLE_RUSTPhase 4: Bug Fix & Testing
ccxr_process_cc_data()to use persistentDtvccRustfromdec_ctx.dtvcc_rustinstead of creating newDtvccfromdec_ctx.dtvcc(which is NULL when Rust enabled)do_cb_dtvcc_rust()function for processing withDtvccRustTesting
Automated Tests
Manual Testing
Files Modified
src/rust/src/decoder/mod.rsDtvccRuststruct and methodssrc/rust/src/lib.rsccxr_process_cc_datasrc/lib_ccx/ccx_decoders_structs.hdtvcc_rustfieldsrc/lib_ccx/ccx_dtvcc.hsrc/lib_ccx/lib_ccx.hccxr_dtvcc_set_encoderdeclarationsrc/lib_ccx/ccx_decoders_common.hccxr_flush_active_decodersdeclarationsrc/lib_ccx/ccx_decoders_common.csrc/lib_ccx/general_loop.csrc/lib_ccx/mp4.cFixes: #1499