demux: reclaim demux_packets to reduce memory allocator pressure #14955
+317
−56
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Download the artifacts for this pull request: |
na-na-hi
reviewed
Sep 30, 2024
kasper93
force-pushed
the
demux_pool
branch
6 times, most recently
from
8830632 to
48006ce
Compare
sfan5
requested changes
Jan 21, 2025
sfan5
reviewed
Jan 26, 2025
This update introduces a demux_packet_pool, allowing for the reuse of previously allocated packets when needed. sizeof(AVPacket) is not a part of the lavc public ABI, which prevents us to allocate memory in larger blocks. However, we can substantially decrease the amount of alloc/free operations during playback by reusing both mpv's demux_packet and AVPacket. This adjustment addresses the root cause of issue mpv-player#12076, which, although resolved upstream, did not fully tackle the persistent problem of allocating small blocks of aligned memory. This issue largely stems from the FFmpeg design of the AVPacket API. After this change memory will no longer be allocated once cache limits is reached. The demux_packet_pool is shared as a global pool of packets for a given MPContext. This change significantly speeds up the demuxer deinitialization, benefiting file switching scenarios, especially when a large demuxer cache is used. See: mpv-player#12294 See: mpv-player#12563 Signed-off-by: Kacper Michajłow <kasper93@gmail.com>
For debugging without a pool, maybe.
|
Hopefully it will be fine, we can always revert. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This update introduces a demux_packet_pool, allowing for the reuse of previously allocated packets when needed.
sizeof(AVPacket) is not a part of the lavc public ABI, which prevents us to allocate memory in larger blocks. However, we can substantially decrease the amount of alloc/free operations during playback by reusing both mpv's demux_packet and AVPacket.
This adjustment addresses the root cause of issue #12076, which, although resolved upstream, did not fully tackle the persistent problem of allocating small blocks of aligned memory. This issue largely stems from the FFmpeg design of the AVPacket API. After this change memory will no longer be allocated once cache limits is reached.
The demux_packet_pool is shared as a global pool of packets for a given MPContext.
This change significantly speeds up the demuxer deinitialization, benefiting file switching scenarios, especially when a large demuxer cache is used.
See: #12294
See: #12563
I decided to open a PR without the "memory leak" part, which was the main discussion point in #12566. After refining the patches, it is no longer the main objective of this change. Therefore, #12566 remains focused on the "fast quit" option, and here we introduce the demux pool. Hopefully, we will be able to iterate on these patches in this way.