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

[Nicolas George]

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:

    avfilter_buffersink_create(mandatory_args,
			       .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.

Regards,

-- 
  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>