Fix silent success return for unhandled DVB string-coded subtitle obj… - #2304
Open
GuTS805 wants to merge 3 commits into
Open
Fix silent success return for unhandled DVB string-coded subtitle obj…#2304GuTS805 wants to merge 3 commits into
GuTS805 wants to merge 3 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.
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. |
… 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.
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. |
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.
[FIX]
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 #2303
In raising this pull request, I confirm the following (please check boxes):
Reason for this PR:
Sanity check:
Repro instructions:
ccextractor dvb_string_coding_sample.ts -out=spupng -o /tmp/testFIXME support for string coding standardand finishes withNo captions were found in input.. the failure is completely silent; nothing signals that a caption object was actually dropped.Return from dvbsub_decode: -1— the same condition is now surfaced as a real, explicit failure instead of a masked success.dvb_string_coding_sample.ts(committed in this PR) is a minimal, spec-valid MPEG-TS stream — PAT → PMT with a DVBsubtitling_descriptor→ PES packets carrying a page/region/object-segment sequence where the object segment setsobject_coding_method = 1(ETSI EN 300 743 §7.2.5, "character coded" object) — built specifically to exercise this code path.scratch_dvb_sample/build_dvb_sample.pydocuments/reconstructs the sample byte-by-byte for anyone who wants to verify or regenerate it.This is a real, spec-conformant encoding some DVB broadcasters use to save bandwidth (as opposed to bitmap/pixel-coded objects, which CCExtractor already handles). Because the failure was previously silent, this is an accessibility-relevant data-loss bug — users relying on extracted captions would get nothing for the affected segment with no indication anything went wrong.