[FFmpeg-devel] [PATCH] lavc: add new API for iterating codecs and codec parsers

Michael Niedermayer michael at niedermayer.cc
Sun Dec 24 13:43:27 EET 2017


On Sun, Dec 24, 2017 at 02:31:16AM +0100, wm4 wrote:
> On Sun, 24 Dec 2017 02:06:40 +0100
> Michael Niedermayer <michael at niedermayer.cc> wrote:
> 
> > If you and others agree we can also easily maintain support for user apps
> > to register codecs. The only thing needed is to make the array bigger and
> > add codecs which arent ours in there as long as there is space.
> > doing this would be very easy, a atomic integer pointing to the begin of
> > the free space which is atomically increased for each register is all that
> > is needed
> 
> Not sure how often I repeated this (specifically for you), but:

why are you so upset ? We have different oppinions here or we misunderstand
each other.


> 
> - users can't provide AVCodec, because it would require accessing
>   internal API and private ABI unstable fields

That is expected without there being an effort to design a public API
for external codecs.

Point being, this doesnt matter.
One can still maintain an external codec register it and use it.
It would need to be rebuild for each libavcodec version and would
occasionally need to be changed like any internal codec.
But it would not be harder than maintaining a codec inside ffmpeg.

Especially if one targets a release like 3.4.x it would be close to 0
work to have a external codec that is registered compared to having it
internal


> - if we provide the ability to add user codecs (which I'm not
>   fundamentally against), then it should not modify global state.
>   Having named user codecs in a process wide list would definitely lead
>   to name clashes or other conflicts

I dont see how you could do this without modifying global state.
You would need some kind of context for all codec "lookup" functions

Such context could be added and iam not against this at all, but i feel
this is fixing a problem noone has ever had. registering codecs was
supported since forever and ive never heared of a conflict like you describe


> - also, we would have to provide stable/public API for implementing
>   codecs in the first place (all AVCodec callback fields are private),
>   and we're nowhere near the state of adding it

I would love to see such a API and i would certainly contribute but
as explained above, we dont have it and we dont really need it for
registering user codec to be usefull in practice IMO


> - dropping avcodec_register() is definitely not the worst blocker for
>   user codecs - it's very, very far from it, because once we have
>   fixed the things above, we can just add a new public API for
>   registering (which would have to have a different function signature
>   to avoid global mutable lists). So I don't know why you complain.

Like elaborated above, it is the main problem.
Currently external codecs are possible for all i know. And i have
seen it used a few years ago, so it did work at some point in real
applications.


> - these points are technical, not ideological
> 
> Can you point out any user application which registers its own codecs
> (and this violates the API)?

I do not remember which application it was but i did a few years ago review
all user apps using libavcodec / libavformat for things we could merge.
I do remember seeing some that added codecs.
It was some code that was very specific to the app or platform IIRC

There are more usecases. For example codec optimizations for niche
codecs where rejected on ffmpeg-devel.
I find it unfriendly from us if we reject improvments because they
arent in a area most of us care and then also drop the possibility
for externally maintaining and registering code without doing a
full fork (which is much more work to maintain)

> 
> > > +AVCodec *av_codec_next(const AVCodec *c)
> > > +{
> > > +    pthread_once(&av_codec_next_init, av_codec_init_next);
> > > +
> > > +    if (c)
> > > +        return c->next;  
> > 
> > AVCodec->next should be removed as it makes the structs non constant
> 
> That has to happen after a deprecation phase, unless you have an idea
> how to make av_codec_next() not O(n^2) without this.

That can be done and there are many ways. Not saying we want to do it
first possibility, one can add a int "constant" to each AVCodec.
configure would have to choose these or there would be holes in the array.
a hashtable with a mutex is an option too but that seems not worth the
complexity / work
the 3rd obvious option is to make "next" a pointer to a int which is
initialized in each codec to some static int. Which can then be set
to the array index, the main struct can then remain in read only memory
iam pretty sure there are many more options

PS: merry Xmas!

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship: All citizens are under surveillance, all their steps and
actions recorded, for the politicians to enforce control.
Democracy: All politicians are under surveillance, all their steps and
actions recorded, for the citizens to enforce control.
-------------- 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/20171224/9d4945da/attachment.sig>


More information about the ffmpeg-devel mailing list