[FFmpeg-devel] [PATCH 1/4] lavu: add simple array implementation

[Lukasz Marek]

>> +/**
>> + * 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[42] =
> 1729.

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.

-- 
Best Regards,
Lukasz Marek

If you can't explain it simply, you don't understand it well enough. - 
Albert Einstein
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavu-add-simple-array-implementation.patch
Type: text/x-patch
Size: 21838 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140301/2c67abf8/attachment.bin>