Fix: DVR recording leaks one heap buffer per frame

[iflyhere]

The bug

VideoPlayer::onNewNALU() hands every NALU to the DVR writer thread by copying
it onto the heap:

uint8_t* m_data_copy = new uint8_t[nalu.getSize()];
memcpy(m_data_copy, nalu.getData(), nalu.getSize());
NALU nalu_(m_data_copy, nalu.getSize(), nalu.IS_H265_PACKET);
enqueueNALU(nalu_);

NALU is a non-owning view. From NALU.hpp:

NOTE: NALU only takes a c-style data pointer - it does not do any memory
management. Use NALUBuffer if you need to store a NALU.

Its destructor is = default, so naluQueue.pop() in processQueue() frees
nothing. There is no matching delete[] anywhere.

Every recorded frame leaks its full buffer, so the leak rate is the video
bitrate: about 1 MB/s at 8 Mbit/s. A ten minute recording leaks a few hundred MB
and the process eventually gets killed by the OOM killer — while recording, i.e.
while flying. Anything still queued when recording stops leaks as well.

The fix

The queue holds an owning value instead:

struct DvrNalu
{
    std::vector<uint8_t> data;
    bool                 is_h265 = false;
};
std::queue<DvrNalu> naluQueue;

enqueueNALU() takes it by rvalue reference and moves it in, processQueue()
moves it back out. The bytes are freed with the queue entry, and whatever is
still queued is released when the queue is destroyed.

One small restructure in processQueue(): the H.265 flag of the front element is
needed for mp4_h26x_write_init() before the element is consumed, and the
existing continue path (waiting for currentFPS > 0) must not consume it. The
flag is therefore read from naluQueue.front() and the element is only moved out
after that.

Noticed while in there, not fixed here

processQueue() calls MP4E_close(mux) before mp4_h26x_write_close(&mp4wr),
and mp4wr is uninitialised if the thread stops before the first frame was
written (framerate == 0), so that close reads uninitialised stack. Left out to
keep this PR to the leak — happy to add it here or open a separate one.

---

Part of a small series of independent fixes found while profiling the receive path.
Each one is standalone and mergeable on its own, in any order — no dependencies
between them, and no shared files except VideoActivity.java / VideoPlayer.*,
which touch different methods:

• Add a low latency decoder option and actually apply the MediaCodec keys #113 — apply the MediaCodec low-latency keys (behind a new switch)
• Render the main video on a SurfaceView unless object detection needs the TextureView #114 — SurfaceView for the main video unless object detection needs the TextureView
• Keep the activity alive across configuration changes #117 — keep the activity alive across configuration changes
• Fix: DVR recording leaks one heap buffer per frame #115 — DVR memory leak
• Harden the USB adapter lifecycle #116 — USB adapter lifecycle

All five compile clean for arm64-v8a + armeabi-v7a.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Prevent DVR per-frame NALU buffer leak

🐞 Bug fix 🕐 10-20 Minutes

AI Description

• Replaces non-owning queued NALUs with move-managed owned byte buffers.
• Releases frame memory automatically after DVR writes or queue destruction.
• Preserves codec initialization without consuming frames while waiting for valid FPS.

Diagram

sequenceDiagram
    participant P as H26X Parser
    participant V as VideoPlayer
    participant Q as DVR Queue
    participant W as Writer Thread
    participant M as MP4 Writer
    P->>V: Parsed NALU view
    V->>V: Copy owned bytes
    V->>Q: Move DvrNalu
    W->>Q: Inspect codec flag
    W->>Q: Move front entry
    Q-->>W: Owned frame
    W->>M: Write frame bytes
    W->>W: Release vector

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Queue existing NALUBuffer values

• ➕ Reuses the repository's documented owning NALU abstraction.
• ➕ Keeps NALU metadata and byte lifetime encapsulated together.
• ➖ NALUBuffer is explicitly non-copyable and non-movable, requiring pointer-based queue entries or broader redesign.
• ➖ Its shared_ptr and unique_ptr members add unnecessary indirection for sequential DVR ownership.

2. Use smart-pointer byte arrays

• ➕ Minimally changes the existing NALU-based writer flow.
• ➕ Can make ownership explicit with unique_ptr storage.
• ➖ Requires separately tracking size and codec metadata.
• ➖ Is less idiomatic and less self-describing than vector-backed value ownership.

Recommendation: Keep the lightweight DvrNalu value used by this PR: it directly models the queue's required bytes and codec flag, supports efficient moves, and guarantees deterministic cleanup. A future repository-wide redesign could make NALUBuffer movable, but expanding that abstraction is unnecessary for this focused leak fix.

Files changed (2) +22 / -13

Bug fix (2) +22 / -13

VideoPlayer.cppTransfer owned NALU bytes through the DVR writer queue +8/-10

Transfer owned NALU bytes through the DVR writer queue

• Copies parser-backed NALU bytes into DvrNalu vector storage and moves each queued entry into the writer before popping it. Reads the H.265 flag before consumption so FPS-gated initialization can continue without discarding the pending frame.

app/videonative/src/main/cpp/VideoPlayer.cpp

VideoPlayer.hDefine an owning DVR queue entry type +14/-3

Define an owning DVR queue entry type

• Adds DvrNalu with vector-owned bytes and codec metadata, replaces the non-owning NALU queue, and moves entries into the synchronized queue. This ties buffer cleanup to normal value destruction.

app/videonative/src/main/cpp/VideoPlayer.h

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can ask Qodo to dismiss a finding you disagree with, with your reason on record

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗