[FFmpeg-devel] [PATCH] avcodec: remove av_codec_init_static()

James Almer jamrial at gmail.com
Mon Feb 12 21:40:51 EET 2018


On 2/12/2018 2:42 AM, Muhammad Faiz wrote:
> Modify the behavior of init_static_data().
> 
> Signed-off-by: Muhammad Faiz <mfcc64 at 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[] = {
> 



More information about the ffmpeg-devel mailing list