[FFmpeg-devel] [PATCH 1/4] lavu: add simple array implementation
lukasz.m.luki at gmail.com
Sat Mar 1 02:54:49 CET 2014
>> + * callback called inside av_array_free() to free each element of the array.
>> + */
>> +typedef void (*avarray_element_deleter)(void *element);
> A type name in all lowercase seems rather unusual in the av* API.
I don't understand. This is a function. I have already added such
declarations and no remarks were posted.
> On the whole, I am very dubious with this API.
> First, there are already several parts of array implementations in the code,
> each with various pros and cons. Adding a new one will only make matter
> worse. See av_dynarray_add() and av_dynarray2_add(): they both are
> interesting, but IIRC they have flaw that prevent them to be used in a
> generic case. Fixing them seems like a better idea.
The issue with mentioned functions is they delete buffer and doesn't
bother to delete nested pointers. In fact they only reallocate memory
when need and append new element. I updated previous patch according
your suggestions and I hope you consider it as fixed version of existing API
> Second, I do not like the insert/append/prepend stuff, and the av_array_at()
> function even worse. To set the value of an array cell, you write tab =
I have problems to agree with it as a whole. I can understand that
direct access to buffer may be useful, but this approach leaves
implementation of everything on the developer each time they need an
array. So what developer may push element to the array by tab[i] = X,
but they need to check if buffer is full and reallocate it. Also, it
works only when elements doesn't contains dynamically allocated pointers
or new element is push to the end.
The insert/append/prepend/at stuff is the kind of thing you find in
> the ultra-generic abstract java classes. So really, you would just need a
> single function that ensures a cell is free.
There is nothing wrong in being generic, glib also provides an array
implementation and it is not a Java...
Updated patch attached.
If you can't explain it simply, you don't understand it well enough. -
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 21838 bytes
Desc: not available
More information about the ffmpeg-devel