Two packet/buffer lifecycle fixes (encoder persistent packet, decoder input packet ownership)

[wangjiamingg]

Two packet/buffer lifecycle fixes (encoder persistent packet, decoder input packet ownership)

Hi,

While chasing a ~1.3 GB/day memory leak in a long-running kiosk product
(RK3588, GStreamer + MPP decode/encode pipelines restarted every few
seconds), we found two lifecycle issues in MPP core. Both are fixed
locally and verified in production for several days.

Patch 1: mpp_enc_impl — stale buffer ref in persistent encoder packet

mpp_enc_check_pkt_buf() assigns pkt->buffer = buffer directly. For
the sync path, enc->packet is a persistent packet reused across
frames, so this raw assignment drops the previous frame's MppBuffer
reference every frame → one leaked MppBuffer per encoded frame.

Fix: use mpp_packet_set_buffer(), which puts the previously attached
buffer before attaching the new one. Teardown behaviour is unchanged.

Repro: any long-running encode (h264/h265) with frequent start/stop;
buffer-group usage grows linearly before, flat after.

Patch 2: mpp_dec_normal — single-owner transfer of input packet to frame meta

When the decoded output frame carries KEY_INPUT_PACKET so that the
consumer can release the bitstream buffer, dec->mpp_pkt_in still owns
the same packet. dec_release_input_packet() can then deinit it a
second time after the consumer already did ("found non-positive
ref_count" on the packet's MppBuffer, then crash).

MppPacket has no inc_ref API and MppMeta set/get copies raw pointers
without touching refcounts, so the only safe hand-off is a single-owner
transfer: clear dec->mpp_pkt_in right after attaching the packet to
the output frame meta.

Repro: decode where the consumer reads KEY_INPUT_PACKET back from the
output frame meta and deinits it (e.g. gstreamer-rockchip drain path);
per-packet alloc/free accounting shows double deinit before, balanced
after.

Both fixes are independent of each other; happy to split/rework as
preferred.

[FumasterLin]

Thanks for digging into the packet lifecycle! After a careful review against the current code, we don't think these two issues
exist. Details below.

1. mpp_enc_impl: the buffer leak does not exist, and the fix would introduce one

The direct assignment pkt->buffer = buffer in mpp_enc_check_pkt_buf() only runs when enc->pkt_buf == NULL. This invariant guarantees the packet never holds an old buffer at that point:

• mpp_enc_check_frm_pkt() re-derives enc->pkt_buf from mpp_packet_get_buffer(enc->packet) at the beginning of every task, so the two are always in sync;
• reset_enc_task() clears enc->packet after every terminated task, so each frame starts with a fresh packet from mpp_packet_new().
The "persistent packet reused across frames" scenario described in the commit message cannot be constructed in this code.

More importantly, the proposed change is a leak in the opposite direction: mpp_packet_set_buffer() calls mpp_buffer_inc_ref() on the new buffer, but the reference returned by mpp_buffer_get() is never put. With the patch applied, every newly created output buffer would gain a net +1 reference and never return to the buffer group — which produces exactly the linear RSS growth described in the commit message. If you measured RSS growth before and flat RSS after, the root cause was likely something else.

2. mpp_dec_normal: the double-free does not exist

dec->mpp_pkt_in is only used by the parser-thread mode (try_get_input_packet() / try_proc_dec_task(), called from
mpp_dec_parser_thread()). The only place that attaches KEY_INPUT_PACKET to a frame meta is mpp_dec_advanced_thread(). The two modes are mutually exclusive (either parser+hal threads, or the advanced thread), so dec->mpp_pkt_in can never alias the packet in the advanced thread, and dec_release_input_packet() cannot deinit the same packet twice.