[FFmpeg-devel] [PATCH 2/6] sink_buffer: make opaque argument optional.

Stefano Sabatini stefasab at gmail.com
Sun Jul 1 23:57:55 CEST 2012


On date Sunday 2012-07-01 15:09:14 +0200, Nicolas George encoded:
> Le decadi 10 messidor, an CCXX, Stefano Sabatini a écrit :
[...]
> > As for what regards the syntax, many filters were ported from mplayer
> > and we tried to keep the same syntax, also when we started working on
> > lavfi many parsing facilities were not available (opt/error/eval in
> > libavcodec, no parsing facilities).
> 
> Thanks for the explanations. Do not take my mail for a reproach about the
> past, but as ideas for the future.

No offense taken, I just believed that it was useful to explain why
things are the way they are, which could make more or less sense
depending on the knowledge of the past codebase history.
 
> > This would require a custom API for each filter which needs opaque
> > data, which seems a worse solution than the previous one (before the
> > opaque field "cleanup").
> 
> The declaration of the opaque structure is already a custom API for each
> filter. The function to allocate said structure so as not to make it size
> part of the ABI even more so (but this one could be avoided with a field
> "opaque_size" in the filter structure).
> 

> I do not see the problem with custom API, provided they are well designed
> and consistent. Gtk+ does that for object creation, and it never bothered
> me, for example.

A struct *is* an object in a way, and provides already the most
flexible way (from the programmer point of view) for passing around
data, provided that you put some care when doing it, and in my
experience that's hardly a problem, and it is usually less problematic
than figuring out how to use a custom API (and requires less mental
effort to the programmer - she just need to pass around this and this
to the init function, no need to juggle around with layers and layers
of OO-obsessed APIs).
 
> The other solution would be to use AVOption extensively, but that would
> require a redesign of the init mechanism (init assumes that all the
> arguments are already set), and extending it with more complex types (lists,
> to begin with).

Anyway I'm not against an alternative solution, if can work and is not
less flexible than opaque struct passing.

The problem with the sink opaque API as observed is that it was
*verbose*, and so tendentially error prone (create the params object,
check for existence, fill it, check in case you need to create objects
in the params, free it when you're finished).

The problem with the current args-only approach is that it is
*unflexible*, clearly filling args is not very convenient if you need
to pass around complex structures like lists/dictionaries (which
implies serialization/deserialization), or if you need to pass around
pointers to global/internal program structures (we could still pass
around a string in the form: "opaque=0xdeadbeef" but how would that be
better than a separate opaque field??).

Note that in the case of the sink buffer, we lost the possibility to
define the list of accepted formats, so this now requires the creation
of a specific format filter to insert just before the sink, which is
IMO a rather bloated and inelegant solution. Note also that this
resulted in lavfi+aevalsrc being broken when you have a number of
channels >= 2. This because the buffersink is now accepting all the
sample formats, while lavfi is able to only set output for *packed*
audio formats and aevalsrc produces planar output.

A tag system may alleviate the verbosity problem (at some point I was
a user of such a tagging system implemented in C:
http://sofia-sip.sourceforge.net/refdocs/nua/ and implemented
something along that line myself in a (proprietary) application), but
of course that may need to be carefully designed.
-- 
FFmpeg = Freak & Fantastic Mythic Puritan Elitist Gigant


More information about the ffmpeg-devel mailing list