[FFmpeg-devel] [PATCH] [RFC][WIP] avutil/buffer: add add a dynamnic size buffer pool API

James Almer jamrial at gmail.com
Sun Mar 18 03:46:22 EET 2018


On 3/17/2018 10:33 PM, Michael Niedermayer wrote:
> On Sat, Mar 17, 2018 at 09:26:32PM -0300, James Almer wrote:
>> On 3/17/2018 8:48 PM, Michael Niedermayer wrote:
>>> On Fri, Mar 16, 2018 at 03:21:41PM -0300, James Almer wrote:
>>>> Signed-off-by: James Almer <jamrial at gmail.com>
>>>> ---
>>>> This is a proof of concept for a dynamic size buffer pool API.
>>>>
>>>> For the purpose of easy testing and reviewing I replaced the current
>>>> linked list used to keep a pool of fixed size buffers with the tree
>>>> based pool that will be used to keep a pool of varying size buffers,
>>>> instead of adding a new set of functions exclusively for the new API.
>>>> The final committed work doesn't necessarely have to do the above, as
>>>> there's no real benefit using a tree when you only need a fixed size
>>>> buffer pool, other than simplying things.
>>>>
>>>> I'm open to suggestions about how to introduce this. Completely
>>>> separate set of functions and struct names? Sharing the struct and
>>>> init/uninit functions and only adding a new get() one like in this
>>>> patch?
>>>> Any preferences with function/struct naming, for that matter?
>>>>
>>>>  libavutil/buffer.c          | 98 ++++++++++++++++++++++++++++++++++++---------
>>>>  libavutil/buffer.h          |  2 +
>>>>  libavutil/buffer_internal.h |  6 ++-
>>>>  3 files changed, 85 insertions(+), 21 deletions(-)
>>>
>>> not sure its not intended but this causes differences
>>> in error concealment on many files
>>
>> Not intended by me, at least. And not sure how using a tree instead of a
>> linked list to keep a pool of buffers could affect that. The way they
>> are allocated or returned is not changed, and all the existing users are
>> still only creating fixed sized buffers.
>>
> 
>> Do you have an idea of what could be happening? The way the tree is
> 
> is always the same buffer used as before ?

Was thinking that, yes. The pool organized by the tree most assuredly
orders the buffers in a different way than the linked list (Which i
think is simply FIFO), so the h264 decoder or whatever is handling the
pool here now gets a different buffer after calling av_buffer_pool_get().
This would mean the EC code is overreading bytes, and that the buffer is
not zeroed after being requested. The former should probably be fixed.

Fortunately not a bug from this patch/implementation, in that case.

> a different buffer with different content could probably leak the content
> through into the output on some errors.
> 
> 
>> being handled is afaik correct, judging by the doxy. Removing and adding
>> one element at a time using a simple cmp() function and with no way to
>> have races since it's all controlled by a mutex.
>>
>>>  an example would be something like this:
>>>
>>> ffmpeg -threads 1 -i 1069/green-block-artifacts-from-canon-100-hs.MOV test.avi
>>>
>>>
>>> [...]
>>>
>>>
>>>
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel at ffmpeg.org
>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 



More information about the ffmpeg-devel mailing list