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 <jamr...@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@ffmpeg.org >>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>> >> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel