Improve diagnostic message for unsupported DVB string-coded (character-coded) subtitle objects - #2313
Open
GuTS805 wants to merge 4 commits into
Open
Improve diagnostic message for unsupported DVB string-coded (character-coded) subtitle objects#2313GuTS805 wants to merge 4 commits into
GuTS805 wants to merge 4 commits into
Conversation
…ects dvbsub_parse_object_segment() previously returned 0 (success) when encountering a character-coded (coding_method == 1) DVB subtitle object, silently dropping the caption content. Now returns -1 so the caller (dvbsub_decode) correctly treats this as a failure. Fixes CCExtractor#2303
dvb_string_coding_sample.ts is a minimal MPEG-TS file (PAT+PMT+DVB subtitle PES) whose object segment uses object_coding_method=1 (character-coded), triggering the previously-silent failure fixed in the previous commit. build_dvb_sample.py documents how it was constructed for future reference. Repro: ccextractor dvb_string_coding_sample.ts -out=spupng -o /tmp/test Before fix: prints 'FIXME support for string coding standard' and reports 'No captions were found' with no error. After fix: prints 'Return from dvbsub_decode: -1', a real, surfaced failure.
… message CI caught a regression: returning -1 aborted dvbsub_decode()'s entire segment loop for the packet, dropping other valid objects/regions that happened to share the same packet as an unsupported string-coded one. Restored the original safe return value; only the diagnostic message changed to make the failure unambiguous instead of a bare FIXME.
These don't belong in the main source tree per maintainer feedback - sample-based testing goes through the dedicated sample platform, not committed files. Repro steps are documented in the PR description instead.
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 6077cf5...:
Your PR breaks these cases:
NOTE: The following tests have been failing on the master branch as well as the PR:
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 c328108...:
Your PR breaks these cases:
NOTE: The following tests have been failing on the master branch as well as the PR:
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
2 participants
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.
[IMPROVEMENT]
dvbsub_parse_object_segment()(src/lib_ccx/dvb_subtitle_decoder.c) hits an unimplemented branch when a DVB subtitle object segment usesobject_coding_method == 1(character-coded / "string coding", ETSI EN 300 743 §7.2.5) instead of the supported bitmap encoding. Previously this only printed a bare"FIXME support for string coding standard"with no further context. This PR replaces that with a clear, actionable message explaining what happened and that it's non-fatal (other objects/regions in the same packet are still processed normally).The return value is intentionally left unchanged (
return 0) — an earlier version of this PR changed it toreturn -1, but that causeddvbsub_decode()'s segment loop to abort the entire packet, which broke other valid objects/regions sharing that packet in CI (sampleplatformregression test 21). This version keeps behavior identical to master, changing only the diagnostic text.Fixes #2303
In raising this pull request, I confirm the following (please check boxes):
Reason for this PR:
Sanity check:
Repro instructions:
subtitling_descriptor(tag0x59) → PES packets carrying a page/region/object-segment sequence where the object segment setsobject_coding_method = 1. Sample file attached below (not committed to the repo, per earlier feedback — this is just an attachment for reviewers).ccextractor <sample>.ts -out=spupng -o /tmp/testFIXME support for string coding standardwith no further context.Note on CI:
format_rust (./src/rust)fails on this PR, but this is a pre-existing, unrelated clippy lint failure insrc/demuxer/stream_functions.rs(a file this PR never touches) — the same failure currently appears on other unrelated open PRs (e.g. #2301, #2302), so it isn't caused by this change.