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

wm4 nfxjfg at googlemail.com
Mon Mar 3 12:38:38 CET 2014


On Mon, 3 Mar 2014 12:28:19 +0100
Michael Niedermayer <michaelni at gmx.at> wrote:

> On Mon, Mar 03, 2014 at 11:55:24AM +0100, wm4 wrote:
> > On Mon, 3 Mar 2014 11:33:20 +0100
> > Michael Niedermayer <michaelni at gmx.at> wrote:
> > 
> > > On Mon, Mar 03, 2014 at 11:00:18AM +0100, Lukasz M wrote:
> > > > W dniu poniedziałek, 3 marca 2014 Michael Niedermayer <michaelni at gmx.at>
> > > > napisał(a):
> > > > 
> > > > > On Mon, Mar 03, 2014 at 01:59:43AM +0100, Lukasz Marek wrote:
> > > > > >
> > > > > > >You're adding it to lavu, and it's not marked as internal/experimental.
> > > > > If
> > > > > > >it stinks or needs 3 API updates or re-implementations, the old cruft
> > > > > > >sticks around for years to come.
> > > > > >
> > > > > > Can you precise how to mark it? I added it as public, but don't
> > > > > > understand how can i make it experimental.
> > > > > >
> > > > >
> > > > > > >That's why we now have 2 (and with yours, soon 3) array implementations.
> > > > > > >Can we stick to one? At least java uses the same API for all its
> > > > > array/list
> > > > > > >things.
> > > > > >
> > > > > > The existing two doesn't allow to free "objects" with allocated
> > > > > > pointers inside. If you fix them then you have 4. I really think it
> > > > > > is better to have one common for wide range of use cases. I can
> > > > > > refactor existing code to use mine if accepted.
> > > > >
> > > > > if i have an array of "object" with pointers inside them and want
> > > > > to free them id do
> > > > >
> > > > > for(i=0; i<arraysize; i++)
> > > > >     my_free(my_array + i);
> > > > >
> > > > > that needs no API beyond ANSI/ISO C
> > > > > what am i missing ?
> > > > >
> > > > 
> > > > nothing. existing array impl is not so smart to do that on memory
> > > > allocation fail AFAIR.
> > > 
> > > maybe it could be solved with something like
> > > int av_dynarray_add_nofree(...)
> > > 
> > > which would not free the array on failure but return an error code
> > > and leave deallocation (if needed) to the caller
> > 
> > Freeing the whole array just because adding one element failed sounds
> > quite idiotic. (Though I guess it's ok for certain use cases, like
> > initialization.) But apparently these are the existing semantics. Adding
> > the function you propose sounds like a good idea.
> > 
> 
> > Even with the av_dynarray_add() semantics, one could just pass a free
> > callback to the function, instead of needing an array struct.
> 
> yes, i was thinking of that too but i couldnt decide what arguments
> the callback would need. It might need an opaque / context pointer
> possibly but then one needs to pass that too to av_dynarray_add
> so i ended up suggesting what i did suggest ...

Yeah, leaving destruction to the user is simpler anyway. The user just
needs to create its own destruction function. (He'll need only one.)

In general, this seems like a typical case where trying to add a
generic trivial data structure to a C program causes more pain than it
solves.

> 
> > 
> > By the way, av_dynarray_add() seems to violate strict aliasing rules...
> 
> not sure if what iam thinking of is what you meant but
> ISO/ANSI C allows char types to alias anything

It does:

   tab = *(intptr_t**)tab_ptr;

Where tab_ptr is supposed to be "an array of pointers to structures",
so it's similar to av_freep() as in that the implementation is free of
aliasing violations, but it encourages/requires aliasing violations on
the caller site.


More information about the ffmpeg-devel mailing list