[FFmpeg-devel] avcodec: remove av_codec_init_static()

Submitted by Muhammad Faiz on Feb. 12, 2018, 5:42 a.m.

Details

Message ID 20180212054210.21183-1-mfcc64@gmail.com
State New
Headers show

Commit Message

Muhammad Faiz Feb. 12, 2018, 5:42 a.m.
Modify the behavior of init_static_data().

Signed-off-by: Muhammad Faiz <mfcc64@gmail.com>
---
 libavcodec/allcodecs.c | 16 ++++------------
 libavcodec/avcodec.h   |  4 +++-
 libavcodec/libvpxdec.c | 15 ++++++++++++++-
 libavcodec/libvpxenc.c | 15 ++++++++++++++-
 libavcodec/libx264.c   | 11 ++++++++++-
 libavcodec/libx265.c   | 11 ++++++++++-
 6 files changed, 55 insertions(+), 17 deletions(-)

Comments

James Almer Feb. 12, 2018, 7:40 p.m.
On 2/12/2018 2:42 AM, Muhammad Faiz wrote:
> Modify the behavior of init_static_data().
> 
> Signed-off-by: Muhammad Faiz <mfcc64@gmail.com>
> ---
>  libavcodec/allcodecs.c | 16 ++++------------
>  libavcodec/avcodec.h   |  4 +++-
>  libavcodec/libvpxdec.c | 15 ++++++++++++++-
>  libavcodec/libvpxenc.c | 15 ++++++++++++++-
>  libavcodec/libx264.c   | 11 ++++++++++-
>  libavcodec/libx265.c   | 11 ++++++++++-
>  6 files changed, 55 insertions(+), 17 deletions(-)
> 
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index 774b78ef09..02910b5594 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -757,24 +757,16 @@ extern AVCodec ff_vp9_vaapi_encoder;
>  
>  #include "libavcodec/codec_list.c"
>  
> -static AVOnce av_codec_static_init = AV_ONCE_INIT;
> -static void av_codec_init_static(void)
> -{
> -    for (int i = 0; codec_list[i]; i++) {
> -        if (codec_list[i]->init_static_data)
> -            codec_list[i]->init_static_data((AVCodec*)codec_list[i]);
> -    }
> -}
> -
>  const AVCodec *av_codec_iterate(void **opaque)
>  {
>      uintptr_t i = (uintptr_t)*opaque;
>      const AVCodec *c = codec_list[i];
>  
> -    ff_thread_once(&av_codec_static_init, av_codec_init_static);
> -
> -    if (c)
> +    if (c) {
> +        if (c->init_static_data)
> +            c->init_static_data();
>          *opaque = (void*)(i + 1);
> +    }
>  
>      return c;
>  }
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index ad0b48a839..d89bf300fc 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -3443,8 +3443,10 @@ typedef struct AVCodec {
>       *
>       * This is not intended for time consuming operations as it is
>       * run for every codec regardless of that codec being used.
> +     * This may be called multiple times from different threads, the callee
> +     * has responsibility for thread synchronization.
>       */
> -    void (*init_static_data)(struct AVCodec *codec);
> +    void (*init_static_data)(void);

What's the benefit of removing the parameter?

>  
>      int (*init)(AVCodecContext *);
>      int (*encode_sub)(AVCodecContext *, uint8_t *buf, int buf_size,
> diff --git a/libavcodec/libvpxdec.c b/libavcodec/libvpxdec.c
> index 04f27d3396..f2003b836b 100644
> --- a/libavcodec/libvpxdec.c
> +++ b/libavcodec/libvpxdec.c
> @@ -30,6 +30,7 @@
>  #include "libavutil/common.h"
>  #include "libavutil/imgutils.h"
>  #include "libavutil/intreadwrite.h"
> +#include "libavutil/thread.h"
>  #include "avcodec.h"
>  #include "internal.h"
>  #include "libvpx.h"
> @@ -299,6 +300,18 @@ static av_cold int vp9_init(AVCodecContext *avctx)
>      return vpx_init(avctx, &vpx_codec_vp9_dx_algo, 0);
>  }
>  
> +static av_cold void vp9_init_static_once(void)
> +{
> +    extern AVCodec ff_libvpx_vp9_decoder;
> +    ff_vp9_init_static(&ff_libvpx_vp9_decoder);
> +}
> +
> +static av_cold void vp9_init_static(void)
> +{
> +    static AVOnce once = AV_ONCE_INIT;
> +    ff_thread_once(&once, vp9_init_static_once);
> +}
> +
>  AVCodec ff_libvpx_vp9_decoder = {
>      .name           = "libvpx-vp9",
>      .long_name      = NULL_IF_CONFIG_SMALL("libvpx VP9"),
> @@ -309,7 +322,7 @@ AVCodec ff_libvpx_vp9_decoder = {
>      .close          = vpx_free,
>      .decode         = vpx_decode,
>      .capabilities   = AV_CODEC_CAP_AUTO_THREADS | AV_CODEC_CAP_DR1,
> -    .init_static_data = ff_vp9_init_static,
> +    .init_static_data = vp9_init_static,

I think you can remove the init_static_data call to ff_vp9_init_static
from the decoder altogether. It just sets AVCodec.pix_fmts, which afaics
is only needed for the encoder.

Doing it would also let us get rid of libvpx.c, as everything there can
then be moved into libvpxenc.c

>      .profiles       = NULL_IF_CONFIG_SMALL(ff_vp9_profiles),
>      .wrapper_name   = "libvpx",
>  };
> diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
> index d0bd1e997a..086dd5defa 100644
> --- a/libavcodec/libvpxenc.c
> +++ b/libavcodec/libvpxenc.c
> @@ -39,6 +39,7 @@
>  #include "libavutil/intreadwrite.h"
>  #include "libavutil/mathematics.h"
>  #include "libavutil/opt.h"
> +#include "libavutil/thread.h"
>  
>  /**
>   * Portion of struct vpx_codec_cx_pkt from vpx_encoder.h.
> @@ -1209,6 +1210,18 @@ static av_cold int vp9_init(AVCodecContext *avctx)
>      return vpx_init(avctx, vpx_codec_vp9_cx());
>  }
>  
> +static av_cold void vp9_init_static_once(void)
> +{
> +    extern AVCodec ff_libvpx_vp9_encoder;
> +    ff_vp9_init_static(&ff_libvpx_vp9_encoder);
> +}
> +
> +static av_cold void vp9_init_static(void)
> +{
> +    static AVOnce once = AV_ONCE_INIT;
> +    ff_thread_once(&once, vp9_init_static_once);
> +}
> +
>  static const AVClass class_vp9 = {
>      .class_name = "libvpx-vp9 encoder",
>      .item_name  = av_default_item_name,
> @@ -1229,7 +1242,7 @@ AVCodec ff_libvpx_vp9_encoder = {
>      .profiles       = NULL_IF_CONFIG_SMALL(ff_vp9_profiles),
>      .priv_class     = &class_vp9,
>      .defaults       = defaults,
> -    .init_static_data = ff_vp9_init_static,
> +    .init_static_data = vp9_init_static,
>      .wrapper_name   = "libvpx",
>  };
>  #endif /* CONFIG_LIBVPX_VP9_ENCODER */
> diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
> index 12379ff763..0da61a0fcd 100644
> --- a/libavcodec/libx264.c
> +++ b/libavcodec/libx264.c
> @@ -26,6 +26,7 @@
>  #include "libavutil/pixdesc.h"
>  #include "libavutil/stereo3d.h"
>  #include "libavutil/intreadwrite.h"
> +#include "libavutil/thread.h"
>  #include "avcodec.h"
>  #include "internal.h"
>  
> @@ -896,8 +897,10 @@ static const enum AVPixelFormat pix_fmts_8bit_rgb[] = {
>  };
>  #endif
>  
> -static av_cold void X264_init_static(AVCodec *codec)
> +static av_cold void X264_init_static_once(void)
>  {
> +    extern AVCodec ff_libx264_encoder;
> +    AVCodec *codec = &ff_libx264_encoder;
>  #if X264_BUILD < 153
>      if (x264_bit_depth == 8)
>          codec->pix_fmts = pix_fmts_8bit;
> @@ -910,6 +913,12 @@ static av_cold void X264_init_static(AVCodec *codec)
>  #endif
>  }
>  
> +static av_cold void X264_init_static(void)
> +{
> +    static AVOnce once = AV_ONCE_INIT;
> +    ff_thread_once(&once, X264_init_static_once);
> +}
> +
>  #define OFFSET(x) offsetof(X264Context, x)
>  #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
>  static const AVOption options[] = {
> diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
> index 3c97800ccb..63e1240473 100644
> --- a/libavcodec/libx265.c
> +++ b/libavcodec/libx265.c
> @@ -31,6 +31,7 @@
>  #include "libavutil/common.h"
>  #include "libavutil/opt.h"
>  #include "libavutil/pixdesc.h"
> +#include "libavutil/thread.h"
>  #include "avcodec.h"
>  #include "internal.h"
>  
> @@ -395,8 +396,10 @@ static const enum AVPixelFormat x265_csp_twelve[] = {
>      AV_PIX_FMT_NONE
>  };
>  
> -static av_cold void libx265_encode_init_csp(AVCodec *codec)
> +static av_cold void libx265_encode_init_csp_once(void)
>  {
> +    extern AVCodec ff_libx265_encoder;
> +    AVCodec *codec = &ff_libx265_encoder;
>      if (x265_api_get(12))
>          codec->pix_fmts = x265_csp_twelve;
>      else if (x265_api_get(10))
> @@ -405,6 +408,12 @@ static av_cold void libx265_encode_init_csp(AVCodec *codec)
>          codec->pix_fmts = x265_csp_eight;
>  }
>  
> +static av_cold void libx265_encode_init_csp(void)
> +{
> +    static AVOnce once = AV_ONCE_INIT;
> +    ff_thread_once(&once, libx265_encode_init_csp_once);
> +}
> +
>  #define OFFSET(x) offsetof(libx265Context, x)
>  #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
>  static const AVOption options[] = {
>
wm4 Feb. 12, 2018, 8:57 p.m.
On Mon, 12 Feb 2018 12:42:10 +0700
Muhammad Faiz <mfcc64@gmail.com> wrote:

> Modify the behavior of init_static_data().
> 
> Signed-off-by: Muhammad Faiz <mfcc64@gmail.com>
> ---

Seems OK, but I'm also not sure about the benefit. The fundamental
problem that these codecs need to mutate AVCodec before the users sees
it won't go away.
Muhammad Faiz Feb. 14, 2018, 12:05 a.m.
On Tue, Feb 13, 2018 at 2:40 AM, James Almer <jamrial@gmail.com> wrote:
> On 2/12/2018 2:42 AM, Muhammad Faiz wrote:
>> Modify the behavior of init_static_data().
>>
>> Signed-off-by: Muhammad Faiz <mfcc64@gmail.com>
>> ---
>>  libavcodec/allcodecs.c | 16 ++++------------
>>  libavcodec/avcodec.h   |  4 +++-
>>  libavcodec/libvpxdec.c | 15 ++++++++++++++-
>>  libavcodec/libvpxenc.c | 15 ++++++++++++++-
>>  libavcodec/libx264.c   | 11 ++++++++++-
>>  libavcodec/libx265.c   | 11 ++++++++++-
>>  6 files changed, 55 insertions(+), 17 deletions(-)
>>
>> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
>> index 774b78ef09..02910b5594 100644
>> --- a/libavcodec/allcodecs.c
>> +++ b/libavcodec/allcodecs.c
>> @@ -757,24 +757,16 @@ extern AVCodec ff_vp9_vaapi_encoder;
>>
>>  #include "libavcodec/codec_list.c"
>>
>> -static AVOnce av_codec_static_init = AV_ONCE_INIT;
>> -static void av_codec_init_static(void)
>> -{
>> -    for (int i = 0; codec_list[i]; i++) {
>> -        if (codec_list[i]->init_static_data)
>> -            codec_list[i]->init_static_data((AVCodec*)codec_list[i]);
>> -    }
>> -}
>> -
>>  const AVCodec *av_codec_iterate(void **opaque)
>>  {
>>      uintptr_t i = (uintptr_t)*opaque;
>>      const AVCodec *c = codec_list[i];
>>
>> -    ff_thread_once(&av_codec_static_init, av_codec_init_static);
>> -
>> -    if (c)
>> +    if (c) {
>> +        if (c->init_static_data)
>> +            c->init_static_data();
>>          *opaque = (void*)(i + 1);
>> +    }
>>
>>      return c;
>>  }
>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>> index ad0b48a839..d89bf300fc 100644
>> --- a/libavcodec/avcodec.h
>> +++ b/libavcodec/avcodec.h
>> @@ -3443,8 +3443,10 @@ typedef struct AVCodec {
>>       *
>>       * This is not intended for time consuming operations as it is
>>       * run for every codec regardless of that codec being used.
>> +     * This may be called multiple times from different threads, the callee
>> +     * has responsibility for thread synchronization.
>>       */
>> -    void (*init_static_data)(struct AVCodec *codec);
>> +    void (*init_static_data)(void);
>
> What's the benefit of removing the parameter?

It is unused because the callee should use ff_thread_once.


>
>>
>>      int (*init)(AVCodecContext *);
>>      int (*encode_sub)(AVCodecContext *, uint8_t *buf, int buf_size,
>> diff --git a/libavcodec/libvpxdec.c b/libavcodec/libvpxdec.c
>> index 04f27d3396..f2003b836b 100644
>> --- a/libavcodec/libvpxdec.c
>> +++ b/libavcodec/libvpxdec.c
>> @@ -30,6 +30,7 @@
>>  #include "libavutil/common.h"
>>  #include "libavutil/imgutils.h"
>>  #include "libavutil/intreadwrite.h"
>> +#include "libavutil/thread.h"
>>  #include "avcodec.h"
>>  #include "internal.h"
>>  #include "libvpx.h"
>> @@ -299,6 +300,18 @@ static av_cold int vp9_init(AVCodecContext *avctx)
>>      return vpx_init(avctx, &vpx_codec_vp9_dx_algo, 0);
>>  }
>>
>> +static av_cold void vp9_init_static_once(void)
>> +{
>> +    extern AVCodec ff_libvpx_vp9_decoder;
>> +    ff_vp9_init_static(&ff_libvpx_vp9_decoder);
>> +}
>> +
>> +static av_cold void vp9_init_static(void)
>> +{
>> +    static AVOnce once = AV_ONCE_INIT;
>> +    ff_thread_once(&once, vp9_init_static_once);
>> +}
>> +
>>  AVCodec ff_libvpx_vp9_decoder = {
>>      .name           = "libvpx-vp9",
>>      .long_name      = NULL_IF_CONFIG_SMALL("libvpx VP9"),
>> @@ -309,7 +322,7 @@ AVCodec ff_libvpx_vp9_decoder = {
>>      .close          = vpx_free,
>>      .decode         = vpx_decode,
>>      .capabilities   = AV_CODEC_CAP_AUTO_THREADS | AV_CODEC_CAP_DR1,
>> -    .init_static_data = ff_vp9_init_static,
>> +    .init_static_data = vp9_init_static,
>
> I think you can remove the init_static_data call to ff_vp9_init_static
> from the decoder altogether. It just sets AVCodec.pix_fmts, which afaics
> is only needed for the encoder.
>
> Doing it would also let us get rid of libvpx.c, as everything there can
> then be moved into libvpxenc.c

It should be in separate patch.


>
>>      .profiles       = NULL_IF_CONFIG_SMALL(ff_vp9_profiles),
>>      .wrapper_name   = "libvpx",
>>  };
>> diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
>> index d0bd1e997a..086dd5defa 100644
>> --- a/libavcodec/libvpxenc.c
>> +++ b/libavcodec/libvpxenc.c
>> @@ -39,6 +39,7 @@
>>  #include "libavutil/intreadwrite.h"
>>  #include "libavutil/mathematics.h"
>>  #include "libavutil/opt.h"
>> +#include "libavutil/thread.h"
>>
>>  /**
>>   * Portion of struct vpx_codec_cx_pkt from vpx_encoder.h.
>> @@ -1209,6 +1210,18 @@ static av_cold int vp9_init(AVCodecContext *avctx)
>>      return vpx_init(avctx, vpx_codec_vp9_cx());
>>  }
>>
>> +static av_cold void vp9_init_static_once(void)
>> +{
>> +    extern AVCodec ff_libvpx_vp9_encoder;
>> +    ff_vp9_init_static(&ff_libvpx_vp9_encoder);
>> +}
>> +
>> +static av_cold void vp9_init_static(void)
>> +{
>> +    static AVOnce once = AV_ONCE_INIT;
>> +    ff_thread_once(&once, vp9_init_static_once);
>> +}
>> +
>>  static const AVClass class_vp9 = {
>>      .class_name = "libvpx-vp9 encoder",
>>      .item_name  = av_default_item_name,
>> @@ -1229,7 +1242,7 @@ AVCodec ff_libvpx_vp9_encoder = {
>>      .profiles       = NULL_IF_CONFIG_SMALL(ff_vp9_profiles),
>>      .priv_class     = &class_vp9,
>>      .defaults       = defaults,
>> -    .init_static_data = ff_vp9_init_static,
>> +    .init_static_data = vp9_init_static,
>>      .wrapper_name   = "libvpx",
>>  };
>>  #endif /* CONFIG_LIBVPX_VP9_ENCODER */
>> diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
>> index 12379ff763..0da61a0fcd 100644
>> --- a/libavcodec/libx264.c
>> +++ b/libavcodec/libx264.c
>> @@ -26,6 +26,7 @@
>>  #include "libavutil/pixdesc.h"
>>  #include "libavutil/stereo3d.h"
>>  #include "libavutil/intreadwrite.h"
>> +#include "libavutil/thread.h"
>>  #include "avcodec.h"
>>  #include "internal.h"
>>
>> @@ -896,8 +897,10 @@ static const enum AVPixelFormat pix_fmts_8bit_rgb[] = {
>>  };
>>  #endif
>>
>> -static av_cold void X264_init_static(AVCodec *codec)
>> +static av_cold void X264_init_static_once(void)
>>  {
>> +    extern AVCodec ff_libx264_encoder;
>> +    AVCodec *codec = &ff_libx264_encoder;
>>  #if X264_BUILD < 153
>>      if (x264_bit_depth == 8)
>>          codec->pix_fmts = pix_fmts_8bit;
>> @@ -910,6 +913,12 @@ static av_cold void X264_init_static(AVCodec *codec)
>>  #endif
>>  }
>>
>> +static av_cold void X264_init_static(void)
>> +{
>> +    static AVOnce once = AV_ONCE_INIT;
>> +    ff_thread_once(&once, X264_init_static_once);
>> +}
>> +
>>  #define OFFSET(x) offsetof(X264Context, x)
>>  #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
>>  static const AVOption options[] = {
>> diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
>> index 3c97800ccb..63e1240473 100644
>> --- a/libavcodec/libx265.c
>> +++ b/libavcodec/libx265.c
>> @@ -31,6 +31,7 @@
>>  #include "libavutil/common.h"
>>  #include "libavutil/opt.h"
>>  #include "libavutil/pixdesc.h"
>> +#include "libavutil/thread.h"
>>  #include "avcodec.h"
>>  #include "internal.h"
>>
>> @@ -395,8 +396,10 @@ static const enum AVPixelFormat x265_csp_twelve[] = {
>>      AV_PIX_FMT_NONE
>>  };
>>
>> -static av_cold void libx265_encode_init_csp(AVCodec *codec)
>> +static av_cold void libx265_encode_init_csp_once(void)
>>  {
>> +    extern AVCodec ff_libx265_encoder;
>> +    AVCodec *codec = &ff_libx265_encoder;
>>      if (x265_api_get(12))
>>          codec->pix_fmts = x265_csp_twelve;
>>      else if (x265_api_get(10))
>> @@ -405,6 +408,12 @@ static av_cold void libx265_encode_init_csp(AVCodec *codec)
>>          codec->pix_fmts = x265_csp_eight;
>>  }
>>
>> +static av_cold void libx265_encode_init_csp(void)
>> +{
>> +    static AVOnce once = AV_ONCE_INIT;
>> +    ff_thread_once(&once, libx265_encode_init_csp_once);
>> +}
>> +
>>  #define OFFSET(x) offsetof(libx265Context, x)
>>  #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
>>  static const AVOption options[] = {
>>
>

Thank's.
Muhammad Faiz Feb. 14, 2018, 12:07 a.m.
On Tue, Feb 13, 2018 at 3:57 AM, wm4 <nfxjfg@googlemail.com> wrote:
> On Mon, 12 Feb 2018 12:42:10 +0700
> Muhammad Faiz <mfcc64@gmail.com> wrote:
>
>> Modify the behavior of init_static_data().
>>
>> Signed-off-by: Muhammad Faiz <mfcc64@gmail.com>
>> ---
>
> Seems OK, but I'm also not sure about the benefit. The fundamental
> problem that these codecs need to mutate AVCodec before the users sees
> it won't go away.

Actually, I'm too. Any idea how to remove init_static_data()?

Thank's.
wm4 Feb. 14, 2018, 5:09 a.m.
On Wed, 14 Feb 2018 07:07:09 +0700
Muhammad Faiz <mfcc64@gmail.com> wrote:

> On Tue, Feb 13, 2018 at 3:57 AM, wm4 <nfxjfg@googlemail.com> wrote:
> > On Mon, 12 Feb 2018 12:42:10 +0700
> > Muhammad Faiz <mfcc64@gmail.com> wrote:
> >  
> >> Modify the behavior of init_static_data().
> >>
> >> Signed-off-by: Muhammad Faiz <mfcc64@gmail.com>
> >> ---  
> >
> > Seems OK, but I'm also not sure about the benefit. The fundamental
> > problem that these codecs need to mutate AVCodec before the users sees
> > it won't go away.  
> 
> Actually, I'm too. Any idea how to remove init_static_data()?

The only way is to change the API somehow and deprecate
AVCodec.pix_fmts. Or we could define that entries in pix_fmts are not
always available at runtime, but this would still be an API break and
require deprecation and a replacement mechanism. Honestly I'm not
really sure how to do this - maybe the simplest possible replacement
would be fine, like avcodec_get_pix_fmts(AVCodec)?

For now it actually seems less trouble to just leave those maybe 3 or 4
AVCodecs mutable. At least there are no race anymore thanks to
init_once.
James Almer Feb. 14, 2018, 1:11 p.m.
On 2/14/2018 2:09 AM, wm4 wrote:
> On Wed, 14 Feb 2018 07:07:09 +0700
> Muhammad Faiz <mfcc64@gmail.com> wrote:
> 
>> On Tue, Feb 13, 2018 at 3:57 AM, wm4 <nfxjfg@googlemail.com> wrote:
>>> On Mon, 12 Feb 2018 12:42:10 +0700
>>> Muhammad Faiz <mfcc64@gmail.com> wrote:
>>>  
>>>> Modify the behavior of init_static_data().
>>>>
>>>> Signed-off-by: Muhammad Faiz <mfcc64@gmail.com>
>>>> ---  
>>>
>>> Seems OK, but I'm also not sure about the benefit. The fundamental
>>> problem that these codecs need to mutate AVCodec before the users sees
>>> it won't go away.  
>>
>> Actually, I'm too. Any idea how to remove init_static_data()?
> 
> The only way is to change the API somehow and deprecate
> AVCodec.pix_fmts.

That's not the only thing you can do to an AVCodec during init_static.
Before we dropped support for old libvpx versions we'd set the
experimental cap to the encoder's AVCodec if it was a version known for
generating bad streams.
The same may end up being done for libaom and AV1.

> Or we could define that entries in pix_fmts are not
> always available at runtime, but this would still be an API break and
> require deprecation and a replacement mechanism. Honestly I'm not
> really sure how to do this - maybe the simplest possible replacement
> would be fine, like avcodec_get_pix_fmts(AVCodec)?
> 
> For now it actually seems less trouble to just leave those maybe 3 or 4
> AVCodecs mutable. At least there are no race anymore thanks to
> init_once.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

Patch hide | download patch | download mbox

diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index 774b78ef09..02910b5594 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -757,24 +757,16 @@  extern AVCodec ff_vp9_vaapi_encoder;
 
 #include "libavcodec/codec_list.c"
 
-static AVOnce av_codec_static_init = AV_ONCE_INIT;
-static void av_codec_init_static(void)
-{
-    for (int i = 0; codec_list[i]; i++) {
-        if (codec_list[i]->init_static_data)
-            codec_list[i]->init_static_data((AVCodec*)codec_list[i]);
-    }
-}
-
 const AVCodec *av_codec_iterate(void **opaque)
 {
     uintptr_t i = (uintptr_t)*opaque;
     const AVCodec *c = codec_list[i];
 
-    ff_thread_once(&av_codec_static_init, av_codec_init_static);
-
-    if (c)
+    if (c) {
+        if (c->init_static_data)
+            c->init_static_data();
         *opaque = (void*)(i + 1);
+    }
 
     return c;
 }
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index ad0b48a839..d89bf300fc 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -3443,8 +3443,10 @@  typedef struct AVCodec {
      *
      * This is not intended for time consuming operations as it is
      * run for every codec regardless of that codec being used.
+     * This may be called multiple times from different threads, the callee
+     * has responsibility for thread synchronization.
      */
-    void (*init_static_data)(struct AVCodec *codec);
+    void (*init_static_data)(void);
 
     int (*init)(AVCodecContext *);
     int (*encode_sub)(AVCodecContext *, uint8_t *buf, int buf_size,
diff --git a/libavcodec/libvpxdec.c b/libavcodec/libvpxdec.c
index 04f27d3396..f2003b836b 100644
--- a/libavcodec/libvpxdec.c
+++ b/libavcodec/libvpxdec.c
@@ -30,6 +30,7 @@ 
 #include "libavutil/common.h"
 #include "libavutil/imgutils.h"
 #include "libavutil/intreadwrite.h"
+#include "libavutil/thread.h"
 #include "avcodec.h"
 #include "internal.h"
 #include "libvpx.h"
@@ -299,6 +300,18 @@  static av_cold int vp9_init(AVCodecContext *avctx)
     return vpx_init(avctx, &vpx_codec_vp9_dx_algo, 0);
 }
 
+static av_cold void vp9_init_static_once(void)
+{
+    extern AVCodec ff_libvpx_vp9_decoder;
+    ff_vp9_init_static(&ff_libvpx_vp9_decoder);
+}
+
+static av_cold void vp9_init_static(void)
+{
+    static AVOnce once = AV_ONCE_INIT;
+    ff_thread_once(&once, vp9_init_static_once);
+}
+
 AVCodec ff_libvpx_vp9_decoder = {
     .name           = "libvpx-vp9",
     .long_name      = NULL_IF_CONFIG_SMALL("libvpx VP9"),
@@ -309,7 +322,7 @@  AVCodec ff_libvpx_vp9_decoder = {
     .close          = vpx_free,
     .decode         = vpx_decode,
     .capabilities   = AV_CODEC_CAP_AUTO_THREADS | AV_CODEC_CAP_DR1,
-    .init_static_data = ff_vp9_init_static,
+    .init_static_data = vp9_init_static,
     .profiles       = NULL_IF_CONFIG_SMALL(ff_vp9_profiles),
     .wrapper_name   = "libvpx",
 };
diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
index d0bd1e997a..086dd5defa 100644
--- a/libavcodec/libvpxenc.c
+++ b/libavcodec/libvpxenc.c
@@ -39,6 +39,7 @@ 
 #include "libavutil/intreadwrite.h"
 #include "libavutil/mathematics.h"
 #include "libavutil/opt.h"
+#include "libavutil/thread.h"
 
 /**
  * Portion of struct vpx_codec_cx_pkt from vpx_encoder.h.
@@ -1209,6 +1210,18 @@  static av_cold int vp9_init(AVCodecContext *avctx)
     return vpx_init(avctx, vpx_codec_vp9_cx());
 }
 
+static av_cold void vp9_init_static_once(void)
+{
+    extern AVCodec ff_libvpx_vp9_encoder;
+    ff_vp9_init_static(&ff_libvpx_vp9_encoder);
+}
+
+static av_cold void vp9_init_static(void)
+{
+    static AVOnce once = AV_ONCE_INIT;
+    ff_thread_once(&once, vp9_init_static_once);
+}
+
 static const AVClass class_vp9 = {
     .class_name = "libvpx-vp9 encoder",
     .item_name  = av_default_item_name,
@@ -1229,7 +1242,7 @@  AVCodec ff_libvpx_vp9_encoder = {
     .profiles       = NULL_IF_CONFIG_SMALL(ff_vp9_profiles),
     .priv_class     = &class_vp9,
     .defaults       = defaults,
-    .init_static_data = ff_vp9_init_static,
+    .init_static_data = vp9_init_static,
     .wrapper_name   = "libvpx",
 };
 #endif /* CONFIG_LIBVPX_VP9_ENCODER */
diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
index 12379ff763..0da61a0fcd 100644
--- a/libavcodec/libx264.c
+++ b/libavcodec/libx264.c
@@ -26,6 +26,7 @@ 
 #include "libavutil/pixdesc.h"
 #include "libavutil/stereo3d.h"
 #include "libavutil/intreadwrite.h"
+#include "libavutil/thread.h"
 #include "avcodec.h"
 #include "internal.h"
 
@@ -896,8 +897,10 @@  static const enum AVPixelFormat pix_fmts_8bit_rgb[] = {
 };
 #endif
 
-static av_cold void X264_init_static(AVCodec *codec)
+static av_cold void X264_init_static_once(void)
 {
+    extern AVCodec ff_libx264_encoder;
+    AVCodec *codec = &ff_libx264_encoder;
 #if X264_BUILD < 153
     if (x264_bit_depth == 8)
         codec->pix_fmts = pix_fmts_8bit;
@@ -910,6 +913,12 @@  static av_cold void X264_init_static(AVCodec *codec)
 #endif
 }
 
+static av_cold void X264_init_static(void)
+{
+    static AVOnce once = AV_ONCE_INIT;
+    ff_thread_once(&once, X264_init_static_once);
+}
+
 #define OFFSET(x) offsetof(X264Context, x)
 #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
 static const AVOption options[] = {
diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
index 3c97800ccb..63e1240473 100644
--- a/libavcodec/libx265.c
+++ b/libavcodec/libx265.c
@@ -31,6 +31,7 @@ 
 #include "libavutil/common.h"
 #include "libavutil/opt.h"
 #include "libavutil/pixdesc.h"
+#include "libavutil/thread.h"
 #include "avcodec.h"
 #include "internal.h"
 
@@ -395,8 +396,10 @@  static const enum AVPixelFormat x265_csp_twelve[] = {
     AV_PIX_FMT_NONE
 };
 
-static av_cold void libx265_encode_init_csp(AVCodec *codec)
+static av_cold void libx265_encode_init_csp_once(void)
 {
+    extern AVCodec ff_libx265_encoder;
+    AVCodec *codec = &ff_libx265_encoder;
     if (x265_api_get(12))
         codec->pix_fmts = x265_csp_twelve;
     else if (x265_api_get(10))
@@ -405,6 +408,12 @@  static av_cold void libx265_encode_init_csp(AVCodec *codec)
         codec->pix_fmts = x265_csp_eight;
 }
 
+static av_cold void libx265_encode_init_csp(void)
+{
+    static AVOnce once = AV_ONCE_INIT;
+    ff_thread_once(&once, libx265_encode_init_csp_once);
+}
+
 #define OFFSET(x) offsetof(libx265Context, x)
 #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
 static const AVOption options[] = {