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

Nicolas George nicolas.george at normalesup.org
Tue Jun 26 15:53:18 CEST 2012

Le nonidi 9 messidor, an CCXX, Stefano Sabatini a écrit :
> > +    if (!common_init(ctx))
> > +        return 0;
> Why is not propagating the error?

The "return 0" propagates success, error is propagated by the fall-through
case, since currently the only possible failure is ENOMEM. It will require
some caution if common_init() is expanded, but at this time it is the
simplest method.

> BTW, what's your opinion with regards to the opaque field?
> The opaque field was designed to help applications which for a reason
> or another needs to access a binary struct programmatically.
> In case of the buffer and buffersink components they are supposed to
> be used programmatically, so IMO it makes sense to keep that
> interface.
> The problem I was addressing with the buffersink parameter structure
> was passing a *list* of formats, and I wanted to avoid all the
> serialization/deserialization complications to the programmer.

I think the principle is a good one, but there are two issues that makes it
doomed to fail.

First, the lack of uniformity: buffersink is the only filter using the
opaque argument, some filters use key=value syntax with the option system,
other a custom parser. We will absolutely need to clean up that.

Second, the API is too verbose: ask the library to allocate the args
structure, check for error, fill it, use it, free it. The check for error is
especially annoying.

I may have an idea to make things easier: we could use a macro that would
enable us to write something like that:

			       .formats = (int[]){ AV_SAMPLE_FMT_S16, -1 },

the macro will use the named arguments (through __VA_ARGS__ to create a
structure initializer.

The catch with that method is that the size of the structure becomes part of
the ABI. Here is the trick to solve it: put sizeof(args) as the first field
of the structure, and let the macro fill it.

For now, I believe the best course is to let the fork cripple the API as
they want and import their changes, wait until they lose interest, and then
rebuild it properly.


  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120626/abd20763/attachment.asc>

More information about the ffmpeg-devel mailing list