fix(scc/ccd): encode EIA-608 special/extended characters instead of raw internal bytes - #2301
Open
x15sr71 wants to merge 2 commits into
Open
fix(scc/ccd): encode EIA-608 special/extended characters instead of raw internal bytes#2301x15sr71 wants to merge 2 commits into
x15sr71 wants to merge 2 commits into
Conversation
Pre-existing clippy::byte_char_slices lint on master fails 'cargo clippy -- -D warnings' for every PR. Identical to the fix in CCExtractor#2298 so the two branches merge without conflict regardless of order.
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 2feb09a...:
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. |
Collaborator
CCExtractor CI platform finished running the test files on windows. Below is a summary of the test results, when compared to test for commit 9f78685...:
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. |
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:
Sample
From issue #2098 (posted by @yukichigai, a 26-second segment):
DresdenTest.ts— https://1drv.ms/v/c/8b5ee05414946e03/IQDLLzwLvNY3RJi8rYvDN7PyAVTV7dC2YotrGHTGfnRkFUM?e=4YsJPQBefore this PR — the bug
The apostrophe is a single raw byte
0x99(invalid UTF-8). On macOS/BSDgrep(and GNU grep in a UTF-8 locale) a plaingrepwill silently drop the line because it contains that invalid byte — so view it withcat -v,LC_ALL=C grep, orhexdump:Fixes #2098
Summary
Special/extended EIA-608 characters (apostrophes, quotes, music notes, accented letters, etc.) are corrupted in
--out=sccand--out=ccd. The reported case is the possessive apostrophe inMOTHER'S/THAT'S:--out=ccdwrites the apostrophe as a single raw byte0x99(invalid UTF-8) — renders as�in a UTF-8 terminal,™under Windows-1252.--out=sccemits that same0x99byte verbatim, which re-decodes as a stray control code and destroys the apostrophe and the following letter.--out=ttxt,--out=srt,--out=g608are all correct, which localizes the bug to the SCC/CCD writer.Root cause
The SCC and CCD writers (
src/lib_ccx/ccx_encoders_scc.c, both viawrite_cc_buffer_as_scenarist) re-encode the already-decodedeia608_screengrid. In that grid, special/extended characters are stored as CCExtractor'sinternal codes (
>= 0x80), not as valid single-byte characters. For the apostrophe, the 608 decoder collapses the on-aira7 80+92 29into a single grid cell holding internal byte0x99.write_character()emitted that internal byte verbatim — one raw byte for SCC (odd_parity(0x99)), and the raw byte for CCD. The internal→Unicode table (get_char_in_utf_8) is already correct (0x99 → U+0027); the writer simplybypassed it. The issue is in the writer, not the internal→Unicode mapping table.
SCC fix
Reverse-map the internal code back to its EIA-608 two-byte code and emit a padded fallback base character followed by the extended pair:
0x80–0x8f0x11c − 0x500x90–0xaf0x12c − 0x700xb0–0xcf0x13c − 0x90(field 2 / CC3/CC4:
hi += 8, matching the existingis_odd_channelconvention.)The extended code destructively backspace-replaces the preceding cell, so a padded base character must occupy that cell first, and the extended pair must stay 2-byte aligned (handled by
check_padding). The base character is the single-byte ASCII form when the character has one (apostrophe →0x27), otherwise a space.Result for the apostrophe:
a7 80 92 29.CCD fix
Emit the real UTF-8 glyph via
get_char_in_utf_8instead of the raw byte. This fixes every special/extended character (the table is already correct for all of0x80–0xcf), not just the apostrophe.How FFmpeg does it
FFmpeg's raw 608 bytes for the same file (
subcc+ stream copy) encode the apostrophe exactly as this fix now does — base char + pad frame, then the extended frame:Our output matches the expected character sequence. FFmpeg additionally duplicates some control codes for transmission reliability, while CCExtractor emits each once; this is pre-existing behavior and unchanged by this PR.
Testing
After this PR — fixed
Round-trip (no character loss, no control-code misinterpretation)
No regression to other formats
--out=srt,--out=ttxt,--out=g608, and--out=binare byte-identical before and after this change (verified by rebuilding the parent commit and diffing each output). The change only touches the SCC/CCD writer path.Scope & follow-up
src/lib_ccx/ccx_encoders_scc.c(the fix) and a one-linesrc/rust/src/demuxer/stream_functions.rsdrive-by fix for a pre-existingclippy::byte_char_sliceslint that blocks CI0x80–0xCF) verified to round-trip correctly (e.g.♪ ' — " " Ã } ⌟ ¥ Ø).calculate_caption_bytesstill counts extended chars as 1 byte (now 3–4). This only affects--scc_accurate_timing(off by default) as a minor preroll undercount — left out to keep this fix minimal.