decoder: one access unit per MediaCodec input buffer by vertexodessa · Pull Request #142 · OpenIPC/PixelPilot

vertexodessa · 2026-10-31T14:09:18Z

VideoDecoder queued every NAL as its own input buffer. When a frame is encoded as several slices, that's several NALs per frame, and some decoders then show only the first slice. On a Pixel 7 Pro you get the top strip of the picture and black below.

NALs now go into one buffer per frame. The buffer is queued on the RTP marker (last packet of the frame), so there's no added delay. For a stream without markers it's queued on the next frame's first slice. A buffer with only parameter sets is queued as CODEC_CONFIG.

Single-slice streams aren't affected: one NAL per frame was already one frame per buffer.

Tested on a Pixel 7 Pro with an x265 test stream (1280x720@60) sent over RTP to port 5600:

	1 slice	4 slices
master	full frame	top strip only
this PR	full frame	full frame

qodo-free-for-open-source-projects · 2026-10-31T14:10:21Z

PR Summary by Qodo

Queue one access unit per MediaCodec input buffer

🐞 Bug fix 🕐 20-40 Minutes

AI Description

• Group frame NALs into one MediaCodec input buffer to prevent partial-frame rendering.
• Flush on RTP markers, or at the next first slice when markers are absent.
• Flag parameter-set-only buffers as codec configuration and discard stale buffered units.

Diagram

graph TD
  RTP["RTP packets"] --> Parser["H26X parser"] --> Player["VideoPlayer"] --> Buffer["AU accumulator"] --> End{"AU boundary?"} --> Kind{"VCL present?"} -->|"yes"| Codec["MediaCodec input"]
  End -->|"no: keep gathering"| Buffer
  Kind -->|"no: CODEC_CONFIG"| Codec

High-Level Assessment

The following are alternative approaches to this PR:

1. Assemble access units in the parser

➕ Keep access-unit framing upstream of the decoder.
➖ Broaden the parser's responsibility and affect consumers that currently receive individual NALs.

Recommendation: Keep assembly in VideoDecoder and expose only the marker boundary from the parser. This preserves existing per-NAL consumers while giving MediaCodec AU-aligned input; parser-side assembly would require a broader interface change.

Files changed (5) +123 / -64

Bug fix (5) +123 / -64

VideoDecoder.cppAccumulate and queue complete access units +98/-62

Accumulate and queue complete access units

• Replaces per-NAL MediaCodec input with a byte accumulator flushed by an RTP marker or a subsequent first slice. Queues parameter-set-only units with CODEC_CONFIG, clears stale buffered data when input is closed, and returns oversized dequeued buffers to MediaCodec.

app/videonative/src/main/cpp/VideoDecoder.cpp

VideoDecoder.hDeclare access-unit buffering and flush interface +15/-2

Declare access-unit buffering and flush interface
• Adds the public boundary notification, private queueing helpers, and state for accumulated NAL bytes, VCL presence, and the first NAL's arrival time.
app/videonative/src/main/cpp/VideoDecoder.h

VideoPlayer.cppConnect parser boundaries to the decoder +1/-0

Connect parser boundaries to the decoder
• Registers a parser callback that tells VideoDecoder to finish the current access unit.
app/videonative/src/main/cpp/VideoPlayer.cpp

H26XParser.cppReport RTP marker boundaries after parsing +4/-0

Report RTP marker boundaries after parsing
• Invokes the access-unit-end callback after parsing an RTP packet with its marker bit set, allowing the decoder to flush without waiting for another frame.
app/videonative/src/main/cpp/parser/H26XParser.cpp

H26XParser.hExpose access-unit-end callback +5/-0

Expose access-unit-end callback
• Declares a callback for notifying consumers when a marked RTP packet ends an access unit.
app/videonative/src/main/cpp/parser/H26XParser.h

qodo-free-for-open-source-projects · 2026-10-31T14:11:09Z

Code Review by Qodo

🐞 Bugs (5) 📘 Rule violations (1) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Malformed video packets can read past a buffer 🐞 Bug ⛨ Security

Description

interpretNALU() checks only that a NALU exceeds four bytes before reading the H.264 slice byte at
p[1] or the H.265 slice byte at p[2]. A short VCL NALU forwarded by the RTP parser can pass that
check, so a release build reads beyond its data while processing packet input.

Code

app/videonative/src/main/cpp/VideoDecoder.cpp[R119-120]

+            const uint8_t* p = nalu.getDataWithoutPrefix();
+            firstSlice       = (p[nalu.IS_H265_PACKET ? 2 : 1] & 0x80) != 0;

Evidence
The constructor's minimum-size assertion is not release-build validation. RTP forwarding accepts a
five-byte prefixed NALU, while the new slice-header read requires more bytes.
app/videonative/src/main/cpp/VideoDecoder.cpp[94-101]
app/videonative/src/main/cpp/VideoDecoder.cpp[111-120]
app/videonative/src/main/cpp/NALU/NALU.hpp[36-42]
app/videonative/src/main/cpp/parser/ParseRTP.cpp[365-378]
app/videonative/src/main/cpp/parser/ParseRTP.cpp[433-443]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Slice-header detection reads bytes that a short parsed NALU may not contain.
## Fix Focus Areas
- app/videonative/src/main/cpp/VideoDecoder.cpp[111-121]
- app/videonative/src/main/cpp/parser/ParseRTP.cpp[365-378]
## Recommended Fix
Before reading a VCL slice-header byte, require at least two bytes after the prefix for H.264 or three for H.265. Reject short NALUs at the parser boundary as well; do not rely on assertions for packet validation.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. Markerless packets can exhaust app memory 🐞 Bug ⛨ Security

Description

interpretNALU() appends every accepted NALU to mAuBuffer without limiting the accumulated access
unit. Repeated continuation slices with unset RTP markers trigger neither flush condition, allowing
packet input to grow the vector until memory is exhausted.

Code

app/videonative/src/main/cpp/VideoDecoder.cpp[130]
+        mAuBuffer.insert(mAuBuffer.end(), nalu.getData(), nalu.getData() + nalu.getSize());

Evidence
The parser forwards completed NALUs individually, but the new vector has no aggregate limit. A
continuation slice does not satisfy the first-slice flush, and only a marked RTP packet invokes the
other flush.
app/videonative/src/main/cpp/VideoDecoder.cpp[111-131]
app/videonative/src/main/cpp/VideoDecoder.h[132-136]
app/videonative/src/main/cpp/parser/H26XParser.cpp[29-45]
app/videonative/src/main/cpp/VideoPlayer.cpp[110-143]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
An indefinite sequence of NALUs without a detected boundary grows the decoder's access-unit buffer without limit.
## Fix Focus Areas
- app/videonative/src/main/cpp/VideoDecoder.cpp[122-131]
- app/videonative/src/main/cpp/VideoDecoder.h[132-136]
## Recommended Fix
Enforce a maximum total access-unit size before appending. On overflow, discard the incomplete unit and reset its boundary state, then resume at a known new access-unit boundary.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. New decoder controls break formatting 📘 Rule violation ⚙ Maintainability

Description

VideoDecoder.cpp adds single-line if statements at the access-unit buffer and codec-input checks
instead of using the repository’s required braced control-statement layout. Running the prescribed
formatter on these changed lines rewrites the new decoder code, affecting the PR’s native-source
format check.

Code

app/videonative/src/main/cpp/VideoDecoder.cpp[131]
+        if (isVcl) mAuHasVcl = true;

Evidence
The changed file contains single-line control statements at the cited lines, while the
project-specific clang-format configuration requires AfterControlStatement: Always. The repository
formatter script applies clang-format to project C++ sources.
CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources: CLAUDE.md: Format Project C and C++ Sources
app/videonative/src/main/cpp/VideoDecoder.cpp[131-131]
app/videonative/src/main/cpp/VideoDecoder.cpp[164-164]
app/videonative/src/main/cpp/VideoDecoder.cpp[176-176]
app/videonative/src/main/cpp/.clang-format[8-15]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The added decoder code contains single-line `if` statements, but the local clang-format configuration requires braces and multiline control statements.
## Fix Focus Areas
- app/videonative/src/main/cpp/VideoDecoder.cpp[131-131]
- app/videonative/src/main/cpp/VideoDecoder.cpp[164-164]
- app/videonative/src/main/cpp/VideoDecoder.cpp[176-176]
## Recommended Fix
Rewrite each affected `if` statement with braces and the repository’s multiline layout, then run the prescribed clang-format version over the changed native files.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

4. A restarted surface can receive old video 🐞 Bug ≡ Correctness

Description

setOutputSurface() tears down a codec without clearing the newly persistent mAuBuffer or
mAuHasVcl. If a surface is removed after a partial frame and reinstalled before another NALU
arrives, the closed-pipe cleanup never runs and the old bytes can be queued with data from the new
session.

Code

app/videonative/src/main/cpp/VideoDecoder.cpp[R101-102]
+        mAuBuffer.clear();
+        mAuHasVcl = false;

Evidence
Teardown does not reset the new accumulator; its existing cleanup runs only in the closed-pipe
packet path. Reinstalling a surface reopens that path before the next packet.
app/videonative/src/main/cpp/VideoDecoder.cpp[25-67]
app/videonative/src/main/cpp/VideoDecoder.cpp[94-105]
app/videonative/src/main/cpp/VideoDecoder.cpp[122-130]
app/videonative/src/main/cpp/VideoDecoder.cpp[162-171]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The access-unit accumulator survives surface teardown when no packet arrives during the closed interval.
## Fix Focus Areas
- app/videonative/src/main/cpp/VideoDecoder.cpp[25-57]
- app/videonative/src/main/cpp/VideoDecoder.cpp[98-102]
## Recommended Fix
Clear the accumulator and VCL state while holding the input-pipe mutex when closing a surface, rather than relying on a later packet or marker to perform cleanup.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

5. Invalid packets can cut a frame short 🐞 Bug ≡ Correctness

Description

parse_rtp_stream() invokes onAccessUnitEnd() for any marked RTP packet, even when its payload
type is unsupported or video parsing rejected its payload. Because the player sends all non-audio
packets to this parser, such a packet can flush a previously accumulated frame without contributing
a NALU.

Code

app/videonative/src/main/cpp/parser/H26XParser.cpp[R42-45]

+    if (rtpPacket.header.marker && onAccessUnitEnd)
+    {
+        onAccessUnitEnd();
+    }

Evidence
Payload selection guards the parsing calls but not the new callback. The caller routes unknown
non-audio payloads here, and the decoder flushes whatever it has buffered on that callback.
app/videonative/src/main/cpp/VideoPlayer.cpp[128-143]
app/videonative/src/main/cpp/parser/H26XParser.cpp[29-45]
app/videonative/src/main/cpp/parser/ParseRTP.cpp[115-124]
app/videonative/src/main/cpp/VideoDecoder.cpp[148-171]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A marker on an unrecognized or rejected packet currently ends the decoder's pending access unit.
## Fix Focus Areas
- app/videonative/src/main/cpp/parser/H26XParser.cpp[29-45]
- app/videonative/src/main/cpp/VideoPlayer.cpp[128-143]
## Recommended Fix
Limit the marker callback to recognized video packets whose parsing established a valid access-unit end. Do not let an unsupported or rejected packet flush pending decoded data.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

View review recommended (1)

6. Partitioned slices can split a frame 🐞 Bug ≡ Correctness

Description

interpretNALU() treats H.264 partition B and C NALUs as though their first payload bit encoded
first_mb_in_slice. Those partition types do not carry the ordinary slice header at that position,
so a set bit can flush the accumulated access unit between partitions of the same picture.

Code

app/videonative/src/main/cpp/VideoDecoder.cpp[R112-120]

+        const bool isVcl = nalu.IS_H265_PACKET ? (type < 32) : (type >= 1 && type <= 5);
+        bool firstSlice  = false;
+        if (isVcl)
+        {
+            // first_slice_segment_in_pic_flag (H.265) / first_mb_in_slice == 0
+            // (H.264, ue(v) with a leading 1 bit): the MSB of the first
+            // slice-header byte.
+            const uint8_t* p = nalu.getDataWithoutPrefix();
+            firstSlice       = (p[nalu.IS_H265_PACKET ? 2 : 1] & 0x80) != 0;

Evidence
The new condition includes H.264 types 3 and 4 in its first-slice test, while the repository
identifies those types as slice data partitions B and C rather than ordinary coded slices.
app/videonative/src/main/cpp/VideoDecoder.cpp[111-125]
app/videonative/src/main/cpp/NALU/NALUnitType.hpp[19-25]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The H.264 first-slice test applies ordinary slice-header syntax to partition B and C payloads.
## Fix Focus Areas
- app/videonative/src/main/cpp/VideoDecoder.cpp[111-125]
- app/videonative/src/main/cpp/NALU/NALUnitType.hpp[19-25]
## Recommended Fix
Apply the first_mb_in_slice boundary test only to H.264 NALU types that carry that field. Keep partition B and C data attached to the current picture rather than interpreting their payload bits as a new-picture signal.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Tip of the day

💡 Did you know, you can keep summaries lean with Findings visible per group, which tucks the rest behind a View link

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

VideoDecoder queued every NAL as its own input buffer with its own timestamp. A frame encoded as several slices is several NALs, and some decoders then show only the first slice of each frame. NALs now gather into one buffer per access unit, flushed on the RTP marker (set on the last packet of each frame) or, for a stream without markers, on the next frame's first slice. A buffer with only parameter sets is queued as CODEC_CONFIG. A single-slice stream is unchanged: one NAL per frame was already one access unit per buffer.

The RTP marker flushed the gathered access unit for any H.264/H.265 packet, including ones the RTP decoder then dropped: too short, empty, a duplicate sequence number, an unsupported NAL type. A late copy of the previous frame's last packet could also flush the current frame half-built. parseRTPH264toNALU/parseRTPH265toNALU now say whether they took the packet, and the marker counts only for a taken packet whose sequence number is newer than any seen so far. A jump of more than 512 behind is taken as a restarted sender and resyncs, so a new stream's markers are not ignored until its numbers catch up.

qodo-free-for-open-source-projects Bot reviewed Oct 31, 2026

View reviewed changes

vertexodessa force-pushed the decoder/au-per-buffer branch from dd66474 to 42fe1c5 Compare October 31, 2026 14:17

vertexodessa force-pushed the decoder/au-per-buffer branch from 42fe1c5 to b7861a0 Compare October 31, 2026 16:38

vertexodessa merged commit 8141548 into OpenIPC:master Oct 31, 2026
4 checks passed

vertexodessa deleted the decoder/au-per-buffer branch October 31, 2026 17:57

Navigation Menu

Repository navigation

decoder: one access unit per MediaCodec input buffer - #142

decoder: one access unit per MediaCodec input buffer#142
vertexodessa merged 2 commits into
OpenIPC:masterfrom
vertexodessa:decoder/au-per-buffer

vertexodessa commented Oct 31, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Oct 31, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Oct 31, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

vertexodessa commented Oct 31, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Oct 31, 2026

PR Summary by Qodo

Uh oh!

qodo-free-for-open-source-projects Bot commented Oct 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

qodo-free-for-open-source-projects Bot commented Oct 31, 2026 •

edited

Loading