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

[wm4]

On Tue, 20 Dec 2016 22:58:28 +0100
Nicolas George <george at nsup.org> wrote:

> Le decadi 30 frimaire, an CCXXV, wm4 a écrit :
> > > You mean a single structure returned by a single accessor with all the
> > > stream's properties instead of individual accessors for individual
> > > properties? Well, the naive methods of returning a structure to the
> > > caller would make the size of the structure part of the API, but there
> > > are ways around it.
> > > 
> > > I do not dislike the idea, if that is what people prefer.  
> 
> You did not answer that block. Was it an oversight?

No. I didn't see it as necessary. We seemed to be on the same page for
something that could be done theoretically.

> 
> > In general, replacing public fields with macro-generated accessors is
> > really just a rename.  
> 
> Yes. But I was thinking of this case specifically. In this case
> specifically, I think the accessors are an enhancement by themselves
> because they avoid digging in the filter's innards.

Not sure if I'd call that "innards". Though I admit that it requires
some understanding how the filters are linked together. Anyway,
accessing the links could be useful for inspecting other connections
between filters and so on. (With your patch this whole introspection
ability seems to go away? Though I'm not sure how much of it was
declared public API/ABI.)

> > AVOptions are unfortunately a good example how to create
> > encoder-specific options without "polluting" the common API. I don't
> > quite like AVOptions though, because they're too "fuzzy" for a C API
> > (av_opt essentially emulates dynamic/weak typing to some degree), and
> > they are typically even less well documented and defined like the C
> > API.  
> 
> I agree on this judgement about AVOptions.
> 
> > I don't really have a good idea how options specific to single codecs
> > or a small number of codecs should be handled. In some cases, side-data
> > is a good mechanism to move overly specific use-cases out of the common
> > API, especially for decoding.  
> 
> For that, I strongly disagree. Side data id an awful API. Whether it is
> implemented as side data or a separate field, there is an optional bit
> of information, and each part of the code needs to decide if it cares
> about it or not.
> 
> As is, side data brings exactly nothing. For each AVFrameSideDataType,
> we could have had a pointer field in the AVFrame structure, with NULL
> meaning it is not present, and that would have worked exactly the same.
> For our needs, really exactly. That would have cost 96 octets per
> frame.

Well yes, having such pointers to "optional" data would be equivalent
on some higher conceptual level, just with quite different ways to
use/access it in practice.

IMHO the important part is that presence or absence of a whole group of
fields can be signaled. Which is good for API. In general, side data
can be ignored until you're looking for certain information. That's
much better than just dumping everything into AVFrame, where everything
demands attention at once, along with essential fields.

> The differences that side data brings, or could bring if we needed it,
> or could have brought if it was implemented properly:
> 
> - Lots of accounting, memory management, unsafe casts, etc.
> 
> - Impossible to use complex data structure, side data is always flat.
> 
> - Side data can be repeated. But AFAIK we never use it. And fields could
>   point to an array or a list anyway.
> 
> - Side data is ordered. We do not use it.
> 
> - Side data can be processed in sequence, without looking at its type.
>   Except that to do anything useful except copying and freeing, we need
>   to know the type.
> 
>   So yeah, we gain something by using side data: each time we add a new
>   type, we gain two lines:
>   "if (av_something_copy(&dst->field, src->field)) goto fail;" in
>   frame_copy_props() and "av_something_freep(&frame->field)" in
>   av_frame_unref(). Big whoop!
> 
> - Side data could have been decentralized: lavc/fooenc and lavc/foodec
>   define their own type, nobody else cares about it; lavfi/foodetect and
>   lavfi/footweak define their own type, nobody else cares about it.
>   Except AVFrameSideDataType is centralized. Too bad.
> 
> I think some people entertained the fantasy that AVFrame could be
> "generic", but did not really think it through.

I agree about those points. I particularly dislike that we can't seem
to decide whether side data should be a bytestream (defined explicitly,
like a file format) or the contents of a struct (defined by C API/ABI).

You forgot to mention the code duplication between frame/packet/stream
side data management.

> At some point (after reinventing the options system as outlined in
> http://ffmpeg.org/pipermail/ffmpeg-devel/2015-December/184525.html), I
> will probably propose to get rid of all this, unless it has actually
> become useful.
> 
> But it has gotten quite out of topic.
> 
> > If you look how Microsoft handles this, you'll see that they put
> > additional encoder controls into separate COM interfaces, which can be
> > not present in simpler encoders.  
> 
> I have no idea how the COM system works, but skimming over "COM
> Technical Overview" from MS, I have to say: I do not want anything like
> that gas factory near us!

I think you're not dismissing it too early. It provides some
interesting concepts in term of ABI and API compatibility. Although
it's terrible to use, there's something that can be learned from it.

To make it short, everything in COM consists of structs with function
pointers. Structs are never extended, if you need new function
pointers, you just add new structs, which you can dynamically query
from the old ones. This gives you 100% ABI downwards and upwards
compatibility. You also don't have to worry about linking to functions
not present in older Windows or whatever versions, because structs with
function pointers are a compile-time thing. You will merely get a
runtime failure when trying to query support for an unsupported struct.
(I tried to avoid COM terms, but COM calls these structs "interfaces".)

It's a pretty fascinating contrast to the fuckhackery we do with
extending structs while trying to keep ABI compatibility, version
and configure checks in API user code when trying to stay compatible
with multiple libavcodec versions, etc.

> > I'm just saying we're not doing it even where we could. I would claim
> > that the right mindset is just not there.  
> 
> The problem is that often APIs that "force correct use by API design"
> are actually "APIs that are so annoying to use that decent developers
> flee to other languages", the Java way.

Not necessarily. Java API design might serve as negative example, sure.