upipe_mpgv_framer: guard against NULL next_uref when flushing buffere…#1173
Draft
kierank wants to merge 3 commits into
Draft
upipe_mpgv_framer: guard against NULL next_uref when flushing buffere…#1173kierank wants to merge 3 commits into
kierank wants to merge 3 commits into
Conversation
…d frame Avoid handling a buffered frame when next_uref is NULL, both during work and on free, preventing a NULL dereference. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
This fixes a genuine crash |
… code When the framer is destroyed (or reaches the end of a complete input) while holding the start of a new frame that does not yet contain a picture start code -- e.g. a stream truncated right after a sequence or GOP header -- next_frame_offset is still its -1 sentinel. upipe_mpgvf_handle_frame() tested this with "if (next_frame_offset)", which is true for -1, and passed it as the size_t extraction length to the first upipe_mpgvf_extract_uref_stream() call. That (size_t)-1 length drained the entire uref stream, leaving next_uref == NULL, so the second extract_uref_stream() tripped assert(next_uref != NULL) and aborted. The commit adding the next_uref != NULL guards on the flush paths only covered next_uref being NULL on entry; here it is nulled out mid-flush. A frame with no picture header cannot be output, so bail out early (consume the buffered octets and return NULL), mirroring the existing missing-sequence-header case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
nto
requested changes
Jul 6, 2026
nto
left a comment
Collaborator
There was a problem hiding this comment.
- Please try to follow basic coding style conventions such as the 50/72 Rule for commit messages and max line length for code.
- The test cases should be added to the already existing
upipe_mpgv_framer_testexecutable. - Co-Authored-By trailers should be removed as Upipe's commit history is not the place for advertising proprietary products from AI companies.
Two regression cases for flushing a buffered frame whose next_frame_offset
is still its -1 sentinel (no picture start code seen yet) when the pipe is
destroyed. Such a frame used to be handed to upipe_mpgvf_extract_uref_stream()
with a (size_t)-1 length, draining the uref stream to NULL and tripping
assert(next_uref != NULL) in upipe_mpgvf_free(). They enter
upipe_mpgvf_handle_frame() through the two sides of its first guard:
- a complete I-frame followed by a lone GOP header, so the sequence header
is already stored and next_frame_sequence is false;
- only a sequence header (+ extension) truncated before any picture, so
next_frame_sequence is true and no sequence header is stored.
Both must run to completion without aborting, dropping the picture-less
buffered frame instead of extracting it. They use a count-and-drop sink so
they do not collide with the strict frame-validating sink used by the
existing cases.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
…d frame
Avoid handling a buffered frame when next_uref is NULL, both during work and on free, preventing a NULL dereference.