Repository navigation
[FIX] Use the detected frame rate for CEA-708 SCC timecodes - #2406
Open
harshalsp07 wants to merge 1 commit into
Open
harshalsp07 wants to merge 1 commit into
harshalsp07 wants to merge 1 commit into
Conversation
print_scc_time() and its Rust counterparts computed the SCC frame field with a hardcoded 29.97, so SCC output from 24/25/30 fps sources carried the wrong frame numbers. Use current_fps in C and in the Rust 708 writer, and pass the rate to Timestamp::to_scc_time(). The Rust writer also still subtracted a hardcoded 29.97 frame delay in the overlap/clear cases; match the C path, which has used current_fps since CCExtractor#2173. While there, fix the u8 overflow in to_scc_time() and print an integer frame instead of a fractional value.
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:
Fixes #2145.
What changed
CEA-708 SCC timecodes used a hardcoded 29.97 for the frame field. The writers now use the detected
current_fps:print_scc_time()insrc/lib_ccx/ccx_common_timing.c.get_scc_time_str()insrc/rust/src/decoder/timing.rs, plus the one-frame delay indtvcc_tv_screen::write_scc().Timestamp::to_scc_time()now takes the frame rate. Its oldu8seconds arithmetic overflowed after 255 seconds; it usesi64now and prints an integer frame.--scc-framerateis unchanged. It still controls the CEA-608 SCC encoder and SCC input parsing. The 708 writer uses the stream rate, as requested in #2145.Why
For a 25 fps source, the frame field was calculated with 29.97, so frame numbers drifted within each second. This is the same bug as the closed #2146, fixed here for both writers.
cfsmp3's review on #2146 asked forget_scc_time_strandto_scc_timeto be updated too; this PR includes them. #2173 already switched the C 708 delay tocurrent_fps; the Rust writer still subtracted1000 / 29.97 as i64in its overlap branches.Testing
I used a 25 fps H.264 MPEG-TS with one CEA-708 service 1 caption. It is generated for this PR, not a broadcast capture; the base64 is at the end. The SPS has
fixed_frame_rate_flag=1, socurrent_fpsis 25.Save the base64 block at the end as
sample25.b64, then:base64 -d sample25.b64 > sample25.ts ccextractor sample25.ts --service 1 --out=scc cat sample25.p1.svc01.sccResults with the same sample:
d7eb132e:00:00:04;07 942c 942c ...00:00:04;06 942c 942c ...The caption FTS is 4.28 s. SCC subtracts one frame (40 ms), so the timecode is 4.24 s, which is frame 6 at 25 fps. 29.97 gives frame 7.
Checks:
cargo testinsrc/rust: 428 passed.cargo testinsrc/rust/lib_ccxr: 55 unit and 46 doc tests passed.cargo fmt --all -- --checkandcargo clippy -- -D warningsin both crates.ctestin a local CMake build: 1/1 passed.clang-format --dry-run -Werror src/lib_ccx/ccx_common_timing.c.The default build uses the Rust 708 writer, so the C
print_scc_time()change is not on the sample path. I checked it directly by linking the compiledccx_common_timing.cobject into a small harness: 960 ms at 29.97/25/24 gives frames 28/24/23, and 500 ms at 30 gives frame 15.Output changes
Non-29.97 SCC output changes, which is the point. For 29.97, the Rust one-frame delay was
1000 / (29.97 as i64)(34 ms) and is now(1000 / 29.97) as i64(33 ms), closer to the C writer. That can move a timecode by one frame at a boundary. Any sample-platform test that compares SCC output from a non-29.97 source will need its baseline regenerated.sample25.ts (base64, 21,808 bytes, sha256 00341a21cc2381a09397bf4356f70adb61b9b063dfa459ac299f273ebf471c09)