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

Michael Niedermayer michael at niedermayer.cc
Tue Feb 20 18:30:32 EET 2018


On Tue, Feb 20, 2018 at 10:17:02AM -0300, James Almer wrote:
> On 2/20/2018 9:21 AM, wm4 wrote:
> > On Tue, 20 Feb 2018 08:47:51 -0300
> > James Almer <jamrial at gmail.com> wrote:
> > 
> >>> 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 are screwed.

Seriously, why should we do it this way ?
An API that allows registering subsets is not very complex

Thanks

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

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180220/85634048/attachment.sig>


More information about the ffmpeg-devel mailing list