[FIX] Clear stale CEA-708 window rows in no-rollup mode - #2318
Open
nlp201 wants to merge 1 commit into
Open
Conversation
Collaborator
CCExtractor CI platform finished running the test files on linux. Below is a summary of the test results, when compared to test for commit 6077cf5...:
Your PR breaks these cases:
NOTE: The following tests have been failing on the master branch as well as the PR:
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In raising this pull request, I confirm the following (please check boxes):
Reason for this PR:
Sanity check:
Repro instructions:
The input is a 35.1-second, video-only H.264 MPEG-TS sample. To avoid distributing the original broadcast pictures, every decoded frame was replaced with black before H.264 re-encoding. FFmpeg preserved the A53/CEA-708 caption side data in the new H.264 SEI messages. The sample contains Korean CEA-708 service 1 and no audio.
Sample URL: https://sampleplatform.ccextractor.org/sample/188
Sample properties:
The Sample Platform identifies the uploaded caption stream as EIA-708 and reports the same SHA-256 as the locally verified redacted artifact.
Baseline commit:
Build from the checked-out source:
docker build \ --build-arg USE_LOCAL_SOURCE=1 \ --build-arg BUILD_TYPE=minimal \ -f docker/Dockerfile \ -t ccextractor:local .Run the same command against the baseline and this change:
For this service, CCExtractor writes
result.p1.svc01.srt.Description
When a CEA-708 carriage return reaches a roll-up boundary, CCExtractor first emits the current window. With
--no-rollup, it then clears only the current pen row. Text in previously emitted upper rows remains in the backing window and is emitted again with later cues.This change clears every row in the active window after emission when
--no-rollupis enabled, then marks the window empty. It deliberately preserves the current pen location, pen color, and pen attributes. Normal roll-up behavior is unchanged.The C and Rust CEA-708 implementations are updated together. The Rust regression test exercises
process_cr(..., no_rollup = true), verifies that all emitted rows are cleared, verifiesis_empty, and verifies that active pen styling is preserved.Before
The second cue repeats two lines already emitted by the first cue:
Those stale lines recur in later cues. The baseline also emits a cue at
00:00:21,255 --> 00:00:27,327containing only stale text.After
The second cue contains only its new text:
The original two lines remain together in cue 1, where they are valid, but do not reappear. The stale-only cue disappears. The redacted sample produces 12 baseline cues and 11 fixed cues; all fixed lines at shared timestamps are the expected ordered suffix of the baseline lines.
Cross-broadcaster validation
Representative 10-minute CEA-708 service-1 samples from five Korean broadcasters were compared with identical options:
All five samples use three-row CEA-708 windows. SBS starts at row 0 and relies on CR to advance through the window. Once the pen reaches the final row, clearing only that row leaves rows 0 and 1 available for re-emission. The other four samples set the pen directly to the final row for each cue, so their upper rows are already empty.
An initial candidate used
clear_text(). That fixed SBS but reset the active pen patterns and removed all 40 yellow color spans from MBC. The final implementation clears row contents and setsis_empty = 1without resetting active pen color or attributes. This restores byte-for-byte output parity for the four unaffected broadcasters.Tests
The focused regression was first confirmed to fail on the old one-row behavior. It was then strengthened after the rejected
clear_text()candidate to require preservation of a non-default foreground color and italic state. The unit test drives the realprocess_cr(..., no_rollup = true)path and fixes the decoder-state contract; the redacted-sample A/B run below separately verifies the user-visible SRT output across consecutive emissions.The redacted-sample output comparison confirms that the first valid multi-line cue is preserved, later cues no longer repeat its stale upper rows, no fixed-only text or timestamp interval is introduced, and the stale-only baseline cue disappears.
cargo clippy --lib -- -D warningsreports four existingunnecessary_castfindings inlibccxr_exports/time.rsandlibccxr_exports/util.rs. The same four findings occur on the clean baseline; this change introduces no new clippy findings.No public checks existed for the fork commit before this PR. The local results above are exact-artifact results; repository CI for this branch should run on the PR.
Production validation
The fixed binary was also exercised by the production caption pipeline against seven archived days from the same five broadcasters. The normal transactional publication and verifier paths completed successfully:
All 35 publication manifests verified the expected binary path, version
0.96.5, extraction policy, and current generation. This operational run supplements—but does not replace—the focused regression, reproducible sample, or repository CI.Scope
This PR does not change CEA-608, the
no_rollup = falsepath, public APIs, or dependencies. It does not include the original broadcast video. The provided reproduction sample contains black video plus the caption packets and timing needed to reproduce the decoder bug.Checklist