[FFmpeg-devel] [PATCH 1/6] lavfi/buffersink: add accessors for the stream properties.

[James Almer]

On 12/18/2016 3:32 PM, Nicolas George wrote:
> L'octidi 28 frimaire, an CCXXV, James Almer a écrit :
>> Is this to make AVFilterLink a fully internal struct?
> 
> As you can see in patch 6/6 in this series.
> 
>> 							What's the reason
>> to do that in that case, instead of simply making its private fields
>> actually internal and private?
> 
> Can you suggest a method?
> 
>> The idea for next bump was to clean all this mess for once and for all by
> 
> Good: after this patch, there is no mess, everything is accessed through
> the buffersink API.

By breaking the API, adding extra abstraction and a bunch of new symbols.
You didn't answer what's the gain here. How is this better than keeping the
struct public and letting library users keep accessing its fields normally?
Why are you trying to make libavfilter so different than the rest? We have
scheduled the deprecation and removal of /all/ accessors, and now you want
to add more?

If people didn't use lavfi before, they will feel less motivated to do it
now since they can't even expect consistency with lavf or lavc.
And those that currently do will find themselves having to adapt their
programs without being given a reason as to why they are forced to, unlike
with other big API changes.

> 
>> deprecating all accessors, removing all the "no direct access" notes and
>> moving all the private fields in public structs (Marked with a big "not
>> part of the public API" warning like in the case of with AVFilterLink)
>> into actual internal structs.
> 
> By "actual internal structs", I suspect you mean something similar to
> this:
> 
> typedef struct AVFormatContext {
>     ...
>     AVFormatInternal *internal;
>     ...
> };
> 
> Introducing these structures was a big mistake. For the reasons, see the
> recent discussion about making filter_frame() non-recursive (short of
> it: it makes the actual code unreadable), plus another discussion I did

Back to what i said above. You're breaking API and bothering lavfi users
to make internal code "more readable"?

> not take part about using options on these structure (short of it: a lot
> of work if even possible).
> 
> I do not intend to extend that mistake in libavfilter. If possible, I
> would rather like to kick it out, but fortunately the current uses are
> very limited.

I get you don't like it, but the objective should be to provide a familiar
and user friendly API. Opaque internal structs don't affect users, they
don't ever have to worry about them, it's one field they will never touch.
Making the entire struct internal and bloating the library with accessors
to write or read what has been for years and is expected to be public
fields only because you want another way to hide internal state is a bit
overkill and disruptively user unfriendly.