[FFmpeg-devel] [PATCH 0/3] Finish new iteration APIs

[wm4]

On Mon, 19 Feb 2018 23:30:24 -0300
James Almer <jamrial at gmail.com> wrote:

> On 2/19/2018 11:16 PM, Michael Niedermayer wrote:
> > On Mon, Feb 19, 2018 at 09:57:35PM +0100, Hendrik Leppkes wrote:  
> >> On Mon, Feb 19, 2018 at 8:30 PM, Michael Niedermayer
> >> <michael at niedermayer.cc> wrote:  
> >>> On Sun, Feb 18, 2018 at 05:58:16PM +0000, Josh de Kock wrote:  
> >>>> This should be the last of the major API changes. I'm not entirely
> >>>> sure if I missed anything.  
> >>>
> >>> Moving from a register based system where a user app can register
> >>> any subset to a system which registers all via an array will
> >>> increase the size of statically linked binaries for users only
> >>> using a subset.
> >>>  
> >>
> >> User apps did not have the ability to register a subset. How would
> >> they do that? They can't access the internals (ie. the ff_ references
> >> to those components)  
> > 
> > I think you are mistaken here.
> > 
> > What you are thinking of here, i think is, that ff_* symbols are not
> > accessable to user apps. This is true with shared libs but the issue
> > above is primarely an issue with static linking. There the ff_* symbols
> > are available.
> > 
> > But much more important, we are designing a new API here, it doesnt matter
> > all that much what was possible, what matters is that it IS possible
> > and its IMHO not a obscure use case to want to only "register" parts that are
> > actually needed. Every security concious application that deals with
> > some standarized input from the net, like a browser, would IMHO want to
> > limit the parts that are enabled by as many and as hard ways as possible.
> > 
> >   
> >> That was only something some internal tools used, and you can probably
> >> find different options for dealing with those.  
> > 
> > I can imagine some ways to hack around it, for the fuzzer yes, but a
> > clean way though proper public API (no ff_*, no #ifdefs around the array)
> > would seem better
> > 
> > So, yeah, i would prefer if a new API would allow registering subsets.
> > 
> > Without this and if the fuzzer runs out of diskspace someone will probably
> > need to hack around the new API so the arrays with all the pointers to
> > every part arent linked in. I may be missing some solution but this
> > sounds like a bunch of ugly code ...  
> 
> Afaik, the objective of this new API was to make the modules const and
> not mutable during init/registration by the requirement of setting the
> *next pointer.
> Admittedly, by keeping the init_static feature that can also set fields
> like pix_fmt or change reported capabilities, the benefits from this new
> API are more or less nullified.

That doesn't even affect filters. The pix_fmt thing affects only less
than 5 codecs.

> So i agree with you that, seeing the drawbacks this new API introduced
> without having actually achieved its objective, a different, better one
> that allows "registration", the modules to be const while setting at
> least some subset of capabilities based on the runtime environment
> (things like enabled pix_fmts, codec capabilities and such) should be
> written instead.

It has fully achieved its objectives. There's no more visible global
mutable state, and the actual global mutable state has been reduced to
less than 5 codec entries.

Why are we discussing this _again_?

> Whatever is done, in any case, should be decided fast. The current new
> API is in the tree and should be removed without delay if we decide to
> not use it in the end, even if a proper replacement is not written in
> the short term.

What needs to be done is testing and applying these patches.