Repository navigation
[IMPROVEMENT] Document why store_hdcc overwrites the slot - #2392
Merged
Merged
Conversation
The comments around the reset in store_hdcc() say it was "changed to concat" with a TODO, next to a commented-out stream_mode guard, which reads as if the unconditional reset were a bug (issue #2350, PR #2351). It is load-bearing. On a real ATSC MPEG-2 stream each picture carries its 608 pairs twice, as SCTE-20 user data followed by GA94: every same-slot store_hdcc() call in that file overwrites a block whose 608 pairs are identical to the GA94 block's. Appending instead duplicates every character ("♪ S Standtandining g" for "♪ Standing"). None of the other 40 local 608/708 samples ever stores twice into one slot. Replace the stale comments with the reason and drop the dead stream_mode lines. No code change.
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
1 participant
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:
Comment-only change, no behaviour change. To see why the reset matters: remove
dec_ctx->cc_data_count[seq_index] = 0;instore_hdcc()(so same-picture blocks append) and run sample9a496d38…(ATSC MPEG-2 TS) with--out=srt. Every 608 character is doubled:♪ S Standtandining g initinstead of♪ Standing in front.Re #2350. The comments around the unconditional reset in
store_hdcc()("Changed by CFS to concat… TODO", plus a commented-outstream_modeguard) make it look like a bug, and #2350 and #2351 set out to "restore" concatenation.Instrumenting
store_hdcc()across all 41 local samples with 608/708 captions: 40 never store twice into one slot with the same timestamp. The one that does, an ATSC MPEG-2 stream, carries each picture's 608 pairs twice, as SCTE-20 user data followed by GA94, and in all 455 such calls the overwritten block's 608 pairs are identical to the GA94 block's. The overwrite is what de-duplicates them; appending doubles every character. The decoder side would handle several envelopes per slot fine, so it's the content, not the format, that makes concatenation wrong.This replaces the stale comments with that reason and drops the dead
stream_modelines. Output is byte-identical to master.