[FFmpeg-devel] [PATCH 1/5] avutil/buffer: add av_buffer_pool_flush()

[Jonas Karlman]

On 2020-12-10 00:17, James Almer wrote:
> On 12/9/2020 8:06 PM, Lynne wrote:
>> Dec 9, 2020, 23:42 by jonas at kwiboo.se:
>>
>>> On 2020-12-09 23:09, Lynne wrote:
>>>
>>>> Dec 9, 2020, 21:25 by jonas at kwiboo.se:
>>>>
>>>>> Signed-off-by: Jonas Karlman <jonas at kwiboo.se>
>>>>> ---
>>>>>   doc/APIchanges      |  3 +++
>>>>>   libavutil/buffer.c  | 13 +++++++++++++
>>>>>   libavutil/buffer.h  |  5 +++++
>>>>>   libavutil/version.h |  2 +-
>>>>>   4 files changed, 22 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/doc/APIchanges b/doc/APIchanges
>>>>> index 3fb9e12525..4a739ce453 100644
>>>>> --- a/doc/APIchanges
>>>>> +++ b/doc/APIchanges
>>>>> @@ -15,6 +15,9 @@ libavutil:     2017-10-21
>>>>>   
>>>>>   API changes, most recent first:
>>>>>   
>>>>> +2020-xx-xx - xxxxxxxxxx - lavu 56.63.100 - buffer.h
>>>>> +  Add av_buffer_pool_flush().
>>>>> +
>>>>>   2020-12-03 - xxxxxxxxxx - lavu 56.62.100 - timecode.h
>>>>>   Add av_timecode_init_from_components.
>>>>>   
>>>>> diff --git a/libavutil/buffer.c b/libavutil/buffer.c
>>>>> index d67b4bbdaf..a0683664cf 100644
>>>>> --- a/libavutil/buffer.c
>>>>> +++ b/libavutil/buffer.c
>>>>> @@ -300,6 +300,19 @@ static void buffer_pool_free(AVBufferPool *pool)
>>>>>   av_freep(&pool);
>>>>>   }
>>>>>   
>>>>> +void av_buffer_pool_flush(AVBufferPool *pool)
>>>>> +{
>>>>> +    ff_mutex_lock(&pool->mutex);
>>>>> +    while (pool->pool) {
>>>>> +        BufferPoolEntry *buf = pool->pool;
>>>>> +        pool->pool = buf->next;
>>>>> +
>>>>> +        buf->free(buf->opaque, buf->data);
>>>>> +        av_freep(&buf);
>>>>> +    }
>>>>> +    ff_mutex_unlock(&pool->mutex);
>>>>> +}
>>>>>
>>>>
>>>> This frees all buffers from the pool, which an API user probably has references to,
>>>> leading to very nasty crashes. I can't see how this is even useful nor needed.
>>>>
>>>
>>> This function should only free buffers that has been returned to the pool and
>>> is once again part of the pool->pool linked list.
>>>
>>> Main use-case is to flush and free all returned buffers in a pool used by a
>>> hwaccel ctx while a new hwaccel ctx is being initialized, i.e. during seek
>>> of H.264 video and a SPS change triggers initialization of a new hwaccel ctx.
>>>
>>> For devices with memory constraints keeping all previously allocated buffers
>>> for the old hwaccel ctx around while trying to allocate new buffers for a new
>>> hwaccel ctx tend to cause to much memory usage, especially for 2160p video.
>>>
>>> This function works around this limitation by freeing all buffers that has
>>> already been returned to the pool during hwaccel uninit, before the last
>>> buffer is returned, the last buffer is most of the time being displayed and
>>> is not returned to the pool until next frame has been decoded and displayed
>>> from next hwaccel ctx.
>>> Without this the pool buffers is only freed once the last buffer is returned.
>>>
>>> Please note that I may have mixed up hwaccel and hw frames ctx above :-)
>>>
>>> This function is being called from ff_v4l2_request_uninit and
>>> v4l2_request_hwframe_ctx_free in next patch.
>>>
>>
>> Why can't av_buffer_pool_uninit() free all the buffers currently returned in the pool?
>> You only seem to be calling it the function during uninitialization.
> 
> av_buffer_pool_uninit() frees them only after *all* the buffers have 
> been returned. In the example Jonas gave above, there may be one or two 
> frames created from buffers allocated by the pool still active and yet 
> to be unref'd by the time another hwcontext is created and new buffers 
> from a new pool start being allocated.
> 
> His intention is to free all the buffers currently held by the pool (as 
> they will not be reused anymore) to free as much memory as possible. The 
> remaining buffers will be freed once the few remaining frames using them 
> are unref'd.
> 

Correct, bellow is a short log on what happens using kodi-gbm as a player:

- start playback, new hwaccel is init:ed
ff_v4l2_request_init: avctx=0xb223d3c0 hw_device_ctx=0xb2224e10 hw_frames_ctx=(nil)
v4l2_request_init_context: pixelformat=842094158 width=3840 height=2160 bytesperline=3840 sizeimage=14515232 num_planes=1
ff_v4l2_request_frame_params: avctx=0xb223d3c0 ctx=0xa72c5c10 hw_frames_ctx=0xab90a680 hwfc=0xa726edc0 pool=0x9c83db00 width=3840 height=2160 initial_pool_size=3
- buffers gets allocated

- frames decoded

- seek to new position, old hwaccel gets uninit and a new hwaccel is init:ed

ff_v4l2_request_uninit: avctx=0xb223d3c0 ctx=0xa72c5c10
- buffer pool flushed and released buffers is freed thanks to av_buffer_pool_flush

ff_v4l2_request_init: avctx=0xb223d3c0 hw_device_ctx=0xb2224e10 hw_frames_ctx=(nil)
v4l2_request_init_context: pixelformat=842094158 width=3840 height=2160 bytesperline=3840 sizeimage=14515232 num_planes=1
ff_v4l2_request_frame_params: avctx=0xb223d3c0 ctx=0xab903130 hw_frames_ctx=0xab908740 hwfc=0xa723fc20 pool=0x99964270 width=3840 height=2160 initial_pool_size=3
- buffers gets allocated

- a few frames is decoded, player can now display a frame from new hwaccel/frames ctx and release the last frame from old hwaccel/frames ctx

v4l2_request_hwframe_ctx_free: hwfc=0xa726edc0 pool=0x9c83db00
- this is where av_buffer_pool_uninit is called and where buffers would have been freed without a flush

v4l2_request_frame_free: avctx=0xb223d3c0 data=0xab90cbf0 request_fd=74
v4l2_request_buffer_free: buf=0xab90cd88 index=5 fd=73 addr=(nil) width=3840 height=2160 size=14515232
v4l2_request_buffer_free: buf=0xab90cd24 index=5 fd=-1 addr=0x932cc000 width=3840 height=2160 size=4194304
v4l2_request_pool_free: opaque=0xb223d3c0

- frames decoded until end/stopped

Similar "issue" can be observed with vaapi h264 hwaccel, but usually not a big issue on PC with enough memory.
A more pressing issue on arm devices with limited amount of memory that can be accessed by the VPU block.

Best regards,
Jonas