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

[Michael Niedermayer]

On Sat, Dec 23, 2017 at 01:27:53PM +0000, Josh de Kock wrote:
> This is the first patch to add the new API for iterating items within the libraries to lavc, this completes lavc's support for this API (BSFs were already using it).
> 
> I'm currently working on a similar patch for lavfi, lavf and lavd. Note that I'm not entirely sure how to properly deprecate stuff so if that's incorrect some advice would be appreciated.
> 
> -- 
> Josh de Kock <josh at itanimul.li>

>  configure              |   14 
>  libavcodec/allcodecs.c | 1472 ++++++++++++++++++++++++++++---------------------
>  libavcodec/avcodec.h   |   25 
>  libavcodec/parser.c    |   86 ++
>  libavcodec/utils.c     |  105 ---
>  5 files changed, 962 insertions(+), 740 deletions(-)
> 1d29484b82a8f3aaa4689f316f20819ebe3ff8a6  0001-lavc-add-new-API-for-iterating-codecs-and-codec-pars.patch
> From 1d84641556eea563b82b17a6ffe54226e0e31c4e Mon Sep 17 00:00:00 2001
> From: Josh de Kock <josh at itanimul.li>
> Date: Fri, 22 Dec 2017 22:17:00 +0000
> Subject: [PATCH] lavc: add new API for iterating codecs and codec parsers
> 
> Also replace linked list with an array.
> ---
>  configure              |   14 +-
>  libavcodec/allcodecs.c | 1472 ++++++++++++++++++++++++++++--------------------
>  libavcodec/avcodec.h   |   25 +
>  libavcodec/parser.c    |   86 ++-
>  libavcodec/utils.c     |  105 ----
>  5 files changed, 962 insertions(+), 740 deletions(-)
[...]

> +static void av_codec_init_next(void)
> +{
> +    AVCodec *prev = NULL, *p;
> +    void *i = 0;
> +    while ((p = (AVCodec*)av_codec_iterate(&i))) {
> +        if (prev)
> +            prev->next = p;
> +        prev = p;
>      }
> +    prev->next = NULL;
> +}
>  
> -#define REGISTER_DECODER(X, x)                                          \
> -    {                                                                   \
> -        extern AVCodec ff_##x##_decoder;                                \
> -        if (CONFIG_##X##_DECODER)                                       \
> -            avcodec_register(&ff_##x##_decoder);                        \

> +av_cold void avcodec_register(AVCodec *codec)
> +{
> +#if CONFIG_ME_CMP
> +    pthread_once(&ff_me_cmp_static_init, ff_me_cmp_init_static);
> +#endif
> +
> +    pthread_once(&av_codec_next_init, av_codec_init_next);
> +    
> +    if (codec->init_static_data)
> +        codec->init_static_data(codec);
> +}
> +

This would not register the provided codec. You have to still register it
in case its a AVCodec provided by the user (app).

The commit message does not say that registering user provided codecs is
removed. This is unacceptable. You cannot silently remove this.

If you mean to lock libavcodec down so that registration of codecs is no
longer possible, this has to be made clear in the commit message.

I do in fact not agree to remove this as i belive its moving us away from
the spirit of open source. But in case im in the minority at least what is
done must be clearly stated in the commit message.
Its untrue that this is just adding a new API

Now changing the linked list to an array (which your patch does half of) is
probably a good idea.
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


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

Thanks

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Never trust a computer, one day, it may think you are the virus. -- Compn
-------------- 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/ccbf0258/attachment.sig>