Double-free of MppFrame in async encoding #973

Activity

1. FumasterLin commented on Sep 14, 2026

   

   FumasterLin

   on Sep 14, 2026

   Contributor

   More actions

   Hi @nyanmisaka, thanks for the detailed report. The free in list_wraper_packet() is intentional, and the frame ownership contract in async encoding works as follows:

   1. When the frame is encoded (or dropped/skipped), mpp attaches it to the output packet's meta as KEY_INPUT_FRAME — "for user to release". The caller releases the frame only after dequeuing the corresponding packet via mpp_get_packet_async. Note that mpp_meta_get_frame is a take-once operation: the meta entry is invalidated when read, so a frame can be taken from a packet's meta exactly once.
   2. If packets are still queued in the output list at mpp_reset/mpp_destroy — i.e., packets the caller never dequeued — mpp treats the attached frames as never taken back and releases them on behalf of the caller. That is exactly what list_wraper_packet() does (introduced in 8223241 to fix the leak of those leftover frames). The same applies to frames still pending in the async input queue.

   So through the meta, the two frees never apply to the same frame: a frame already taken from a dequeued packet cannot be freed again by the wrapper, and a frame never taken back is freed only by mpp.

   The double-free warnings indicate that some frames were released by the caller without waiting for the corresponding packet — e.g., releasing the submitted frame right after put_frame, or releasing all created frames at teardown regardless of whether they were taken back from output packets. The expected sequence in async mode is:

   put_frame(frame)            -> ownership moves to mpp; don't touch frame afterwards
   get_packet_async(&packet)   -> meta KEY_INPUT_FRAME -> mpp_frame_deinit (by caller)
   residual packets/frames at reset/destroy -> cleaned up by mpp
   

   If the caller keeps ownership on its side (always freeing every frame it created), it will conflict with mpp's bottom-line cleanup for residual packets and produce exactly the warnings you observed.
2. nyanmisaka commented on Sep 14, 2026

   

   nyanmisaka

   on Sep 14, 2026

   ContributorAuthor

   More actions

   Thanks for your input. I am currently still using mpi->encode_put_frame() + mpi->encode_get_packet() with MPP_SET_{INPUT,OUTPUT}_TIMEOUT set to MPP_TIMEOUT_NON_BLOCK to use async mode. Doing so makes it easy to switch to sync mode to meet the needs of specific users.

   I only retrieve the KEY_INPUT_FRAME from the meta and free the frame after the packet has been returned by encode_get_packet(). Finally, I release this packet.

   Furthermore, all submitted frames have been correctly encoded and passed a frame-by-frame visual inspection, so there should be no packets/frames remaining in the MPP that have not been dequeued. I didn't see any other memory leak warnings.

   • Do I have to switch to mpp_put_frame_async() + mpp_get_packet_async() to correctly use async mode?
   • Are these two ways of using it equivalent?
   • What are the advantages of functions with the _async() suffix compared to my current approach?
3. FumasterLin commented on Sep 15, 2026

   

   FumasterLin

   on Sep 15, 2026

   Contributor

   More actions

   The mpp_put_frame_async interface is not publicly exposed. After you configure non-block mode, you should still use mpi->encode_put_frame() + mpi->encode_get_packet(); It will automatically route to the async path.
4. FumasterLin commented on Sep 15, 2026

   

   FumasterLin

   on Sep 15, 2026

   Contributor

   More actions

   The mPktOut in mpp.c is the queue holding the output packets of the async encoder. When the mpp context is destroyed, mpp checks whether this queue is empty; if it is not, the remaining packets are destroyed one by one, and the input frame attached to each packet's meta (KEY_INPUT_FRAME) is released there — that is exactly the free you see in list_wraper_packet().

   From your description it looks like there are packets accumulated in mPktOut that have not been dequeued when the context is destroyed. Since every encoded frame has its packet pushed to mPktOut, "all frames encoded successfully" does not imply that all packets have been retrieved — the async encoder thread may still be pushing packets after your last encode_get_packet() call returns empty/timeout, or the receive loop may have stopped earlier (e.g. right after the EOS packet).

   In that case the frames attached to those leftover packets were never taken back from the packet meta, so mpp releases them on destroy. If your teardown also releases the frames that were submitted but never returned (e.g. unref'ing all queued AVFrames at encoder close), the same frame gets deinited twice — which matches the four warnings exactly.

   So please check the close/flush sequence:

   1. After sending EOS, keep calling mpi->encode_get_packet() until no more packet is returned, and release the KEY_INPUT_FRAME of every returned packet — instead of stopping the loop early.
   2. At encoder close, do not release frames that were submitted but never returned via a packet meta. mpp's bottom-line cleanup on destroy releases exactly those.

   You can verify this on your side by counting the frames: submitted vs. taken back from packet metas. The difference should be exactly 4, matching the warnings. Internally mpp counts mPacketPutCount / mPacketGetCount and mFramePutCount / mFrameGetCount for this queue, which shows the same thing.