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

[Michael Niedermayer]

On Wed, Feb 21, 2018 at 09:02:40PM +0100, wm4 wrote:
> On Wed, 21 Feb 2018 19:14:59 +0100
> Michael Niedermayer <michael at niedermayer.cc> wrote:
> 
> > On Wed, Feb 21, 2018 at 09:27:02AM +0100, wm4 wrote:
> > > On Tue, 20 Feb 2018 21:45:12 +0100
> > > Michael Niedermayer <michael at niedermayer.cc> wrote:
> > >   
> > > > On Tue, Feb 20, 2018 at 06:28:20PM +0100, wm4 wrote:  
> > > > > On Tue, 20 Feb 2018 17:30:32 +0100
> > > > > Michael Niedermayer <michael at niedermayer.cc> wrote:
> > > > >     
> > > > > > On Tue, Feb 20, 2018 at 10:17:02AM -0300, James Almer wrote:    
> > > > > > > On 2/20/2018 9:21 AM, wm4 wrote:      
> > > > > > > > On Tue, 20 Feb 2018 08:47:51 -0300
> > > > > > > > James Almer <jamrial at gmail.com> wrote:  
> > [...]
> > > > > And for the 100th time: the new API is completely orthogonal to
> > > > > allowing user-registered components. Since nobody could actually use
> > > > > the API before, it's no problem to drop the old APIs now, and to add
> > > > > actually working API once the other, much much much bigger problems are
> > > > > solved.
> > > > > 
> > > > > Even if you argue that keeping the linked list is absolutely necessary,
> > > > > the new API could support a linked list too.
> > > > >     
> > > > > > is it the ff_* symbols you are thinking of ?    
> > > > > 
> > > > > ff_ symbols are private.
> > > > >     
> > > > > > This is a problem for an existing API it is not a problem for a new API as
> > > > > > we can change the symbols if they are intended to be used for individual
> > > > > > component registration.
> > > > > > The whole discussion is about designing a new API. So any limitation of
> > > > > > an existing API can be changed.
> > > > > > 
> > > > > > There also should be no need to call register_all in the existing API,
> > > > > > and there cannot be such a problem in a new design because it would be
> > > > > > a new design you wouldnt design it to "not work".    
> > > > > 
> > > > > You're just being contradictory all across the board here. In other
> > > > > places you're claiming this register mechanism is needed for
> > > > > security or to make it possible to eliminate unused components when
> > > > > static linking. But if all components are already registered without
> > > > > doing anything, how is that supposed to work?    
> > > > 
> > > > If an application wants to register all, it calls the register_all()
> > > > function
> > > > If an application wants to register a subset it registers just that
> > > > subset and does not call register_all  
> > > 
> > > But you just said on your mail "There also should be no need to call
> > > register_all in the existing API,". How is that supposed to mesh with 
> > > "If an application wants to register a subset it registers just that
> > > subset and does not call register_all".  
> > 
> > I see no contradiction between what you quote, if that is what you meant:
> > "There also should be no need to call register_all in the existing API,"
> > and
> > "If an application wants to register a subset it registers just that subset and does not call register_all"
> 
> If there's no contradiction, it implies that either the first register
> call will implicitly unregister all "automatically" registered
> components, or the code has to explicitly unregister components. but in
> both cases this would still require linking all components even if you
> use static linking.
> 
> So you have the following possibilities:
> - register_all needs to be called
>   - contradicts your claim it doesn't need to be called

> - register_all needs not to be called
>   - all components are already registered implicitly

why would not calling register_all(), register all components implicitly ?


>     => static linking will pull in all components
>   - but the application can register a subset of components it needs
>     (your claim)
>     => probably means components get unregistered as soon as the
>        application registers a subset (or some really tricky linker
>        magic that removes the auto registration code if manual
>        registration functions are used?)

This sure would be possible but i dont think this complexity is needed


> 
> Well, which is it? But you could also just explicitly say what you have
> in mind.

Really, the very simple thing that register() registers one component,
register_all() registeres all, and then there is a 3rd function to
stop all further registration that could be called register_lock() or
register_disable() which returns the list of registered components after
further registration is disabled.
A library cannot register more components after that and if it somehow
managed to register a component before that is detected by the user
application which then exits with an error message explaining the user
that the security model is not compatible with a library loaded.
And that the user either has to add the specific listed components to
the components the application registers or to disable secure mode or
to do something about the library that registered the component.

This incompatibility may seem a problem but this is exactly what the
intend of this security layer is. To NEVER allow ANY code in a
process to use a component except the ones the application listed as
allowed.

We have IIRC like 400 decoders, most are odd/rare/fringe and its a valid
request to hard disable these at runtime process wide.
a browser or other security sensitive application that doesnt need them
would not want any lib it uses to open most of these fringe decoders. 


applications which are not that strict about security would not use
register_disable() and would rely on just whitelists.
They would then have to accept that a 3rd party lib that uses libavcodec
libavformat or libavfilter may do things they do not expect.
Whitelists wont protect them from a alsa or gstreamer plugin as the
application likely cannot that easily pass a whitelist throigh these
frameworks to our libs and even if it can it first has to know that
it has to, which it may not know.


> 
> > 
> > > 
> > > But indeed one goal was that applications don't have to call the
> > > completely pointless register_all functions.  
> > 
> > >   
> > > > And even the documentation says this currently:
> > > > /**
> > > >  * Register all the codecs, parsers and bitstream filters which were enabled at
> > > >  * configuration time. If you do not call this function you can select exactly
> > > >  * which formats you want to support, by using the individual registration
> > > >  * functions.
> > > >  *  
> > > 
> > > Explain how that could actually work with dynamic linking.  
> > 
> > If you see an issue with dynamic linking, please
> > explain what you mean, and i will try to show that it does not
> > affect a newly designed API.
> 
> ff_ symbols are not exported across DSO/DLL boundaries. You seem to be
> mixing the discussion about old and new APIs, yes. Part of your
> argument was that it breaks things that _were_ possible, but as I have
> been telling you, selective registration of components could not be
> used with dynamic linking.
> 
> > Fundamentally, one can of course proof this simply by showing that
> > other libs do have working registration style functions even when
> > dynamically linked.
> 
> Of course that's possible, with a new API. But we haven't seen any such
> API. You haven't described any concrete ideas. Your idea might be
> changing the component structs to be prefixed with av_ instead of ff_
> to get them exported or so. I don't think you've described any of that.

I havent really described details before because i am happy about any 
implementation that achives all goals. And i do not want to bikeshed
around on the details, my point is mainly just that 

having only a subset of components enabled is possible
and that it makes sense to support it


Yes, changing the symbols to av_* is an option.

Thanks

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180222/49799798/attachment.sig>