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

[wm4]

On Fri, 23 Feb 2018 01:21:52 +0100
Michael Niedermayer  wrote:

> On Thu, Feb 22, 2018 at 07:31:27PM +0100, wm4 wrote:
> > On Thu, 22 Feb 2018 16:28:56 +0100
> > Michael Niedermayer  wrote:
> >   
> > > On Wed, Feb 21, 2018 at 09:02:40PM +0100, wm4 wrote:  
> > > > On Wed, 21 Feb 2018 19:14:59 +0100
> > > > Michael Niedermayer  wrote:
> > > > 
> > > > > On Wed, Feb 21, 2018 at 09:27:02AM +0100, wm4 wrote:
> > > > > > On Tue, 20 Feb 2018 21:45:12 +0100
> > > > > > Michael Niedermayer  wrote:
> > > > > >   
> > > > > > > On Tue, Feb 20, 2018 at 06:28:20PM +0100, wm4 wrote:  
> > > > > > > > On Tue, 20 Feb 2018 17:30:32 +0100
> > > > > > > > Michael Niedermayer  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  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 
> > > ?  
> > 
> > So what did you mean by "There also should be no need to call
> > register_all in the existing API,"?  
> 
> That as documented the user app should be able to 

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

[Tobias Rapp]

On 22.02.2018 16:47, Nicolas George wrote:

Hi.

Not sure where in the thread would be the best place to reply, so I
might as well reply here.

There is one idea I have been toying with for some time that relates to
codec registration. It is significantly different from the direction
this discussion was taking, but I think it offers a lot of advantages.

Let us have "struct AVCodecLibrary" with the lists of codecs, bsfs, etc.
Whenever a function needs to access the lists, have it take an
AVCodecLibrary argument. It can be made automatic most of the time by
adding an AVCodecLibrary field to all AVSomethingContext structs.

[...]

The same structure could be used to set all mutable global state, like
the log level and callback.


I like the idea of storing global context within some structure.

Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[Michael Niedermayer]

On Thu, Feb 22, 2018 at 07:31:27PM +0100, wm4 wrote:
> On Thu, 22 Feb 2018 16:28:56 +0100
> Michael Niedermayer  wrote:
> 
> > On Wed, Feb 21, 2018 at 09:02:40PM +0100, wm4 wrote:
> > > On Wed, 21 Feb 2018 19:14:59 +0100
> > > Michael Niedermayer  wrote:
> > >   
> > > > On Wed, Feb 21, 2018 at 09:27:02AM +0100, wm4 wrote:  
> > > > > On Tue, 20 Feb 2018 21:45:12 +0100
> > > > > Michael Niedermayer  wrote:
> > > > > 
> > > > > > On Tue, Feb 20, 2018 at 06:28:20PM +0100, wm4 wrote:
> > > > > > > On Tue, 20 Feb 2018 17:30:32 +0100
> > > > > > > Michael Niedermayer  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  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 ?
> 
> So what did you mean by "There also should be no need to call
> register_all in the existing API,"?

That as documented the user app should be able to register individual 
components OR to call register_all().
As register_all() is only required if the user application does not
register components individually, it is not needed to call register_all()

We somehow seem to misunderstand each other here over several mails, iam
not sure why. This is not complex or convoluted 


[...]
> > A library cannot register more components after that and if it 

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

[Rostislav Pehlivanov]

On 22 February 2018 at 18:31, wm4  wrote:

>
>
> > We have IIRC like 400 decoders, most are odd/rare/fringe and its a valid
> > request to hard disable these at runtime process wide.
>
> Maybe we should just disable them by default, instead of forcing the
> user to play security russian roulette with them?
>
>
I really don't think that's a good idea. With no reason to improve them
most will bitrot and people who need to decode them in the future will need
to compile ancient versions which probably won't work without changes. The
whole point of the library is to have something which can handle pretty
much everything out of the box.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[wm4]

On Thu, 22 Feb 2018 16:28:56 +0100
Michael Niedermayer  wrote:

> On Wed, Feb 21, 2018 at 09:02:40PM +0100, wm4 wrote:
> > On Wed, 21 Feb 2018 19:14:59 +0100
> > Michael Niedermayer  wrote:
> >   
> > > On Wed, Feb 21, 2018 at 09:27:02AM +0100, wm4 wrote:  
> > > > On Tue, 20 Feb 2018 21:45:12 +0100
> > > > Michael Niedermayer  wrote:
> > > > 
> > > > > On Tue, Feb 20, 2018 at 06:28:20PM +0100, wm4 wrote:
> > > > > > On Tue, 20 Feb 2018 17:30:32 +0100
> > > > > > Michael Niedermayer  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  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 ?

So what did you mean by "There also should be no need to call
register_all in the existing API,"?

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

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

[Nicolas George]

Hi.

Not sure where in the thread would be the best place to reply, so I
might as well reply here.

There is one idea I have been toying with for some time that relates to
codec registration. It is significantly different from the direction
this discussion was taking, but I think it offers a lot of advantages.

Let us have "struct AVCodecLibrary" with the lists of codecs, bsfs, etc.
Whenever a function needs to access the lists, have it take an
AVCodecLibrary argument. It can be made automatic most of the time by
adding an AVCodecLibrary field to all AVSomethingContext structs.

Amongst the benefits I see:

If an application deals with trusted and untrusted files, it can use two
instances of AVCodecLibrary, one with only reliable codecs, one with
all. It is more robust and easier than the whitelists.

If an application uses lavc directly but also indirectly through a
library (or indirectly twice through two libraries), the instances are
independent since they use a different instance of AVCodecLibrary.

The same structure could be used to set all mutable global state, like
the log level and callback.

Please comment.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [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  wrote:
> 
> > On Wed, Feb 21, 2018 at 09:27:02AM +0100, wm4 wrote:
> > > On Tue, 20 Feb 2018 21:45:12 +0100
> > > Michael Niedermayer  wrote:
> > >   
> > > > On Tue, Feb 20, 2018 at 06:28:20PM +0100, wm4 wrote:  
> > > > > On Tue, 20 Feb 2018 17:30:32 +0100
> > > > > Michael Niedermayer  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  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 

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

[wm4]

On Sun, 18 Feb 2018 17:58:16 +
Josh de Kock  wrote:

> This should be the last of the major API changes. I'm not entirely
> sure if I missed anything. 
> 
> Josh
> 

All 3 patches LGTM if all tests pass and ffmpeg.c codec/device/filter
listing is the same as before the patch.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[wm4]

On Wed, 21 Feb 2018 19:14:59 +0100
Michael Niedermayer  wrote:

> On Wed, Feb 21, 2018 at 09:27:02AM +0100, wm4 wrote:
> > On Tue, 20 Feb 2018 21:45:12 +0100
> > Michael Niedermayer  wrote:
> >   
> > > On Tue, Feb 20, 2018 at 06:28:20PM +0100, wm4 wrote:  
> > > > On Tue, 20 Feb 2018 17:30:32 +0100
> > > > Michael Niedermayer  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  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
=> 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?)

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

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

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

[Michael Niedermayer]

On Wed, Feb 21, 2018 at 09:27:02AM +0100, wm4 wrote:
> On Tue, 20 Feb 2018 21:45:12 +0100
> Michael Niedermayer  wrote:
> 
> > On Tue, Feb 20, 2018 at 06:28:20PM +0100, wm4 wrote:
> > > On Tue, 20 Feb 2018 17:30:32 +0100
> > > Michael Niedermayer  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  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"


> 
> 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.
Fundamentally, one can of course proof this simply by showing that
other libs do have working registration style functions even when
dynamically linked.


> 
> > 
> > >   
> > > >   
> > > > > 
> > > > > Nevermind then, my argument was focused on preventing losing some link
> > > > > time optimization for static libraries assuming proper API usage.
> > > > > 
> > > > > > 
> > > > > > And that can't be made with dynamic linking either. If you 
> > > > > > statically
> > > > > > link anyway, you probably control everything, and you can configure 
> > > > > > this
> > > > > > stuff at compile time.
> > > > > > 
> > > > > > Then I guess there's this very special case where a fuzzer 
> > > > > > statically
> > > > > > links to libavcodec, and registers only a few components 
> > > > > > (technically
> > > > > > violating the API and by guessing the component symbol name), and
> > > > > > without calling the register_all functions. This would make the
> > > > > > resulting executable much smaller, which is apparently important
> > > > > > because Google (who runs the fuzzers for their oss-fuzz project) is 
> > > > > > too
> > > > > > poor (?) to host all the statically linked fuzzers, _or_ the 
> > > > > > whitelist
> > > > > > stuff is not enough to trick the fuzzer into not trying to fuzz the
> > > > > > wrong code. In addition, it's apparently infeasible to just build
> > > > > > every fuzzer with a separate libavcodec. (Not sure about the 
> > > > > > details, it
> > > > > > was something like this.)
> > > > > > 
> > > > > > Those requirements are really quite nebulous. I don't 

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

[James Almer]

On 2/21/2018 5:04 PM, wm4 wrote:
> On Sun, 18 Feb 2018 17:58:16 +
> Josh de Kock  wrote:
> 
>> This should be the last of the major API changes. I'm not entirely
>> sure if I missed anything. 
>>
>> Josh
>>
> 
> All 3 patches LGTM if all tests pass and ffmpeg.c codec/device/filter
> listing is the same as before the patch.

I recall he mentioned on IRC some problems with threading after further
testing this set. I don't know if that was fixed (he at least didn't sed
an updated version), or if it was some artifact of a dirty testing tree.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[Michael Niedermayer]

On Wed, Feb 21, 2018 at 04:04:15PM +0700, Muhammad Faiz wrote:
> On Tue, Feb 20, 2018 at 2:30 AM, Michael Niedermayer
>  wrote:
> > On Sun, Feb 18, 2018 at 05:58:16PM +, 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.
> >
> > An example of this effect for codecs is tools/target_dec_fuzzer.c
> 
> Just an idea.
> Is it possible to generate just one executable to fuzz all decoders?

I suggested this last year already:
https://github.com/google/oss-fuzz/pull/559#issuecomment-298438415

From what i remember, this suggestion was not favoured

IIRC, after this we then eliminated all parts that can be and used 
the  linker with registering just the needed components to reduce the
size to within the constraints



> The target decoder can be added as an argument or environment variable.
> e.g: ./fuzzer --target aac
> or FUZZER_TARGET=aac ./fuzzer
> 
> With this approach, we can save lot of disk space.

ffmpeg-devel really is the wrong place to discuss this.
The best bet is probably just to implement it (make sure it works, existing
testcases still reproduce issues, stack traces still are useable, ...)
and send a pull request to ossfuzz (with a CC to me or ffmpeg-devel)

Iam not able to say yes or no to this as i simply do not know why some
of the preferrances in the design exist.

thx

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

If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[Muhammad Faiz]

On Tue, Feb 20, 2018 at 2:30 AM, Michael Niedermayer
 wrote:
> On Sun, Feb 18, 2018 at 05:58:16PM +, 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.
>
> An example of this effect for codecs is tools/target_dec_fuzzer.c

Just an idea.
Is it possible to generate just one executable to fuzz all decoders?
The target decoder can be added as an argument or environment variable.
e.g: ./fuzzer --target aac
or FUZZER_TARGET=aac ./fuzzer

With this approach, we can save lot of disk space.


>
> This effect results because when the linker collects all objects
> of all static libs with a user application it can remove all symbols
> which are not referenced.
> If there is a single array that references all codecs, filters, formats
> and this array is referenced by a function which must be used then
> nothing can be removed at the link stage
>
> OTOH with a register function a register all which refernces all
> a user application can simply not use the reference all code and register
> the specific subset it needs with individual register calls.
> In this case the linker can ommit the register all and all codecs, formats
> and filters teh user application does not explicitly refer too.
> In the case where this applies this results in significantly smaller files
>
> I also expect they link faster and load faster but i forgot to benchmark this
> when i realized this issue exists and tested ...
>
> I am thus in favor of a system that does not unconditionally reference every
> codec/format/filter
>
> There can also be an advantage security wise if unneeded parts are never
> "registered"
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Many things microsoft did are stupid, but not doing something just because
> microsoft did it is even more stupid. If everything ms did were stupid they
> would be bankrupt already.
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[wm4]

On Tue, 20 Feb 2018 21:45:12 +0100
Michael Niedermayer  wrote:

> On Tue, Feb 20, 2018 at 06:28:20PM +0100, wm4 wrote:
> > On Tue, 20 Feb 2018 17:30:32 +0100
> > Michael Niedermayer  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  wrote:
> > > > > 
> > > > >>> 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.  
> > > > >>
> > > > >> It's true that after the deprecation period they will effectively be 
> > > > >> the
> > > > >> only ones still mutable, but shouldn't the objective be to cover 
> > > > >> them all?
> > > > > 
> > > > > That would be nice. But this has been discussed before. No kind of
> > > > > different registration API could fix this issue either, unless we 
> > > > > start
> > > > > mallocing AVCodec structs and require the user to free them, or
> > > > > something equally absurd. The proper solution for this specific issue
> > > > > would be making a new API that somehow replaces AVCodec.pix_fmts.
> > > > > 
> > > > >>>
> > > > >>> Why are we discussing this _again_?  
> > > > >>
> > > > >> Because a drawback has been found, namely that link time optimization
> > > > >> using static libraries can't remove "non registered" modules anymore,
> > > > >> and now depends fully on what's enabled at configure time.
> > > > >> Ideally, a better registration based API that follows what i 
> > > > >> described
> > > > >> above should be designed with care.
> > > > > 
> > > > > Oh yeah, bikeshed attack by Michael. As it was said a few thousand 
> > > > > times
> > > > > already, public API users could NOT use the registration stuff to
> > > > > register only specific components at runtime. So they have to use the
> > > > > register_all variants, which pull in _all_ components even with static
> > > > > linking.
> > > > 
> > > > Oh, i assumed it was possible to use avcodec_register() on a manual
> > > > basis in a proper API usage scenario, but i see now that's not the 
> > > > case.
> > > 
> > > of course its possible to use avcodec_register() on a manual basis in a 
> > > proper API usage scenario, why would it not be ?
> > > 
> > > why do you think this function exists or was written ?  
> > 
> > I don't know, but it didn't make any sense. And wouldn't have made for
> > years to come.  
> 
> thats backward
> 
> if you look at git history the very first checkin 
> commit 9aeeeb63f7e1ab7b0b7bb839a5f258667a2d2d78 (HEAD)
> Author: Fabrice Bellard 
> Date:   Wed Dec 20 00:02:47 2000 +
> 
> Initial revision
> 
> has code like this:
> 
> ffmpeg.c:register_avencoder(_encoder);
> ffmpeg.c:register_avencoder(_encoder);
> ffmpeg.c:register_avencoder(_encoder);
> ffmpeg.c:register_avencoder(_encoder);
> 
> Here the application did use the register API to selectivly register
> codecs.

That's before FFmpeg has a stable API or even just ABI. It took a
decade or so to get there. I also think we didn't find a single
application that tried to do this these days. Maybe the fact that it
simply didn't work with dynamic linking also mattered.

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

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

[Michael Niedermayer]

On Tue, Feb 20, 2018 at 06:28:20PM +0100, wm4 wrote:
> On Tue, 20 Feb 2018 17:30:32 +0100
> Michael Niedermayer  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  wrote:
> > > >   
> > > >>> 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.
> > > >>
> > > >> It's true that after the deprecation period they will effectively be 
> > > >> the
> > > >> only ones still mutable, but shouldn't the objective be to cover them 
> > > >> all?  
> > > > 
> > > > That would be nice. But this has been discussed before. No kind of
> > > > different registration API could fix this issue either, unless we start
> > > > mallocing AVCodec structs and require the user to free them, or
> > > > something equally absurd. The proper solution for this specific issue
> > > > would be making a new API that somehow replaces AVCodec.pix_fmts.
> > > >   
> > > >>>
> > > >>> Why are we discussing this _again_?
> > > >>
> > > >> Because a drawback has been found, namely that link time optimization
> > > >> using static libraries can't remove "non registered" modules anymore,
> > > >> and now depends fully on what's enabled at configure time.
> > > >> Ideally, a better registration based API that follows what i described
> > > >> above should be designed with care.  
> > > > 
> > > > Oh yeah, bikeshed attack by Michael. As it was said a few thousand times
> > > > already, public API users could NOT use the registration stuff to
> > > > register only specific components at runtime. So they have to use the
> > > > register_all variants, which pull in _all_ components even with static
> > > > linking.  
> > > 
> > > Oh, i assumed it was possible to use avcodec_register() on a manual
> > > basis in a proper API usage scenario, but i see now that's not the case.  
> > 
> > of course its possible to use avcodec_register() on a manual basis in a 
> > proper API usage scenario, why would it not be ?
> > 
> > why do you think this function exists or was written ?
> 
> I don't know, but it didn't make any sense. And wouldn't have made for
> years to come.

thats backward

if you look at git history the very first checkin 
commit 9aeeeb63f7e1ab7b0b7bb839a5f258667a2d2d78 (HEAD)
Author: Fabrice Bellard 
Date:   Wed Dec 20 00:02:47 2000 +

Initial revision

has code like this:

ffmpeg.c:register_avencoder(_encoder);
ffmpeg.c:register_avencoder(_encoder);
ffmpeg.c:register_avencoder(_encoder);
ffmpeg.c:register_avencoder(_encoder);

Here the application did use the register API to selectivly register
codecs.



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

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


> 
> > 
> > > 
> > > Nevermind then, my argument was focused on preventing losing some link
> > > time optimization for static libraries assuming proper API usage.
> > >   
> > > > 
> > > > And that can't be made with dynamic linking either. If you statically
> > > > link anyway, you probably control 

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

[Michael Niedermayer]

On Tue, Feb 20, 2018 at 02:02:14PM -0300, James Almer wrote:
> On 2/20/2018 1:30 PM, Michael Niedermayer 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  wrote:
> >>>
> > 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.  
> 
>  It's true that after the deprecation period they will effectively be the
>  only ones still mutable, but shouldn't the objective be to cover them 
>  all?
> >>>
> >>> That would be nice. But this has been discussed before. No kind of
> >>> different registration API could fix this issue either, unless we start
> >>> mallocing AVCodec structs and require the user to free them, or
> >>> something equally absurd. The proper solution for this specific issue
> >>> would be making a new API that somehow replaces AVCodec.pix_fmts.
> >>>
> >
> > Why are we discussing this _again_?  
> 
>  Because a drawback has been found, namely that link time optimization
>  using static libraries can't remove "non registered" modules anymore,
>  and now depends fully on what's enabled at configure time.
>  Ideally, a better registration based API that follows what i described
>  above should be designed with care.
> >>>
> >>> Oh yeah, bikeshed attack by Michael. As it was said a few thousand times
> >>> already, public API users could NOT use the registration stuff to
> >>> register only specific components at runtime. So they have to use the
> >>> register_all variants, which pull in _all_ components even with static
> >>> linking.
> >>
> >> Oh, i assumed it was possible to use avcodec_register() on a manual
> >> basis in a proper API usage scenario, but i see now that's not the case.
> > 
> > of course its possible to use avcodec_register() on a manual basis in a 
> > proper API usage scenario, why would it not be ?
> > 
> > why do you think this function exists or was written ?
> > 
> > is it the ff_* symbols you are thinking of ?
> 
> Yes. You can't register individual AVCodecs using avcodec_register()
> without those symbols, and in a normal API usage scenario, be it with
> the old or the new iterate one, they are not available.
> 
> > 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.
> 
> Hence me also being a proponent of a different, better registration
> based API.
> I think you misunderstood what i tried to say in the email you replied
> to. You reported a drawback of this new API by giving the example of the
> fuzzer not being able to discard unused codec symbols since all are
> unconditionally referenced now. 


> I assumed that the fuzzer was using the
> API properly all this time, but turns out it's not, digging the
> internals instead to register single codecs. 

You certainly have a point here
though most strictly looking at it, the fuzzer is internal in our tree and
thus it doesnt really violate the API any more than other internal
components do.


> This therefore can't be
> considered a change in behavior compared to the current/old registration
> based API.

Well, the fuzzer binaries are like several times bigger than before that is a
difference caused by the patches. Not by the puiblic API, no
but iam not sure this way of looking at it is really helpfull to any side of
this arguemnt


> 
> Which brings me to the next part below.
> 
> > 
> > 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".
> > 
> > 
> >>
> >> Nevermind then, my argument was focused on preventing losing some link
> >> time optimization for static libraries assuming proper API usage.
> >>
> >>>
> >>> And that can't be made with dynamic linking either. If you statically
> >>> link anyway, you probably control everything, and you can configure this
> >>> stuff at compile time.
> >>>
> >>> Then I guess there's this very special case where a fuzzer statically
> >>> links to libavcodec, and registers only a few components (technically
> >>> violating the API and by guessing the component symbol name), and
> >>> without calling the register_all functions. This would make the
> >>> resulting executable much smaller, which is apparently important
> >>> because Google (who runs the fuzzers for their oss-fuzz project) is too
> >>> poor (?) to host all the statically linked fuzzers, _or_ the whitelist
> >>> stuff is not enough to trick the fuzzer into not trying to fuzz the
> >>> wrong code. In addition, it's apparently 

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

[Michael Niedermayer]

On Tue, Feb 20, 2018 at 04:44:13PM +, Rostislav Pehlivanov wrote:
> On 20 February 2018 at 16:30, Michael Niedermayer 
> 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  wrote:
> > > >
> > > >>> 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.
> > > >>
> > > >> It's true that after the deprecation period they will effectively be
> > the
> > > >> only ones still mutable, but shouldn't the objective be to cover them
> > all?
> > > >
> > > > That would be nice. But this has been discussed before. No kind of
> > > > different registration API could fix this issue either, unless we start
> > > > mallocing AVCodec structs and require the user to free them, or
> > > > something equally absurd. The proper solution for this specific issue
> > > > would be making a new API that somehow replaces AVCodec.pix_fmts.
> > > >
> > > >>>
> > > >>> Why are we discussing this _again_?
> > > >>
> > > >> Because a drawback has been found, namely that link time optimization
> > > >> using static libraries can't remove "non registered" modules anymore,
> > > >> and now depends fully on what's enabled at configure time.
> > > >> Ideally, a better registration based API that follows what i described
> > > >> above should be designed with care.
> > > >
> > > > Oh yeah, bikeshed attack by Michael. As it was said a few thousand
> > times
> > > > already, public API users could NOT use the registration stuff to
> > > > register only specific components at runtime. So they have to use the
> > > > register_all variants, which pull in _all_ components even with static
> > > > linking.
> > >
> > > Oh, i assumed it was possible to use avcodec_register() on a manual
> > > basis in a proper API usage scenario, but i see now that's not the case.
> >
> > of course its possible to use avcodec_register() on a manual basis in a
> > proper API usage scenario, why would it not be ?
> >
> > why do you think this function exists or was written ?
> >
> > is it the ff_* symbols you are thinking of ?
> > 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".
> >
> >
> > >
> > > Nevermind then, my argument was focused on preventing losing some link
> > > time optimization for static libraries assuming proper API usage.
> > >
> > > >
> > > > And that can't be made with dynamic linking either. If you statically
> > > > link anyway, you probably control everything, and you can configure
> > this
> > > > stuff at compile time.
> > > >
> > > > Then I guess there's this very special case where a fuzzer statically
> > > > links to libavcodec, and registers only a few components (technically
> > > > violating the API and by guessing the component symbol name), and
> > > > without calling the register_all functions. This would make the
> > > > resulting executable much smaller, which is apparently important
> > > > because Google (who runs the fuzzers for their oss-fuzz project) is too
> > > > poor (?) to host all the statically linked fuzzers, _or_ the whitelist
> > > > stuff is not enough to trick the fuzzer into not trying to fuzz the
> > > > wrong code. In addition, it's apparently infeasible to just build
> > > > every fuzzer with a separate libavcodec. (Not sure about the details,
> > it
> > > > was something like this.)
> > > >
> > > > Those requirements are really quite nebulous. I don't know why we even
> > > > should care - surely whoever does this will find a solution that works
> > > > for them. For example they could apply a small patch that makes the
> > > > codec_list[] symbol non-static non-const, which would allow them to
> > > > overwrite it (i.e. deleting unneeded entries). It's a really simple
> > > > solution that took me 5 minutes to come up with.
> > >
> > > Something like this is probably the best solution for the fuzzer issue,
> > yes.
> >
> > This basically says one should fork ffmpeg if one has problems with the
> > new API
> >
> > Thats a very chilling response to be honest in a discussion about that APIs
> > design. More so as this is at a time we still can change the API.
> >
> > and anyone who wants to only register a subset of components (due to
> > security)
> > has to either link to a seperate binary (which is  disallowed in some
> > major distributions which mandate shared libs 

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

[wm4]

On Tue, 20 Feb 2018 17:30:32 +0100
Michael Niedermayer  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  wrote:
> > >   
> > >>> 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.
> > >>
> > >> It's true that after the deprecation period they will effectively be the
> > >> only ones still mutable, but shouldn't the objective be to cover them 
> > >> all?  
> > > 
> > > That would be nice. But this has been discussed before. No kind of
> > > different registration API could fix this issue either, unless we start
> > > mallocing AVCodec structs and require the user to free them, or
> > > something equally absurd. The proper solution for this specific issue
> > > would be making a new API that somehow replaces AVCodec.pix_fmts.
> > >   
> > >>>
> > >>> Why are we discussing this _again_?
> > >>
> > >> Because a drawback has been found, namely that link time optimization
> > >> using static libraries can't remove "non registered" modules anymore,
> > >> and now depends fully on what's enabled at configure time.
> > >> Ideally, a better registration based API that follows what i described
> > >> above should be designed with care.  
> > > 
> > > Oh yeah, bikeshed attack by Michael. As it was said a few thousand times
> > > already, public API users could NOT use the registration stuff to
> > > register only specific components at runtime. So they have to use the
> > > register_all variants, which pull in _all_ components even with static
> > > linking.  
> > 
> > Oh, i assumed it was possible to use avcodec_register() on a manual
> > basis in a proper API usage scenario, but i see now that's not the case.  
> 
> of course its possible to use avcodec_register() on a manual basis in a 
> proper API usage scenario, why would it not be ?
> 
> why do you think this function exists or was written ?

I don't know, but it didn't make any sense. And wouldn't have made for
years to come.

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?

> 
> > 
> > Nevermind then, my argument was focused on preventing losing some link
> > time optimization for static libraries assuming proper API usage.
> >   
> > > 
> > > And that can't be made with dynamic linking either. If you statically
> > > link anyway, you probably control everything, and you can configure this
> > > stuff at compile time.
> > > 
> > > Then I guess there's this very special case where a fuzzer statically
> > > links to libavcodec, and registers only a few components (technically
> > > violating the API and by guessing the component symbol name), and
> > > without calling the register_all functions. This would make the
> > > resulting executable much smaller, which is apparently important
> > > because Google (who runs the fuzzers for their oss-fuzz project) is too
> > > poor (?) to host all the statically linked fuzzers, _or_ the whitelist
> > > stuff is not enough to trick the fuzzer into not trying to fuzz the
> > > wrong code. In addition, it's apparently infeasible to just build
> > > every fuzzer with a separate libavcodec. (Not sure about the details, it
> > > was something like this.)
> > > 
> > > Those requirements are really quite nebulous. I don't know why we even
> > > should care - surely whoever does this will find a solution that works
> > > for them. For example they could apply a small patch that makes the
> > > codec_list[] symbol non-static non-const, which would allow them to
> > > overwrite it (i.e. deleting unneeded entries). It's a really 

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

[James Almer]

On 2/20/2018 1:30 PM, Michael Niedermayer 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  wrote:
>>>
> 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.  

 It's true that after the deprecation period they will effectively be the
 only ones still mutable, but shouldn't the objective be to cover them all?
>>>
>>> That would be nice. But this has been discussed before. No kind of
>>> different registration API could fix this issue either, unless we start
>>> mallocing AVCodec structs and require the user to free them, or
>>> something equally absurd. The proper solution for this specific issue
>>> would be making a new API that somehow replaces AVCodec.pix_fmts.
>>>
>
> Why are we discussing this _again_?  

 Because a drawback has been found, namely that link time optimization
 using static libraries can't remove "non registered" modules anymore,
 and now depends fully on what's enabled at configure time.
 Ideally, a better registration based API that follows what i described
 above should be designed with care.
>>>
>>> Oh yeah, bikeshed attack by Michael. As it was said a few thousand times
>>> already, public API users could NOT use the registration stuff to
>>> register only specific components at runtime. So they have to use the
>>> register_all variants, which pull in _all_ components even with static
>>> linking.
>>
>> Oh, i assumed it was possible to use avcodec_register() on a manual
>> basis in a proper API usage scenario, but i see now that's not the case.
> 
> of course its possible to use avcodec_register() on a manual basis in a 
> proper API usage scenario, why would it not be ?
> 
> why do you think this function exists or was written ?
> 
> is it the ff_* symbols you are thinking of ?

Yes. You can't register individual AVCodecs using avcodec_register()
without those symbols, and in a normal API usage scenario, be it with
the old or the new iterate one, they are not available.

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

Hence me also being a proponent of a different, better registration
based API.
I think you misunderstood what i tried to say in the email you replied
to. You reported a drawback of this new API by giving the example of the
fuzzer not being able to discard unused codec symbols since all are
unconditionally referenced now. I assumed that the fuzzer was using the
API properly all this time, but turns out it's not, digging the
internals instead to register single codecs. This therefore can't be
considered a change in behavior compared to the current/old registration
based API.

Which brings me to the next part below.

> 
> 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".
> 
> 
>>
>> Nevermind then, my argument was focused on preventing losing some link
>> time optimization for static libraries assuming proper API usage.
>>
>>>
>>> And that can't be made with dynamic linking either. If you statically
>>> link anyway, you probably control everything, and you can configure this
>>> stuff at compile time.
>>>
>>> Then I guess there's this very special case where a fuzzer statically
>>> links to libavcodec, and registers only a few components (technically
>>> violating the API and by guessing the component symbol name), and
>>> without calling the register_all functions. This would make the
>>> resulting executable much smaller, which is apparently important
>>> because Google (who runs the fuzzers for their oss-fuzz project) is too
>>> poor (?) to host all the statically linked fuzzers, _or_ the whitelist
>>> stuff is not enough to trick the fuzzer into not trying to fuzz the
>>> wrong code. In addition, it's apparently infeasible to just build
>>> every fuzzer with a separate libavcodec. (Not sure about the details, it
>>> was something like this.)
>>>
>>> Those requirements are really quite nebulous. I don't know why we even
>>> should care - surely whoever does this will find a solution that works
>>> for them. For example they could apply a small patch that makes the
>>> codec_list[] symbol non-static non-const, which would allow them to
>>> overwrite it (i.e. deleting unneeded entries). It's a really simple
>>> solution that took me 5 minutes to come up with.
>>
>> Something like this is probably the best solution for the fuzzer issue, yes.
> 
> This basically says one should fork ffmpeg 

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

[Rostislav Pehlivanov]

On 20 February 2018 at 16:30, Michael Niedermayer 
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  wrote:
> > >
> > >>> 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.
> > >>
> > >> It's true that after the deprecation period they will effectively be
> the
> > >> only ones still mutable, but shouldn't the objective be to cover them
> all?
> > >
> > > That would be nice. But this has been discussed before. No kind of
> > > different registration API could fix this issue either, unless we start
> > > mallocing AVCodec structs and require the user to free them, or
> > > something equally absurd. The proper solution for this specific issue
> > > would be making a new API that somehow replaces AVCodec.pix_fmts.
> > >
> > >>>
> > >>> Why are we discussing this _again_?
> > >>
> > >> Because a drawback has been found, namely that link time optimization
> > >> using static libraries can't remove "non registered" modules anymore,
> > >> and now depends fully on what's enabled at configure time.
> > >> Ideally, a better registration based API that follows what i described
> > >> above should be designed with care.
> > >
> > > Oh yeah, bikeshed attack by Michael. As it was said a few thousand
> times
> > > already, public API users could NOT use the registration stuff to
> > > register only specific components at runtime. So they have to use the
> > > register_all variants, which pull in _all_ components even with static
> > > linking.
> >
> > Oh, i assumed it was possible to use avcodec_register() on a manual
> > basis in a proper API usage scenario, but i see now that's not the case.
>
> of course its possible to use avcodec_register() on a manual basis in a
> proper API usage scenario, why would it not be ?
>
> why do you think this function exists or was written ?
>
> is it the ff_* symbols you are thinking of ?
> 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".
>
>
> >
> > Nevermind then, my argument was focused on preventing losing some link
> > time optimization for static libraries assuming proper API usage.
> >
> > >
> > > And that can't be made with dynamic linking either. If you statically
> > > link anyway, you probably control everything, and you can configure
> this
> > > stuff at compile time.
> > >
> > > Then I guess there's this very special case where a fuzzer statically
> > > links to libavcodec, and registers only a few components (technically
> > > violating the API and by guessing the component symbol name), and
> > > without calling the register_all functions. This would make the
> > > resulting executable much smaller, which is apparently important
> > > because Google (who runs the fuzzers for their oss-fuzz project) is too
> > > poor (?) to host all the statically linked fuzzers, _or_ the whitelist
> > > stuff is not enough to trick the fuzzer into not trying to fuzz the
> > > wrong code. In addition, it's apparently infeasible to just build
> > > every fuzzer with a separate libavcodec. (Not sure about the details,
> it
> > > was something like this.)
> > >
> > > Those requirements are really quite nebulous. I don't know why we even
> > > should care - surely whoever does this will find a solution that works
> > > for them. For example they could apply a small patch that makes the
> > > codec_list[] symbol non-static non-const, which would allow them to
> > > overwrite it (i.e. deleting unneeded entries). It's a really simple
> > > solution that took me 5 minutes to come up with.
> >
> > Something like this is probably the best solution for the fuzzer issue,
> yes.
>
> This basically says one should fork ffmpeg if one has problems with the
> new API
>
> Thats a very chilling response to be honest in a discussion about that APIs
> design. More so as this is at a time we still can change the API.
>
> and anyone who wants to only register a subset of components (due to
> security)
> has to either link to a seperate binary (which is  disallowed in some
> major distributions which mandate shared libs without exception IIRC so
> that
> sounds great but simple isnt possible)
> or just not use FFmpeg or fork it or use a fork or just forget about only
> registering a subset.
>

You bring up the security argument but I don't see anything here which
would compromise it. _All_ the 

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

[Michael Niedermayer]

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  wrote:
> > 
> >>> 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.  
> >>
> >> It's true that after the deprecation period they will effectively be the
> >> only ones still mutable, but shouldn't the objective be to cover them all?
> > 
> > That would be nice. But this has been discussed before. No kind of
> > different registration API could fix this issue either, unless we start
> > mallocing AVCodec structs and require the user to free them, or
> > something equally absurd. The proper solution for this specific issue
> > would be making a new API that somehow replaces AVCodec.pix_fmts.
> > 
> >>>
> >>> Why are we discussing this _again_?  
> >>
> >> Because a drawback has been found, namely that link time optimization
> >> using static libraries can't remove "non registered" modules anymore,
> >> and now depends fully on what's enabled at configure time.
> >> Ideally, a better registration based API that follows what i described
> >> above should be designed with care.
> > 
> > Oh yeah, bikeshed attack by Michael. As it was said a few thousand times
> > already, public API users could NOT use the registration stuff to
> > register only specific components at runtime. So they have to use the
> > register_all variants, which pull in _all_ components even with static
> > linking.
> 
> Oh, i assumed it was possible to use avcodec_register() on a manual
> basis in a proper API usage scenario, but i see now that's not the case.

of course its possible to use avcodec_register() on a manual basis in a 
proper API usage scenario, why would it not be ?

why do you think this function exists or was written ?

is it the ff_* symbols you are thinking of ?
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".


> 
> Nevermind then, my argument was focused on preventing losing some link
> time optimization for static libraries assuming proper API usage.
> 
> > 
> > And that can't be made with dynamic linking either. If you statically
> > link anyway, you probably control everything, and you can configure this
> > stuff at compile time.
> > 
> > Then I guess there's this very special case where a fuzzer statically
> > links to libavcodec, and registers only a few components (technically
> > violating the API and by guessing the component symbol name), and
> > without calling the register_all functions. This would make the
> > resulting executable much smaller, which is apparently important
> > because Google (who runs the fuzzers for their oss-fuzz project) is too
> > poor (?) to host all the statically linked fuzzers, _or_ the whitelist
> > stuff is not enough to trick the fuzzer into not trying to fuzz the
> > wrong code. In addition, it's apparently infeasible to just build
> > every fuzzer with a separate libavcodec. (Not sure about the details, it
> > was something like this.)
> > 
> > Those requirements are really quite nebulous. I don't know why we even
> > should care - surely whoever does this will find a solution that works
> > for them. For example they could apply a small patch that makes the
> > codec_list[] symbol non-static non-const, which would allow them to
> > overwrite it (i.e. deleting unneeded entries). It's a really simple
> > solution that took me 5 minutes to come up with.
> 
> Something like this is probably the best solution for the fuzzer issue, yes.

This basically says one should fork ffmpeg if one has problems with the new API

Thats a very chilling response to be honest in a discussion about that APIs
design. More so as this is at a time we still can change the API.

and anyone who wants to only register a subset of components (due to security)
has to either link to a seperate binary (which is  disallowed in some 
major distributions which mandate shared libs without exception IIRC so that
sounds great but simple isnt possible)
or just not use FFmpeg or fork it or use a fork or just forget about only
registering a subset.

IMO, an API that allows registering subsets of components would be wiser
for FFmpeg.

Of course we can go with a API that doesnt allow subset registeration but
then later we need to add an API to seperatly register user componentgs 
(plugins),
so 2 APIs and our fuzzer needs to patch that API, and browsers which may only 
want to register a subset of codecs & formats 

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

[James Almer]

On 2/20/2018 9:21 AM, wm4 wrote:
> On Tue, 20 Feb 2018 08:47:51 -0300
> James Almer  wrote:
> 
>>> 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.  
>>
>> It's true that after the deprecation period they will effectively be the
>> only ones still mutable, but shouldn't the objective be to cover them all?
> 
> That would be nice. But this has been discussed before. No kind of
> different registration API could fix this issue either, unless we start
> mallocing AVCodec structs and require the user to free them, or
> something equally absurd. The proper solution for this specific issue
> would be making a new API that somehow replaces AVCodec.pix_fmts.
> 
>>>
>>> Why are we discussing this _again_?  
>>
>> Because a drawback has been found, namely that link time optimization
>> using static libraries can't remove "non registered" modules anymore,
>> and now depends fully on what's enabled at configure time.
>> Ideally, a better registration based API that follows what i described
>> above should be designed with care.
> 
> Oh yeah, bikeshed attack by Michael. As it was said a few thousand times
> already, public API users could NOT use the registration stuff to
> register only specific components at runtime. So they have to use the
> register_all variants, which pull in _all_ components even with static
> linking.

Oh, i assumed it was possible to use avcodec_register() on a manual
basis in a proper API usage scenario, but i see now that's not the case.

Nevermind then, my argument was focused on preventing losing some link
time optimization for static libraries assuming proper API usage.

> 
> And that can't be made with dynamic linking either. If you statically
> link anyway, you probably control everything, and you can configure this
> stuff at compile time.
> 
> Then I guess there's this very special case where a fuzzer statically
> links to libavcodec, and registers only a few components (technically
> violating the API and by guessing the component symbol name), and
> without calling the register_all functions. This would make the
> resulting executable much smaller, which is apparently important
> because Google (who runs the fuzzers for their oss-fuzz project) is too
> poor (?) to host all the statically linked fuzzers, _or_ the whitelist
> stuff is not enough to trick the fuzzer into not trying to fuzz the
> wrong code. In addition, it's apparently infeasible to just build
> every fuzzer with a separate libavcodec. (Not sure about the details, it
> was something like this.)
> 
> Those requirements are really quite nebulous. I don't know why we even
> should care - surely whoever does this will find a solution that works
> for them. For example they could apply a small patch that makes the
> codec_list[] symbol non-static non-const, which would allow them to
> overwrite it (i.e. deleting unneeded entries). It's a really simple
> solution that took me 5 minutes to come up with.

Something like this is probably the best solution for the fuzzer issue, yes.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[wm4]

On Tue, 20 Feb 2018 08:47:51 -0300
James Almer  wrote:

> > 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.  
> 
> It's true that after the deprecation period they will effectively be the
> only ones still mutable, but shouldn't the objective be to cover them all?

That would be nice. But this has been discussed before. No kind of
different registration API could fix this issue either, unless we start
mallocing AVCodec structs and require the user to free them, or
something equally absurd. The proper solution for this specific issue
would be making a new API that somehow replaces AVCodec.pix_fmts.

> > 
> > Why are we discussing this _again_?  
> 
> Because a drawback has been found, namely that link time optimization
> using static libraries can't remove "non registered" modules anymore,
> and now depends fully on what's enabled at configure time.
> Ideally, a better registration based API that follows what i described
> above should be designed with care.

Oh yeah, bikeshed attack by Michael. As it was said a few thousand times
already, public API users could NOT use the registration stuff to
register only specific components at runtime. So they have to use the
register_all variants, which pull in _all_ components even with static
linking.

And that can't be made with dynamic linking either. If you statically
link anyway, you probably control everything, and you can configure this
stuff at compile time.

Then I guess there's this very special case where a fuzzer statically
links to libavcodec, and registers only a few components (technically
violating the API and by guessing the component symbol name), and
without calling the register_all functions. This would make the
resulting executable much smaller, which is apparently important
because Google (who runs the fuzzers for their oss-fuzz project) is too
poor (?) to host all the statically linked fuzzers, _or_ the whitelist
stuff is not enough to trick the fuzzer into not trying to fuzz the
wrong code. In addition, it's apparently infeasible to just build
every fuzzer with a separate libavcodec. (Not sure about the details, it
was something like this.)

Those requirements are really quite nebulous. I don't know why we even
should care - surely whoever does this will find a solution that works
for them. For example they could apply a small patch that makes the
codec_list[] symbol non-static non-const, which would allow them to
overwrite it (i.e. deleting unneeded entries). It's a really simple
solution that took me 5 minutes to come up with.

The security argument is absurd, because you'd just disable them at
compile time, and if you really want to restrict components at compile
time, there's this whitelist stuff for codecs and a few other things.

I feel like I've written this post 3 trillion times, and Michael never
read any of them.

> >   
> >> 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.  
> 
> What makes me uneasy is that if this API remains is place, introducing a
> new better one would be annoying for downstream users if it's added too
> close to the end of the old API's deprecation period, be it before or
> after. Having to migrate a second time in a short time frame is undesirable.
> 
> In any case, just my two cents. Don't take this as a blocker for this
> set, but as a request to consider better alternatives in the short or
> long term.

Like what? What color is it going to be?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[James Almer]

On 2/20/2018 3:24 AM, wm4 wrote:
> On Mon, 19 Feb 2018 23:30:24 -0300
> James Almer  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
  wrote:  
> On Sun, Feb 18, 2018 at 05:58:16PM +, 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.

It's true that after the deprecation period they will effectively be the
only ones still mutable, but shouldn't the objective be to cover them all?

> 
> Why are we discussing this _again_?

Because a drawback has been found, namely that link time optimization
using static libraries can't remove "non registered" modules anymore,
and now depends fully on what's enabled at configure time.
Ideally, a better registration based API that follows what i described
above should be designed with care.

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

What makes me uneasy is that if this API remains is place, introducing a
new better one would be annoying for downstream users if it's added too
close to the end of the old API's deprecation period, be it before or
after. Having to migrate a second time in a short time frame is undesirable.

In any case, just my two cents. Don't take this as a blocker for this
set, but as a request to consider better alternatives in the short or
long term.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[wm4]

On Mon, 19 Feb 2018 23:30:24 -0300
James Almer  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
> >>  wrote:  
> >>> On Sun, Feb 18, 2018 at 05:58:16PM +, 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.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[James Almer]

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
>>  wrote:
>>> On Sun, Feb 18, 2018 at 05:58:16PM +, 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.

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.

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.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[Michael Niedermayer]

On Mon, Feb 19, 2018 at 09:57:35PM +0100, Hendrik Leppkes wrote:
> On Mon, Feb 19, 2018 at 8:30 PM, Michael Niedermayer
>  wrote:
> > On Sun, Feb 18, 2018 at 05:58:16PM +, 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 ...

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

It is what and why we do it that matters, not just one of them.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[Hendrik Leppkes]

On Mon, Feb 19, 2018 at 8:30 PM, Michael Niedermayer
 wrote:
> On Sun, Feb 18, 2018 at 05:58:16PM +, 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)
That was only something some internal tools used, and you can probably
find different options for dealing with those.

- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[Michael Niedermayer]

On Sun, Feb 18, 2018 at 05:58:16PM +, 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.

An example of this effect for codecs is tools/target_dec_fuzzer.c

This effect results because when the linker collects all objects
of all static libs with a user application it can remove all symbols
which are not referenced.
If there is a single array that references all codecs, filters, formats
and this array is referenced by a function which must be used then
nothing can be removed at the link stage

OTOH with a register function a register all which refernces all
a user application can simply not use the reference all code and register
the specific subset it needs with individual register calls.
In this case the linker can ommit the register all and all codecs, formats
and filters teh user application does not explicitly refer too.
In the case where this applies this results in significantly smaller files

I also expect they link faster and load faster but i forgot to benchmark this
when i realized this issue exists and tested ...

I am thus in favor of a system that does not unconditionally reference every
codec/format/filter

There can also be an advantage security wise if unneeded parts are never
"registered"

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

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel