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 <jamr...@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 ? > > 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 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. > > Again, you misunderstood me. Currently the fuzzer tool is abusing the > API by digging its internal bits in order to register single codecs. wm4 > suggestion, which i adhered to, was to change the way the tool digs the > internals to recover the expected unused symbol stripping behavior. > > Nothing in my previous email suggested that I'm against a better > registration based API. I'm still in favor of one since it's a lot more > powerful and versatile than this fixed, configure time constant codec > list recently introduced. I simply conceded that the argument about the > fuzzer not stripping symbols with the iterate API was not a valid one to > discredit the latter. It is in fact a good reason/example of why a > proper registration based API should be introduced instead. ok, it seems i misunderstood you > > > > > 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. > > I agree. I said as much in a previous email. And that we should decide > if we want to keep this iterate API or not, and soon. > At no point i changed my mind about supporting the idea of a better > registration based API. > > > > > 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 ? > > We don't have to. But the two people willing to write a new API didn't > want to design it to allow runtime registration. Yes but this was before some of the disadvantages of that where fully understood (at least by me, as i was a bit caught by surprise that the fuzzer binaries grew though its very logic in retrospect) Are the people working on this still against runtime registration? thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Does the universe only have a finite lifespan? No, its going to go on forever, its just that you wont like living in it. -- Hiranya Peiri
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
