Skip to content

upipe_mpgv_framer: guard against NULL next_uref when flushing buffere…#1173

Draft
kierank wants to merge 3 commits into
Upipe:masterfrom
kierank:mp2v
Draft

upipe_mpgv_framer: guard against NULL next_uref when flushing buffere…#1173
kierank wants to merge 3 commits into
Upipe:masterfrom
kierank:mp2v

Conversation

@kierank

@kierank kierank commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

…d frame

Avoid handling a buffered frame when next_uref is NULL, both during work and on free, preventing a NULL dereference.

…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>
@kierank

kierank commented Jun 12, 2026

Copy link
Copy Markdown
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 nto left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

  • 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_test executable.
  • Co-Authored-By trailers should be removed as Upipe's commit history is not the place for advertising proprietary products from AI companies.

@kierank kierank marked this pull request as draft July 6, 2026 14:22
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants