Skip to content

[FIX] Warn when non-DVB caption PIDs are silently ignored - #2334

Open
kaihere14 wants to merge 2 commits into
CCExtractor:masterfrom
kaihere14:fix/2333-warn-ignored-caption-pids
Open

[FIX] Warn when non-DVB caption PIDs are silently ignored#2334
kaihere14 wants to merge 2 commits into
CCExtractor:masterfrom
kaihere14:fix/2333-warn-ignored-caption-pids

Conversation

@kaihere14

@kaihere14 kaihere14 commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

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.
  • 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:

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • 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.
  • I am NOT adding new C code unless it's to fix an existing, reproducible bug.

@kaihere14
[FIX] Warn when non-DVB caption PIDs are silently ignored
e9cc695 
Closes CCExtractor#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 CCExtractor#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.
@kaihere14

kaihere14 commented Sep 12, 2026

Copy link
Copy Markdown
Contributor Author

Hey @cfsmp3 , PR for #2333 up whenever you get a sec

cfsmp3
cfsmp3 requested changes Sep 12, 2026
View reviewed changes

@cfsmp3 cfsmp3 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good write-up — particularly calling out that ISDB/ATSC are exercised by a constructed sample rather than real broadcast data, and that the video-PID false positive was caught and fixed before submitting. That is the part reviewers usually have to find themselves.

Three things from the repo's code-quality rules before this can go in:

Braces. All if/else/for/while bodies need { }, including single statements. Three places here:

	if (iter->stream == CCX_STREAM_TYPE_VIDEO_MPEG2 || … )
		return;

	if (!iter->ignore)
		warn_ignored_caption_stream(iter);   // twice

Format specifiers. pid and program_number are int (ccx_demuxer.h:55-56) but printed with %u and %x. Fine for real PID values, wrong for the declared type — please use %d, or cast explicitly if unsigned output is intended.

What I could verify: no over-warning. Across 59 local samples the warning never fires, every output is byte-identical to master, and exit codes match — so the video-PID exclusion is doing its job and nothing regressed.

What I could not: reach the warning at all. The only two local files with multiple dvb_teletext entries turn out to expose the same PID (0x12e) in a single program, so ccextractor sees one caption PID and nothing is discarded. The positive path therefore rests on your samples. If t3514.ts is small enough to add to the regression suite it would be worth doing — this is precisely the kind of behaviour that silently regresses otherwise.

One question on scope: ignore_other_sib_stream() gets the same treatment, but its callers iterate sibling streams of one program. Is it possible for both functions to warn about the same PID in one run, and if so is the message emitted twice?

— Claude on behalf of @cfsmp3

@kaihere14

kaihere14 commented Sep 13, 2026

Copy link
Copy Markdown
Contributor Author

Good write-up — particularly calling out that ISDB/ATSC are exercised by a constructed sample rather than real broadcast data, and that the video-PID false positive was caught and fixed before submitting. That is the part reviewers usually have to find themselves.

Three things from the repo's code-quality rules before this can go in:

Braces. All if/else/for/while bodies need { }, including single statements. Three places here:

	if (iter->stream == CCX_STREAM_TYPE_VIDEO_MPEG2 || … )
		return;

	if (!iter->ignore)
		warn_ignored_caption_stream(iter);   // twice

Format specifiers. pid and program_number are int (ccx_demuxer.h:55-56) but printed with %u and %x. Fine for real PID values, wrong for the declared type — please use %d, or cast explicitly if unsigned output is intended.

What I could verify: no over-warning. Across 59 local samples the warning never fires, every output is byte-identical to master, and exit codes match — so the video-PID exclusion is doing its job and nothing regressed.

What I could not: reach the warning at all. The only two local files with multiple dvb_teletext entries turn out to expose the same PID (0x12e) in a single program, so ccextractor sees one caption PID and nothing is discarded. The positive path therefore rests on your samples. If t3514.ts is small enough to add to the regression suite it would be worth doing — this is precisely the kind of behaviour that silently regresses otherwise.

One question on scope: ignore_other_sib_stream() gets the same treatment, but its callers iterate sibling streams of one program. Is it possible for both functions to warn about the same PID in one run, and if so is the message emitted twice?

— Claude on behalf of @cfsmp3

Thanks for the review. All three addressed in the follow-up commit:

Braces — added to the video-type early return in warn_ignored_caption_stream() and to both if (!iter->ignore) call sites in ignore_other_stream() / ignore_other_sib_stream().

Format specifiers — pid and program_number now print with %d. The hex forms keep %x with an explicit (unsigned int) cast, since hex output is the intent there (matches the existing VBI/teletext stream ID ... (0x...) lines the warning sits next to).

Regression sample — t3514.ts is a real broadcast capture from samples.ffmpeg.org; I'll check its size and, if it's reasonable, open a request to add it to the sample repo so the positive path is covered rather than resting on my local files. Agreed this is exactly the kind of thing that regresses silently.

On the scope question — no, the two functions can't both warn on the same PID in one run, for two reasons:

  1. The callers are on opposite branches of if (!ctx->multiprogram) in general_loop.c:1447. The non-multiprogram path calls ignore_other_stream() (general_loop.c:1151); the multiprogram path iterates pg_stream and calls ignore_other_sib_stream() (general_loop.c:1482). ctx->multiprogram comes from the CLI and doesn't change mid-run, so only one of the two functions ever executes per process.
  2. Within either function, the if (!iter->ignore) guard means a PID warns once even though the caller runs on every demuxer iteration. The only place ignore is reset to 0 is update_capinfo() (ts_info.c:274), and that branch only fires while the entry is still CCX_STREAM_TYPE_UNKNOWNSTREAM or CCX_CODEC_NONE. Once the PMT has typed the stream the condition is false and the flag stays set, so a repeated PMT can't re-arm it.

The one edge case is a PID first registered as CCX_CODEC_NONE (ts_tables.c:510) and later re-typed by a new PMT version: update_capinfo() resets ignore, and the next iteration warns. But the first pass would have hit default: return in the codec switch (codec was still NONE), so the user still sees exactly one warning.

@ccextractor-bot

ccextractor-bot commented Sep 13, 2026

Copy link
Copy Markdown
Collaborator
CCExtractor CI platform finished running the test files on linux. 171/237 tests matched the approved output:
Report Name Tests Passed
Broken 9/13
CEA-708 2/14
DVB 0/7
DVD 3/3
DVR-MS 2/2
General 25/27
Hardsubx 1/1
Hauppage 3/3
MP4 3/3
NoCC 10/10
Options 69/86
Teletext 0/21
WTV 12/13
XDS 32/34

66 tests do not match the approved output. That is the pass/fail verdict. Whether this branch caused it is a separate question, answered below.

Compared with the tip of master — test 9550, commit 2364994:

  • 0 pass there and fail here
  • 0 fail there and pass here
  • 0 fail on both, with different output
  • 66 fail on both, byte for byte the same

Compared with the commit this branch was cut from — test 9520, commit 2ad84df:

  • 0 pass there and fail here
  • 0 fail there and pass here
  • 0 fail on both, with different output
  • 66 fail on both, byte for byte the same

No test changes behaviour relative to the tip of master: every failure above fails there too, byte for byte. The approved output for those tests is out of date, which is a baseline to review rather than a regression in this branch.

@ccextractor-bot

ccextractor-bot commented Sep 13, 2026

Copy link
Copy Markdown
Collaborator
CCExtractor CI platform finished running the test files on windows. 171/237 tests matched the approved output:
Report Name Tests Passed
Broken 9/13
CEA-708 2/14
DVB 0/7
DVD 3/3
DVR-MS 2/2
General 25/27
Hardsubx 1/1
Hauppage 3/3
MP4 3/3
NoCC 10/10
Options 69/86
Teletext 0/21
WTV 12/13
XDS 32/34

66 tests do not match the approved output. That is the pass/fail verdict. Whether this branch caused it is a separate question, answered below.

Compared with the tip of master — test 9545, commit 683bb39:

  • 0 pass there and fail here
  • 0 fail there and pass here
  • 0 fail on both, with different output
  • 66 fail on both, byte for byte the same

Compared with the commit this branch was cut from — test 9521, commit 2ad84df:

  • 0 pass there and fail here
  • 0 fail there and pass here
  • 0 fail on both, with different output
  • 66 fail on both, byte for byte the same

No test changes behaviour relative to the tip of master: every failure above fails there too, byte for byte. The approved output for those tests is out of date, which is a baseline to review rather than a regression in this branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cfsmp3 cfsmp3 cfsmp3 requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] Non-DVB caption PIDs (Teletext/ISDB/ATSC) are silently dropped when multiple exist in one program

3 participants

@kaihere14 @ccextractor-bot @cfsmp3