* [FIX] Warn when non-DVB caption PIDs are silently ignored
Closes#2333
When a program contains multiple caption-carrying PIDs of the same
non-DVB codec (Teletext, ISDB, or ATSC), only one is ever decoded —
every other same-type PID is silently discarded with no warning, no
log line, and no indication to the user that a second language track
existed.
This adds a warning at the actual root cause: the DVB-only exemption
in `ignore_other_stream()` and `ignore_other_sib_stream()`
(`src/lib_ccx/ts_info.c`). A new static helper,
`warn_ignored_caption_stream()`, prints the ignored PID and points the
user to `--datapid` to recover it manually. DVB is explicitly excluded
since it already has its own extra-PID handling and doesn't need this.
**Scope**: this is a warning only. It does NOT add multi-PID decoding
support for Teletext/ISDB/ATSC — see #2333 for why that's a separate,
larger effort (per-PID decoder allocation, per-language encoder
routing, and ISO-639 storage for Teletext don't exist today for
non-DVB codecs).
**Real broadcast sample** (two Teletext PIDs, one program — from
samples.ffmpeg.org/ffmpeg-bugs/trac/ticket3514):
$ ccextractor --tpages-all t3514.ts
VBI/teletext stream ID 3401 (0xd49) for SID 1002 (0x3ea)
VBI/teletext stream ID 3402 (0xd4a) for SID 1002 (0x3ea)
Warning: Teletext caption stream ID 3402 (0xd4a) for SID 1002 (0x3ea)
will be ignored - only one caption stream per program is extracted.
Use --datapid 3402 to extract it in a separate run.
Correct PID, correct SID. The 8 DVB subtitle PIDs in the same file stay
silent, as expected.
**Synthetic two-PID split** (dvbteletext.ts split across two PIDs):
same correct behavior, warns on the dropped PID (202/0xca).
**ISDB and ATSC branches** — verified against a constructed sample
(`mixed_codecs.ts`, built from `dvbteletext.ts` by injecting two ES
entries into its PMT: PID 0xca as ISDB via a data_component descriptor,
PID 0xcb as ATSC via a caption_service descriptor):
$ ccextractor --tpages-all mixed_codecs.ts
VBI/teletext stream ID 201 (0xc9) for SID 201 (0xc9)
*****ISDB subtitles detected
Warning: ISDB caption stream ID 202 (0xca) for SID 201 (0xc9) will be
ignored - only one caption stream per program is extracted.
Use --datapid 202 to extract it in a separate run.
Warning: ATSC caption stream ID 203 (0xcb) for SID 201 (0xc9) will be
ignored - only one caption stream per program is extracted.
Use --datapid 203 to extract it in a separate run.
Teletext output (`mixed_codecs_p694.srt`, `_p765.srt`) is byte-identical
to the unmodified `dvbteletext.ts` baseline — the injected PIDs change
nothing except triggering the new warnings.
Note: I could not source a real multi-PID ISDB recording — the one
public ARIB sample I found (samples.ffmpeg.org/MPEG2/subcc, single PID)
has a multi-TS-packet PMT my synthetic splitter doesn't handle, so the
ISDB/ATSC branches above are exercised by a constructed sample rather
than a genuine broadcast recording. Flagging this rather than implying
otherwise — the Teletext path is verified on real broadcast data (see
above), ISDB/ATSC are verified by construction.
**DVB control** — confirms no regression: ran both a single-PID and a
split two-PID DVB sample before and after the patch. No new warnings
on either, and output is byte-identical (PNG counts and `diff -rq`
match exactly pre- and post-patch). All decoded `.srt` files from the
Teletext samples are also byte-identical to their pre-patch versions —
this change only adds a print statement, no decoding logic is touched.
**Caught during testing, fixed before this PR**: the first version of
this warned on every video PID in every file, because 608/708 captions
are internally tagged `CCX_CODEC_ATSC_CC` even though they live inside
the video stream, not a separate PID (`ts_tables.c:492-498`). Fixed by
excluding `CCX_STREAM_TYPE_VIDEO_MPEG2/_H264/_HEVC` from the warning.
Re-verified against a European DVB file (now silent, as it should be)
and a US ATSC file with real 608/708 captions in video (still silent,
correct — genuine caption-only ATSC PIDs still warn correctly).
**Known cosmetic issue, pre-existing, not introduced by this patch**:
under `--pmt`, the ATSC warning can visually run together with a
preceding debug line (`ts_tables.c:445-449` emits its `dbg_print`
without a trailing newline, so whatever prints next collides with it
on the same line). This only occurs with `--pmt` and would happen with
any output following that debug line, not just this warning. Out of
scope for this PR per its stated scope, noting it here so it isn't
mistaken for something this patch caused.
**Reason for this PR:**
- [ ] This PR adds new functionality.
- [x] This PR fixes a bug that I have personally experienced or that a
real user has reported and for which a sample exists.
- [ ] This PR is porting code from C to Rust.
**Sanity check:**
- [x] I have read and understood the contributors guide.
- [x] I have checked that another pull request for this purpose does not
exist.
- [x] If the PR adds new functionality, I've added it to the changelog.
If it's just a bug fix, I have NOT added it to the changelog.
- [x] I am NOT adding new C code unless it's to fix an existing,
reproducible bug.
* fix: caption stream warning format specifiers
switch_to_next_file() validates current_file, then calls open() and
re-reads inputfile[current_file] afterwards. open() reaches
buffered_read_opt(), which calls switch_to_next_file() again when
binary_concat is set, and that advances current_file. The index the
caller validated no longer points at the file it opened, and with the
last file it points one past the end of the array.
Passing two input files therefore segfaults: the Rust copy dereferences
inputfile[2] of a two-element array and hands the garbage pointer to
CStr::from_ptr. With one file it is an 8-byte read past the allocation,
which valgrind reports on any run.
Hold the name before calling open() and use it afterwards, in both the C
function and its Rust port. Neither re-reads the index it no longer owns.
X-TIMESTAMP-MAP emitted a truncated MPEGTS value on every Windows build.
sync_pts2fts_pts is LLONG (int64_t) but was printed with %ld, and long is
32 bits on Windows (LLP64 on x64, ILP32 on Win32). A 90kHz PTS of 2^33
printed as "MPEGTS:0"; 2^31 printed as "MPEGTS:-2147483648". On Win32 the
argument slots also desynchronise, so the trailing %s reads the PTS high
dword as a char *. HLS players use this field to anchor a WebVTT segment
to the transport clock, so the result is caption desync.
Linux and macOS are LP64, where long is 64 bits and %ld happens to be
correct, which is why this went unnoticed. For the same reason %lld would
be wrong there, since int64_t is long on LP64. Use the <inttypes.h> macros
instead, which are correct on every target and already used elsewhere in
the tree; ccx_common_platform.h already includes <inttypes.h>.
The same defect class appears at 22 sites across 10 files. They are
invisible to the build because mprint() and dbg_print() carry no
format(printf) attribute, so the compiler never checks those call sites.
Adding the attributes locally surfaced all of them; this commit fixes them
with PRId64/PRIu64/PRIx64 for fixed-width types, %zu for size_t, and an
explicit (long long) cast for pointer differences.
Also fixes ts_readpacket() printing payload->start, an unsigned char *,
under the label "PES start". The intended field is payload->pesstart; the
pointer was truncated to int and shifted the three arguments after it, so
counter, payload length and adapt length were all wrong in the trace.
Output on Linux and macOS is byte-identical before and after. No
signature, struct, option or output-format change.
Refs #2336
write_webvtt_header() is called once per cue and guards re-entry with
context->wrote_webvtt_header, which it sets on its last line. The
--webvtt-create-css path returned early when the .css file could not be
created, bypassing that assignment, so the whole header block was written
again before every subsequent cue. The existing trailing comment ("Do it
even if couldn't write the header, because it won't be possible anyway")
already stated the intent.
Replace the early return with an else branch so the flag is always
reached, and move free(css_file_name) after it so both paths release it.
basefilename came from get_basename(), which allocates, but was never
freed on any path. Every other caller in the tree frees it. On its own
that is one small leak per output file; together with the early return it
became a leak per cue.
Refs #2338
ccextractor sample.rcwt --out=webvtt --timestamp-map segfaults.
init_encoder() allocates encoder_ctx with malloc and assigns 60 of its 74
members. timing is one of the nine it does not touch, and it is only set
later in general_loop(), the DVB path, mp4.c, and raw_loop() -- the last
of these only when the output format is MCC. rcwt_loop(), process_hex()
and raw_loop() for every other format therefore leave it holding
indeterminate heap contents.
write_webvtt_header() guards the field with "context->timing != NULL"
before reading sync_pts2fts_set. The guard is correct, but an
indeterminate value is almost never NULL, so it passes and the read
dereferences a wild pointer. --timestamp-map is the only thing that makes
timing be read on this path, which is why the crash needs that flag.
Allocate with calloc so every member starts zeroed. The 60 assigned
members are unaffected; timing becomes NULL and the existing guard works
as written, so RCWT input skips X-TIMESTAMP-MAP instead of crashing --
which is the right result, since sync_pts2fts_set would be 0 there.
Verified by building master and this change from identical source in the
same image: exit 139 -> exit 0, and output across 13 formats (webvtt, srt,
sami, ttxt, txt, scc, ssa, smptett, g608, mcc, spupng, simple_xml, rcwt)
is byte-identical, same exit codes and same SHA-256 hashes.
Refs #2340
ccx_common_char_encoding.h in ccx_decoders_common.c is unused —
the functions it declares moved to ccx_encoders_common.c after a
2014 refactor and the include was never cleaned up.
ccx_demuxer_mxf.h in general_loop.c is also unused — general_loop.c
handles MXF via ccx_mxf_getmoredata, which is already declared in
lib_ccx.h (included separately). This header appears to have been
added by reflex alongside MXF support in 2017 without ever being
needed.
A third flagged include, ffmpeg_intgr.h in general_loop.c, was
investigated and left untouched — it's a genuine dependency under
Closes#2328
Master fails `cargo clippy -- -D warnings` on lib_ccxr, so format_rust
is red on every open pull request regardless of what it changes. Authors
have been fixing it inside unrelated PRs to get their CI green, which
couples a one-line lint fix to whatever else they are working on.
Assign c_len from an if-expression instead of declaring it and filling
it in on each branch. Behaviour is identical.
* fix: NUL-terminate mode and info in add_cc_sub_text
strncpy of length 4 left 4-character strings such as ISDB and BURN without a terminator.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix: satisfy clippy needless_late_init in txt_helpers
The format_rust CI job treats clippy warnings as errors. Initialize c_len in one expression so the check passes.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Revert "fix: satisfy clippy needless_late_init in txt_helpers"
This reverts commit fc7ab9781d.
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
get_file_size() stored an LSEEK result in an int. LSEEK returns a 64-bit
offset, so a successful seek back to a position at or beyond 2 GB truncated
to a negative int and the function reported failure for a seek that worked.
ccx_demuxer_get_file_size() is the same routine written correctly with an
LLONG; this copy had drifted. Latent today -- the only caller opens each file
fresh, so the position is always 0 -- but it is a trap for the next caller.
buffered_read_opt() computed the bytes left in the file buffer as
"bytesinbuffer - filebuffer_pos" on two unsigned ints. That is the underflow
that made the bounds checks in file_buffer.h pass when they should have
failed; the helper added to fix those, buffered_bytes_left(), belongs here too.
The end-of-input test compared inputsize, a signed 64-bit size, against
origin_buffer_size, a size_t. The usual arithmetic conversions turned a
negative inputsize -- what get_filesize() returns when the size cannot be
determined -- into a huge unsigned value, so the test silently read as "the
input is enormous". The comparison is now spelled out. An unknown size still
does not count as fitting in one read, and the short-circuit order is
unchanged, so switch_to_next_file() is called exactly when it was before.
Verified against the current master binary: byte-identical output on 99 files
from the sample library, and on the --bufferinput, multi-file concat and
stdin paths, with matching exit codes throughout.
update_encoder_list_cinfo() declares enc_ctx uninitialised and uses it as the
list_for_each_entry() iterator. For a DVB stream carrying a language, the search
loop continues past every encoder whose dvb_lang does not match, so it can finish
without returning. When the encoder list is non-empty none of the creation
branches below run either, and the function falls through to
enc_ctx->prev = NULL;
with enc_ctx still holding the value the loop terminated on: the list head
reinterpreted as an encoder_ctx. That pointer is written to, and later
general_loop.c does dvb_enc->timing = dvb_dec->timing on it, which segfaults.
It reproduces on any recording that carries both a teletext stream and a DVB
subtitle stream, in every output format, with no extra flags:
ccextractor --out=srt sample.mpg -> SIGSEGV
ccextractor --out=webvtt sample.ts -> SIGSEGV
Restricting to one PID (--datapid) avoids it, which is why the regression suite
never caught it: the only affected sample it runs passes --datapid.
Clear the iterator after the loop so "not found" is distinguishable, and give
such a stream its own per-language encoder, which is what the multi-DVB path
already does. The existing dvb_pid_count >= 2 test stays as an OR so a recording
whose only caption stream is a single DVB PID keeps its plain output filename --
that case reaches the standard path with an empty encoder list. A NULL guard
before the tail keeps the fall-through honest if some future path adds one.
Both streams are now extracted into separate, well-formed files (out.srt plus
out_eng.srt) rather than one file with two interleaved numberings.
* fix(file_buffer): stop unsigned underflow turning bounds checks into passes
buffered_seek() handled a negative offset with
ctx->filebuffer_pos += offset;
if (ctx->filebuffer_pos < 0)
but filebuffer_pos is unsigned int, so the addition wraps instead of going
negative and the check can never be true. The guard below it compares
filebuffer_pos + startbytes_pos against 0 and is dead for the same reason, so
the "seek before buffer start" fatal() is unreachable.
Seeking back further than the current position therefore left filebuffer_pos at
roughly 2^32. buffered_read() then evaluated
bytes <= ctx->bytesinbuffer - ctx->filebuffer_pos
which underflows too, so the bounds check passed and memcpy() read from
filebuffer + 2.3e9 against a 1 MB buffer.
Observed on a malformed WTV file, where a corrupt chunk offset produces
buffered_seek(ctx, -1969339576) with filebuffer_pos = 1048576:
filebuffer_pos -> 2326676296
bytesinbuffer = 1048576
bytesinbuffer - filebuffer_pos = 1969339576 (underflow)
memcpy(dst, filebuffer + 2326676296, 32) -> SIGSEGV
Do the seek arithmetic in int64_t so both guards work as written, and route the
three buffer-space checks through a buffered_bytes_left() helper that returns 0
when the position is past the end. buffered_read_byte() had the same hazard and
would have indexed filebuffer[] out of bounds.
Input files are untrusted, so this is a bounds-check fix, not only a crash fix.
* fix(file_buffer): make the buffer-position narrowing explicit
MSVC warns C4267 on "filebuffer_pos += bytes": filebuffer_pos is an unsigned
int and bytes is a size_t. The warning predates this branch -- it fires 54
times on master -- but the guard this commit series added is what makes the
conversion provably safe, so state that in the code instead of leaving a
warning that a reader has to re-derive.
bytes is known to be no larger than what is left of the buffer, and that
remainder is itself an unsigned int, so the value always fits.
get_data() reads a 32-byte element header and believes whatever 32-bit length
it finds. On one recording the parser walks 182,902 elements correctly and then
meets 8800 bytes it cannot interpret; the "length" read there is 0x8A9E4344.
That value is passed to skip_sized_buffer(), which hands it to buffered_seek()
as an int, turning a forward skip into a 1.8 GB backward seek and a segfault.
Reject a length that cannot describe an element (below the 32-byte header, or
implausibly large) and hunt forward for the next header instead. Most GUIDs that
open a timeline element share the same 15 trailing bytes and differ only in the
first, which makes them a dependable anchor; the scan keeps a rolling 32-byte
window so the caller ends up positioned exactly as after a normal header read.
The end-of-file marker is exempt from the length test. It legitimately carries a
zero length and is handled further down, so treating it as garbage ended the
parse early and changed the last cue of every WTV recording.
On the affected file the parser resynchronises after 8800 bytes -- matching the
gap measured independently between the last valid element and the next one --
and carries on to the end of the recording.
Verified byte for byte over the 45 local WTV samples, master vs this branch:
byte-identical output : 44
differing : 0
crash fixed : 1, cues 100 -> 151
Coverage on that recording goes from 228s to 356s of a 358s file, and the
resync fires exactly once across the whole corpus.
Updates every actions/checkout@v6 usage across the workflows to v7 (32 call
sites, covering all workflows that use it).
v7's breaking change is that it refuses to check out a fork pull request for
pull_request_target and workflow_run events. Nothing here is affected: no
workflow uses pull_request_target, publish_winget.yml has no checkout step, and
publish_chocolatey.yml -- the only workflow_run job that checks out -- passes
neither ref nor repository, so it takes the base repository's default branch
rather than a fork PR.
v7 also moves the action to ESM with a newer Node runtime, which is fine on the
GitHub-hosted runners we use throughout.
On Windows the argv handed to main() is encoded in the process ANSI code page,
so a path containing characters outside it arrived mangled. ccxr_parse_parameters()
builds its strings with CStr::to_string_lossy(), which replaced those bytes with
U+FFFD, and the file never opened.
main() now takes the Unicode command line via GetCommandLineW() /
CommandLineToArgvW() and converts each argument to UTF-8, keeping the array
NULL-terminated as the C standard requires and checking every allocation.
Converting argv is not sufficient on its own: the C file APIs (_open, fopen)
take narrow strings and interpret them in the ANSI code page, so
get_total_file_size() would still fail on the same paths -- only with different
bytes. setlocale(LC_ALL, ".UTF8") switches the UCRT to UTF-8 so those APIs
accept the converted paths, and SetConsoleOutputCP(CP_UTF8) makes the console
render them correctly.
Non-Windows builds are unaffected: the #else branch keeps the existing
setlocale(LC_ALL, "") and setlocale(LC_NUMERIC, "POSIX") calls unchanged.
Fixes#2284.
process_telx_packet() hid each teletext page 40 ms (one frame at 25 fps) before the
page actually changed. That left a gap between consecutive cues, which players
that render WebVTT strictly show as a blink between rolling subtitles, and it
produced zero-length cues whenever a page was displayed for exactly 40 ms --
those vanish entirely.
The hide timestamp is now the page-change timestamp, so consecutive cues touch.
Touching cues are valid WebVTT and are what makes rolling text render
continuously. The underflow guard below it is removed: it existed only to catch
timestamp - 40 wrapping when timestamp was under 40 ms, which can no longer
happen.
Measured over the 20 teletext samples used by the regression suite: zero-length
cues drop from 103 to 0, 1054 cue boundaries become touching, and cue text and
start timestamps are byte-identical. Only end timestamps move, by +40 ms. On
sources whose page cadence is 80-100 ms a smaller gap remains, since the next
page's show timestamp comes from a later packet.
This shifts every teletext end timestamp, so the regression references for the
Teletext category need regenerating.
Fixes#2288.
net_send_cc() takes `int len` but the debug line printed it with %u. The other
two specifiers in these blocks were corrected in #2307; this one was missed.
The blocks only compile with -DNETWORKING_DEBUG=ON, so nothing shipped was
affected. Enabling them and building with -Wall -Wextra -Wformat=2 now reports
no format diagnostics at all; the two remaining -Wpointer-sign warnings on
write_block() are pre-existing and unrelated to the debug output.
Picked up from #2175 by Varadraj75, who reported the underlying issue (#2174)
and caught this specifier there; that PR was closed after going five months
without a rebase.
networking.c gated its debug output behind a hardcoded '#define DEBUG_OUT 0',
so the code was always compiled in and could only be enabled by editing the
source. It now follows the same pattern as VBI_DEBUG: a CMake option
(-DNETWORKING_DEBUG=ON) defines the macro, which in turn sets DEBUG_OUT.
Three fatal error messages were written to stdout via printf(). Since the
caption stream itself can go to stdout (--stdout), those could corrupt output
and were invisible when stdout was redirected. They now go to stderr.
Two format specifiers in the previously-dead debug blocks were wrong: 'len' is
a size_t in net_send_header() and net_send_epg(), printed with %u. Both are now
%zu, verified by enabling the blocks and building with -Wall.
Fixes#2174.
Two defects in the SCC/CCD writer.
1. Special characters gained a spurious leading space.
#2301 emits a fallback base character before every internal code >= 0x80, but
only EXTENDED characters (0x90-0xcf, hi 0x12/0x13) backspace-replace the cell
before them -- handle_extended() decrements cursor_column. SPECIAL characters
(0x80-0x8f, hi 0x11) are stand-alone: handle_double() writes them without
moving the cursor back, so the base character stays on screen and every one of
them came back one column to the right on re-decode.
Only emit the base character for extended codes. check_padding() still runs for
both so the two-byte code starts on an even offset and lands inside a single
SCC word.
Verified with an SCC exercising all 80 codes in 0x80-0xcf, re-encoded and
decoded again: 79/80 wrong before #2301, 16/80 after it, 0/80 now. On real
samples, 725a49f871 (15 music-note rows) and c032183ef0 (3) round-trip with no
altered text; the apostrophe from #2098 is unchanged at "a7 80 92 29".
2. Out-of-bounds control code index when a style change starts at column 0.
get_preamble_code() and get_tab_offset_code() take unsigned char, so the
column - 1 used to place the preamble one cell left wrapped to 255 at column 0,
yielding 255 / 4 = 63 and an index far past the end of control_codes[]. Row 12
produced code 186 against CONTROL_CODE_MAX 147.
In --out=ccd that garbage entry is passed to strlen() and segfaults; in
--out=scc it silently emits whatever ints follow the array. The read has been
there since before #2301 -- 2 of 20 local samples hit it -- but the layout
change from #2301 moved the garbage pointer into unmapped memory, so it now
crashes rather than misbehaving quietly.
Clamp the preamble column to 0, matching what the adjacent space branch already
does.
Non-SCC output (txt, sami, srt, ttxt, webvtt, g608) is byte-identical to master
across 20 samples. Of those 20, SCC output changes on 12 with special
characters and on 1 that hit the out-of-bounds index; the other 7 are
unchanged. Valgrind reports no invalid reads, only the pre-existing 32-byte
init_encoder leak that master has too.
The SCC and CCD writers re-encode the already-decoded eia608_screen grid, in
which special/extended characters are stored as CCExtractor internal codes
(>= 0x80) rather than valid single-byte characters. write_character() emitted
those internal bytes verbatim, so --out=ccd produced invalid UTF-8 (the
apostrophe became a lone 0x99) and --out=scc emitted a byte that re-decodes as
a stray control code, destroying the character and the one after it.
CCD now emits the real UTF-8 glyph via get_char_in_utf_8. SCC reverse-maps the
internal code back to its EIA-608 two-byte code and emits a padded fallback
base character followed by the extended pair, matching what real streams carry.
Fixes#2098.
--out report left options->extract at its default of 1 (field 1 only), so the
608 decoder's field gate skipped field 2 entirely and CC3/CC4 presence was
never probed. Report mode now sets extract = 12.
Explicit --output-field still wins: the Report arm runs during
set_output_format, and --output-field is parsed afterwards, so field selection
is honoured regardless of flag order. The 'both fields to stdout' guard is
relaxed for report mode only (write_format is Null, so there is no caption
stream to stdout); normal extraction is unaffected.
Side effect: XDS is also now reported correctly, since XDS lives on field 2.
Also includes an unrelated clippy byte_char_slices cleanup in
stream_functions.rs.
Fixes#2177.
* refactor(script): enhance error handling and user feedback in CCExtractor scripts
- Added shebang and defined CCEXTRACTOR variable for clarity.
- Implemented checks for the existence of the CCExtractor tool and for input file presence, providing user-friendly error messages.
- Improved exit status handling with specific messages for different outcomes of the CCExtractor execution.
- Ensured consistent user feedback for process completion and error scenarios.
* refactor(script): improve CCExtractor script for better error handling and user feedback
- Updated the CCEXTRACTOR variable assignment to dynamically locate the ccextractor tool.
- Enhanced input file checks to allow multiple files and provide clearer user messages.
- Improved exit status handling with detailed feedback for each processed file, including success, no captions found, and failures.
- Ensured consistent user notifications for process completion and errors.
* refactor(script): enhance user feedback in CCExtractor dialogs
- Added an escape function to properly format messages for display in macOS dialogs.
- Updated dialog display logic to use escaped strings, improving the clarity of user notifications.
When a stream switches from pop-on to roll-up mode and ends before
check_roll_up() reports changes=1 (fewer lines than the roll-up window
requires), the CR handler never runs the backfill at line 836 and
current_visible_start_ms is still zero when flush_608_context fires
EraseDisplayedMemory. Result: the first emitted caption is timestamped
at 00:00:00,000.
Capture the FTS of the first character after the transition in a new
field ts_first_char_rollup_transition. Unlike ts_start_of_current_line,
this field is not overwritten by intermediate changes=0 CRs, so the
first-char time survives to the end-of-stream flush path.
write_cc_buffer uses it to backfill current_visible_start_ms when
rollup_from_popon is still set at emit time, and clears it together
with the flag. The normal popon->roll-up flow (where a scrolling CR
eventually fires) and every other caption path are unchanged.
Verified on c4dd893cb9d6...ts: first subtitle now starts at
00:00:13,913 (was 00:00:00,000). Regression tested against six master
samples the mentor flagged (0069dffd.mpg 03,603; 5cbb21ad.dvr-ms
00,534; 132d7df7.mov 12,812; 6395b281.asf 02,436; 8849331d.mp4 03,771;
b22260d0.ts 04,838) - all match the reference timings unchanged.
Co-authored-by: GAURAV KARMAKAR <gaurav.k@graeon.ai>
* feat(mp4): add dvdsub/VobSub support to FFmpeg MP4 demuxer
Wire the existing vobsub_decoder (OCR-based bitmap subtitle decoder)
into the FFmpeg MP4 demuxer path. Previously, dvdsub tracks in MP4
containers were documented as unsupported — GPAC could extract them
but the FFmpeg path could not.
Changes:
- Add DvdSub track type detection (AV_CODEC_ID_DVD_SUBTITLE) in mp4.rs
- Add C bridge functions (ccx_mp4_vobsub_init/process/free) that call
the existing vobsub_decoder module
- Buffer dvdsub packets to compute end times from next-packet PTS
- Add --undefined linker flags for bridge symbols
Tested on sample 1f3e951d516b.mp4 (dvdsub in MP4):
- GPAC: 5405 bytes output
- FFmpeg + this patch: 5405 bytes, byte-identical to GPAC
With this change, the FFmpeg path extracts captions from every sample
that GPAC can, plus one additional sample (ad9f9e03240e.m4v) that
GPAC cannot handle.
* style: apply clang-format to vobsub bridge
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* ci: add Linux and macOS workflows for FFmpeg MP4 build
Adds a cmake_ffmpeg_mp4 job to both build_linux.yml and build_mac.yml
that configures CCExtractor with -DWITH_FFMPEG=ON -DWITH_OCR=ON
-DWITH_HARDSUBX=ON and builds it, so the FFmpeg-based MP4 demuxer
path is exercised on every PR. Linux job pulls the full set of
ffmpeg/tesseract/leptonica dev packages (including libavdevice-dev);
macOS job installs the corresponding Homebrew bottles.
* build: wire FFmpeg libs into ccx_rust link order
Add the compile-time option to build CCExtractor's MP4 demuxer on top
of libavformat: set -DENABLE_FFMPEG_MP4 and pull libswresample as a
required dependency alongside libavformat/libavutil/libavcodec/
libavfilter/libswscale when -DWITH_FFMPEG=ON.
Link-order handling. Corrosion places ccx_rust at the end of the
ccextractor link line. On Linux (GNU ld), ccx_rust contains rsmpeg
which references FFmpeg symbols like swr_get_out_samples, so the
FFmpeg shared libs must appear after ccx_rust. Collect them into a
separate EXTRA_FFMPEG_LIBS variable and attach them as
INTERFACE_LINK_LIBRARIES on the ccx_rust target so CMake emits them
right after ccx_rust. Make ccx's own link dependencies PRIVATE so
the same libs don't propagate earlier and get deduplicated against
the INTERFACE copy.
Force bridge symbols to be pulled from libccx. GNU ld only pulls
object files from a static archive when they resolve currently-needed
symbols. Bridge functions in libccx.a (ccx_mp4_process_nal_sample,
ccx_mp4_process_cc_packet, etc.) aren't needed until libccx_rust.a
is processed, but libccx.a precedes it on the command line. Use
-Wl,--undefined=<symbol> on ccextractor for each bridge entry
point so the linker pulls them early — same pattern already used
for decode_vbi/do_cb/store_hdcc.
Rust crate wiring. Add the optional rsmpeg dependency guarded behind
the enable_mp4_ffmpeg feature, with platform-specific feature flags
(link_system_ffmpeg on Linux + macOS, link_vcpkg_ffmpeg on Windows,
all using the ffmpeg7 bindings). Extend bindgen's wrapper.h to
expose the bridge headers so Rust can call back into ccx from
mp4_rust_bridge.c.
* feat(mp4): FFmpeg MP4 demuxer with GPAC-parity caption extraction
Alternative MP4 demuxing backend built on rsmpeg (Rust FFmpeg bindings)
that matches GPAC's caption output on every sample reviewed. Activated
via -DWITH_FFMPEG=ON at compile time; default build keeps the GPAC
path untouched.
Architecture
- src/rust/src/demuxer/mp4.rs: opens the MP4 with rsmpeg, classifies
tracks (AVC, HEVC, c608, c708, tx3g), and drives packet dispatch.
- src/rust/src/mp4_ffmpeg_exports.rs: #[no_mangle] entry points
(ccxr_processmp4, ccxr_dumpchapters) called from ccextractor.c.
- src/lib_ccx/mp4_rust_bridge.c/.h: thin C shim around do_NAL,
process608, process_cc_data, ccdp_find_data, store_hdcc, and
encode_sub so the Rust side can feed decoded payloads into
CCExtractor's existing CEA-608/708 pipeline.
Caption parity
GPAC-equivalent output on all six samples from the Apr 18 review:
- 132d7df7e993.mov 108 290 B byte-identical
- 1974a299f050.mov 127 828 B byte-identical
- 99e5eaafdc55.mov 164 099 B byte-identical
- 8849331ddae9.mp4 48 485 B identical size, content, caption
count; uniform ~2 ms timing shift
- b2771c84c2a3.mp4 2 607 B byte-identical
- 5df914ce773d.mp4 1 164 B byte-identical
SEI-embedded CEA-608 in H.264 video
The last caption finishing on the final sample was never encoded
because the interleaved av_read_frame loop exited without an
equivalent of GPAC's per-track encode_sub. Drain sub.got_output at
EOF. Intentionally avoid calling process_hdcc there: the last IDR's
slice_header already flushed the HD-CC buffer, and re-running
process_hdcc re-emits partial post-IDR caption state as trailing
garbage.
c608 payload handling
libavformat delivers c608/c708 samples in two shapes:
1) atom-wrapped raw 608 pairs — [u32 length][4cc cdat|cdt2|ccdp]
[payload]. Strip the 8-byte header to match GPAC's process_clcp.
2) bare cc_data triplets — [cc_info][b1][b2]. Detect via len % 3 == 0
and (payload[0] & 0xF8) == 0xF8. For c608 tracks, extract each
field-1/field-2 pair from the triplet, set dec_ctx->current_field
so process608 picks the right decoder context, and call
process608 directly. Routing through do_cb would hit its
CCX_H264 guard (set by interleaved H.264 packets) and suppress
the cb_field increments process608 relies on for
caption-boundary timing, which merged short captions into their
successors. For c708, keep ccdp_find_data + process_cc_data.
Bridge design
Single unified entry point ccx_mp4_process_nal_sample(..., int
is_hevc, ...) replaces the separate AVC/HEVC helpers — their NAL
iteration was ~90% identical. Uses utility.h's RB16/RB32 macros
instead of hand-rolled byte-swap helpers. Handles HEVC's
end-of-sample cc_data flush inline.
Supported and unsupported tracks are documented in the mp4.rs
module header. Known limitation: dvdsub/bitmap subtitles in MP4
are not decoded (neither GPAC nor this backend handles them).
* chore(rust): fix clippy 1.95 warnings blocking CI
The format_rust CI job runs `cargo clippy --lib -- -D warnings` and
rust-1.95.0 promoted a handful of lints that hadn't been tripped on
master before this branch went through CI. Address all six:
collapsible_match (4 sites):
- src/demuxer/demux.rs: fold the nested `if matches!(...)` inside
the `Some(false) =>` arm into a match guard on the arm itself.
- src/encoder/common.rs: three header-write arms (Ccd, Scc, Raw)
each had a nested `if write_raw(...) == -1 { return -1; }`.
Collapse each into a match guard; the body now just logs and
returns.
unnecessary_cast (2 sites):
- src/libccxr_exports/demuxer.rs: drop `as i64` from
demux_ctx.get_filesize() — it already returns i64.
- src/parser.rs: drop `as u64` from `t as u64` when writing to
UTC_REFVALUE — t is already u64.
Also two lints in the new MP4 demuxer file from this branch:
- if_same_then_else at src/demuxer/mp4.rs:198 — the FOURCC_TX3G
and AV_CODEC_ID_MOV_TEXT arms both produced TrackType::Tx3g;
same for FOURCC_C608 and AV_CODEC_ID_EIA_608 → TrackType::Cea608.
Fold each pair into a single `||` branch.
- manual_is_multiple_of at src/demuxer/mp4.rs:399 —
`packet_count % 100 == 0` → `packet_count.is_multiple_of(100)`.
No behavior change. Caption parity against GPAC on all six mentor
samples verified post-fix.
---------
Co-authored-by: GAURAV KARMAKAR <gaurav02081@users.noreply.github.com>
When building with Docker on an Apple Silicon Mac (M4), the Rust build
fails with type mismatch errors in common.rs and ctorust.rs.
The lang field in CapInfo is hardcoded as [i8; 4], which only matches the
bindgen-generated c_char on x86 (where c_char = i8). On ARM platforms like
Apple Silicon, c_char = u8, so the types don't match and compilation fails
with:
error[E0308]: mismatched types - expected `[u8; 4]`, found `[i8; 4]`...
Change to [c_char; 4] which resolves to the correct type on both architectures.
* fix: move C0 bounds check before match, improve C1 warn message
- C0 handler: bounds check was after the match, meaning process_p16(&block[1..])
could panic with an index out of bounds before the guard ran. Moved the check
before the match to prevent this.
- C1 handler: improved the warn message to include command code, name, and
lengths for easier debugging.
Fixes#1407
* docs: update CHANGES.TXT for #1407 fix
* style: cargo fmt
---------
Co-authored-by: Dhanush Varma <your@email.com>
* fix(build): decouple WITH_FFMPEG from ENABLE_HARDSUBX, fix FFmpeg 5.0+ API
Since 2017 (commit 4f5f564b), WITH_FFMPEG=ON automatically defined
ENABLE_HARDSUBX. In 2018 (PR #966), WITH_HARDSUBX was added as a
separate option but the old coupling was never removed. This made
-DWITH_FFMPEG=ON fail to build because it compiled hardsubx code
without linking tesseract/leptonica.
Fixes:
- Remove ENABLE_HARDSUBX from both WITH_FFMPEG blocks (src/CMakeLists.txt
and src/lib_ccx/CMakeLists.txt). ENABLE_HARDSUBX is now only defined
by the WITH_HARDSUBX block, where it belongs.
- WITH_HARDSUBX now auto-enables WITH_OCR and WITH_FFMPEG since it
requires both. Previously -DWITH_HARDSUBX=ON without -DWITH_OCR=ON
also failed to link.
- Fix const AVCodec* in ffmpeg_intgr.c for FFmpeg 5.0+ API (the
av_find_best_stream signature changed in libavcodec 59).
Tested all 7 build combinations:
1. Default OK (was OK)
2. WITH_FFMPEG OK (was BROKEN)
3. WITH_HARDSUBX + WITH_OCR OK (was OK)
4. WITH_FFMPEG + WITH_HARDSUBX + OCR OK (was BROKEN)
5. WITH_FFMPEG + WITH_OCR OK (was BROKEN)
6. WITH_OCR OK (was OK)
7. WITH_HARDSUBX only OK (was BROKEN)
* fix: MKV subtitle track .(null) extension for KATE and unknown codec IDs
The matroska_track_text_subtitle_id_extensions array had 7 entries for
an 8-value enum, leaving MATROSKA_TRACK_SUBTITLE_CODEC_ID_KATE (index 7)
out of bounds. On most platforms this read NULL, which then caused
strlen(NULL) UB and snprintf to emit .(null) in the output filename.
Two fixes:
- Add "kate" at index 7 in the extensions array so KATE tracks
produce correct .kate output filenames
- Add a NULL guard in generate_filename_from_track() so any future
unknown codec ID safely falls back to .bin instead of crashing or
producing .(null)
Fixes#972
* fix: MKV subtitle track .(null) extension for KATE and unknown codec IDs
The matroska_track_text_subtitle_id_extensions array had 7 entries for
an 8-value enum, leaving MATROSKA_TRACK_SUBTITLE_CODEC_ID_KATE (index 7)
out of bounds. On most platforms this read NULL, which then caused
strlen(NULL) UB and snprintf to emit .(null) in the output filename.
Two fixes:
- Add "kate" at index 7 in the extensions array so KATE tracks
produce correct .kate output filenames
- Add a NULL guard in generate_filename_from_track() so any future
unknown codec ID safely falls back to .bin instead of crashing or
producing .(null)
Fixes#972
* fix: MKV subtitle track .(null) extension for KATE and unknown codec IDs
The matroska_track_text_subtitle_id_extensions array had 7 entries for
an 8-value enum, leaving MATROSKA_TRACK_SUBTITLE_CODEC_ID_KATE (index 7)
out of bounds. On most platforms this read NULL, which then caused
strlen(NULL) UB and snprintf to emit .(null) in the output filename.
Two fixes:
- Add "kate" at index 7 in the extensions array so KATE tracks
produce correct .kate output filenames
- Add a NULL guard in generate_filename_from_track() so any future
unknown codec ID safely falls back to .bin instead of crashing or
producing .(null)
Fixes#972
---------
Co-authored-by: Dhanush Varma <your@email.com>
cc_count * 3 used i32 arithmetic with no upper-bound check. For cc_count
> i32::MAX / 3 (~715 million), debug builds panic on overflow detection
and release builds silently wrap around to a negative range, discarding
all CC data for the frame. Both are triggerable from malformed media files.
Fix:
1. Cast cc_count to usize immediately after the existing <= 0 guard,
before any arithmetic — eliminates the overflow entirely
2. Add MAX_CC_COUNT = 31 upper-bound guard — CEA-708/ATSC A/53 encodes
cc_count in a 5-bit bitstream field (0x1F mask in avc_functions.c:514),
making 31 the spec-defined per-frame maximum; this value is also
independently documented in es/userdata.rs ("Maximum cc_count is 31").
Returns -1 with a warn!() log for out-of-range values, consistent with
existing error-handling style in the function.
3. Remove the now-redundant `x as usize` cast in the map closure since
the range is already usize..usize
Fixes#2234
show_timestamp.to_srt_time().expect() and hide_timestamp.to_srt_time().expect()
in TeletextContext::process_page() panicked for any negative Timestamp value.
Negative timestamps are common in broadcast captures with wrap-around or
uninitialized PTS — crashing after potentially processing an entire file.
to_srt_time() → as_hms_millis() → i64::try_into::<u64>() returns
OutOfRangeError for negative values; .expect() made this fatal.
Fix: process_page() already returns Option<Subtitle>, so replace both
.expect() calls with .ok()? — silently skipping the subtitle when the
timestamp is out of range, matching the function's existing None-on-empty
contract.
Fixes#2233
