diff mbox series

[FFmpeg-devel,27/39] avcodec/ituh263enc: Make static initializations thread-safe

Message ID 20201210111657.2276739-28-andreas.rheinhardt@gmail.com
State New
Headers show
Series Make mpegvideo encoders init-threadsafe
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished

Commit Message

Andreas Rheinhardt Dec. 10, 2020, 11:16 a.m. UTC
This already makes several encoders (namely FLV, H.263, H.263+ and
RealVideo 1.0 and 2.0) that use this init-threadsafe.

It also makes the Snow encoder init-threadsafe; it was already marked
as such since commit d49210788b0836d56dd872d517fe73f83b080101, because
it was thought to be harmless if one and the same object was
initialized by multiple threads at the same time.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavcodec/flvenc.c        |  2 +-
 libavcodec/ituh263enc.c    | 28 ++++++++++++++++------------
 libavcodec/mpegvideo_enc.c |  4 ++--
 libavcodec/rv10enc.c       |  2 +-
 libavcodec/rv20enc.c       |  2 +-
 5 files changed, 21 insertions(+), 17 deletions(-)

Comments

Andreas Rheinhardt Dec. 11, 2020, 12:27 p.m. UTC | #1
Andreas Rheinhardt:
> This already makes several encoders (namely FLV, H.263, H.263+ and
> RealVideo 1.0 and 2.0) that use this init-threadsafe.
> 
> It also makes the Snow encoder init-threadsafe; it was already marked
> as such since commit d49210788b0836d56dd872d517fe73f83b080101, because
> it was thought to be harmless if one and the same object was
> initialized by multiple threads at the same time.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavcodec/flvenc.c        |  2 +-
>  libavcodec/ituh263enc.c    | 28 ++++++++++++++++------------
>  libavcodec/mpegvideo_enc.c |  4 ++--
>  libavcodec/rv10enc.c       |  2 +-
>  libavcodec/rv20enc.c       |  2 +-
>  5 files changed, 21 insertions(+), 17 deletions(-)
> 
> diff --git a/libavcodec/flvenc.c b/libavcodec/flvenc.c
> index b85e4667c4..9429862159 100644
> --- a/libavcodec/flvenc.c
> +++ b/libavcodec/flvenc.c
> @@ -107,7 +107,7 @@ AVCodec ff_flv_encoder = {
>      .init           = ff_mpv_encode_init,
>      .encode2        = ff_mpv_encode_picture,
>      .close          = ff_mpv_encode_end,
> -    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
> +    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE | FF_CODEC_CAP_INIT_CLEANUP,
>      .pix_fmts       = (const enum AVPixelFormat[]) { AV_PIX_FMT_YUV420P,
>                                                       AV_PIX_FMT_NONE},
>      .priv_class     = &flv_class,
> diff --git a/libavcodec/ituh263enc.c b/libavcodec/ituh263enc.c
> index 43260e6984..79c8c9999e 100644
> --- a/libavcodec/ituh263enc.c
> +++ b/libavcodec/ituh263enc.c
> @@ -30,6 +30,7 @@
>  #include <limits.h>
>  
>  #include "libavutil/attributes.h"
> +#include "libavutil/thread.h"
>  #include "avcodec.h"
>  #include "mpegvideo.h"
>  #include "mpegvideodata.h"
> @@ -671,7 +672,7 @@ void ff_h263_encode_motion(PutBitContext *pb, int val, int f_code)
>      }
>  }
>  
> -static av_cold void init_mv_penalty_and_fcode(MpegEncContext *s)
> +static av_cold void init_mv_penalty_and_fcode(void)
>  {
>      int f_code;
>      int mv;
> @@ -754,22 +755,23 @@ static av_cold void init_uni_h263_rl_tab(const RLTable *rl, uint8_t *len_tab)
>      }
>  }
>  
> -av_cold void ff_h263_encode_init(MpegEncContext *s)
> +static av_cold void h263_encode_init_static(void)
>  {
> -    static int done = 0;
> +    static uint8_t rl_intra_table[2][2 * MAX_RUN + MAX_LEVEL + 3];
>  
> -    if (!done) {
> -        static uint8_t rl_intra_table[2][2 * MAX_RUN + MAX_LEVEL + 3];
> -        done = 1;
> +    ff_rl_init(&ff_rl_intra_aic, rl_intra_table);
> +    ff_h263_init_rl_inter();
>  
> -        ff_rl_init(&ff_rl_intra_aic, rl_intra_table);
> -        ff_h263_init_rl_inter();
> +    init_uni_h263_rl_tab(&ff_rl_intra_aic,  uni_h263_intra_aic_rl_len);
> +    init_uni_h263_rl_tab(&ff_h263_rl_inter, uni_h263_inter_rl_len);
>  
> -        init_uni_h263_rl_tab(&ff_rl_intra_aic,  uni_h263_intra_aic_rl_len);
> -        init_uni_h263_rl_tab(&ff_h263_rl_inter, uni_h263_inter_rl_len);
> +    init_mv_penalty_and_fcode();
> +}
> +
> +av_cold void ff_h263_encode_init(MpegEncContext *s)
> +{
> +    static AVOnce init_static_once = AV_ONCE_INIT;
>  
> -        init_mv_penalty_and_fcode(s);
> -    }
>      s->me.mv_penalty= mv_penalty; // FIXME exact table for MSMPEG4 & H.263+
>  
>      s->intra_ac_vlc_length     =s->inter_ac_vlc_length     = uni_h263_inter_rl_len;
> @@ -817,6 +819,8 @@ av_cold void ff_h263_encode_init(MpegEncContext *s)
>          s->y_dc_scale_table=
>          s->c_dc_scale_table= ff_mpeg1_dc_scale_table;
>      }
> +
> +    ff_thread_once(&init_static_once, h263_encode_init_static);
>  }
>  
>  void ff_h263_encode_mba(MpegEncContext *s)
> diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
> index 061081db08..8fb415f42b 100644
> --- a/libavcodec/mpegvideo_enc.c
> +++ b/libavcodec/mpegvideo_enc.c
> @@ -4786,7 +4786,7 @@ AVCodec ff_h263_encoder = {
>      .init           = ff_mpv_encode_init,
>      .encode2        = ff_mpv_encode_picture,
>      .close          = ff_mpv_encode_end,
> -    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
> +    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE | FF_CODEC_CAP_INIT_CLEANUP,
>      .pix_fmts= (const enum AVPixelFormat[]){AV_PIX_FMT_YUV420P, AV_PIX_FMT_NONE},
>      .priv_class     = &h263_class,
>  };
> @@ -4816,7 +4816,7 @@ AVCodec ff_h263p_encoder = {
>      .encode2        = ff_mpv_encode_picture,
>      .close          = ff_mpv_encode_end,
>      .capabilities   = AV_CODEC_CAP_SLICE_THREADS,
> -    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
> +    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE | FF_CODEC_CAP_INIT_CLEANUP,
>      .pix_fmts       = (const enum AVPixelFormat[]){ AV_PIX_FMT_YUV420P, AV_PIX_FMT_NONE },
>      .priv_class     = &h263p_class,
>  };
> diff --git a/libavcodec/rv10enc.c b/libavcodec/rv10enc.c
> index 42316836c5..6add8e4dd1 100644
> --- a/libavcodec/rv10enc.c
> +++ b/libavcodec/rv10enc.c
> @@ -79,7 +79,7 @@ AVCodec ff_rv10_encoder = {
>      .init           = ff_mpv_encode_init,
>      .encode2        = ff_mpv_encode_picture,
>      .close          = ff_mpv_encode_end,
> -    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
> +    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE | FF_CODEC_CAP_INIT_CLEANUP,
>      .pix_fmts       = (const enum AVPixelFormat[]){ AV_PIX_FMT_YUV420P, AV_PIX_FMT_NONE },
>      .priv_class     = &rv10_class,
>  };
> diff --git a/libavcodec/rv20enc.c b/libavcodec/rv20enc.c
> index d9d63d4d9c..95f90389bc 100644
> --- a/libavcodec/rv20enc.c
> +++ b/libavcodec/rv20enc.c
> @@ -76,7 +76,7 @@ AVCodec ff_rv20_encoder = {
>      .init           = ff_mpv_encode_init,
>      .encode2        = ff_mpv_encode_picture,
>      .close          = ff_mpv_encode_end,
> -    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
> +    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE | FF_CODEC_CAP_INIT_CLEANUP,
>      .pix_fmts       = (const enum AVPixelFormat[]){ AV_PIX_FMT_YUV420P, AV_PIX_FMT_NONE },
>      .priv_class     = &rv20_class,
>  };
> 
This also makes the SVQ1 encoder init-threadsafe; have amended the
commit locally.

- Andreas
Anton Khirnov Feb. 3, 2021, 3:07 p.m. UTC | #2
Just noticed
static int16_t basis[64][64];
in mpegvideo_enc.c, which will be accessed in a racy way if
quantizer_noise_shaping is enabled. This applies to all mpegvideo
encoders as far as I can tell.
Andreas Rheinhardt Feb. 3, 2021, 8:25 p.m. UTC | #3
Anton Khirnov:
> Just noticed
> static int16_t basis[64][64];
> in mpegvideo_enc.c, which will be accessed in a racy way if
> quantizer_noise_shaping is enabled. This applies to all mpegvideo
> encoders as far as I can tell.
> 
The reason I didn't spot this is that this part of the code is not
accessible from any init function. And I am not certain that the code
as-is is even correct: basis depends upon the idct_permutation in use
and so one might need different bases for different callers, i.e. one
would need to make this part of the context.

- Andreas
diff mbox series

Patch

diff --git a/libavcodec/flvenc.c b/libavcodec/flvenc.c
index b85e4667c4..9429862159 100644
--- a/libavcodec/flvenc.c
+++ b/libavcodec/flvenc.c
@@ -107,7 +107,7 @@  AVCodec ff_flv_encoder = {
     .init           = ff_mpv_encode_init,
     .encode2        = ff_mpv_encode_picture,
     .close          = ff_mpv_encode_end,
-    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
+    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE | FF_CODEC_CAP_INIT_CLEANUP,
     .pix_fmts       = (const enum AVPixelFormat[]) { AV_PIX_FMT_YUV420P,
                                                      AV_PIX_FMT_NONE},
     .priv_class     = &flv_class,
diff --git a/libavcodec/ituh263enc.c b/libavcodec/ituh263enc.c
index 43260e6984..79c8c9999e 100644
--- a/libavcodec/ituh263enc.c
+++ b/libavcodec/ituh263enc.c
@@ -30,6 +30,7 @@ 
 #include <limits.h>
 
 #include "libavutil/attributes.h"
+#include "libavutil/thread.h"
 #include "avcodec.h"
 #include "mpegvideo.h"
 #include "mpegvideodata.h"
@@ -671,7 +672,7 @@  void ff_h263_encode_motion(PutBitContext *pb, int val, int f_code)
     }
 }
 
-static av_cold void init_mv_penalty_and_fcode(MpegEncContext *s)
+static av_cold void init_mv_penalty_and_fcode(void)
 {
     int f_code;
     int mv;
@@ -754,22 +755,23 @@  static av_cold void init_uni_h263_rl_tab(const RLTable *rl, uint8_t *len_tab)
     }
 }
 
-av_cold void ff_h263_encode_init(MpegEncContext *s)
+static av_cold void h263_encode_init_static(void)
 {
-    static int done = 0;
+    static uint8_t rl_intra_table[2][2 * MAX_RUN + MAX_LEVEL + 3];
 
-    if (!done) {
-        static uint8_t rl_intra_table[2][2 * MAX_RUN + MAX_LEVEL + 3];
-        done = 1;
+    ff_rl_init(&ff_rl_intra_aic, rl_intra_table);
+    ff_h263_init_rl_inter();
 
-        ff_rl_init(&ff_rl_intra_aic, rl_intra_table);
-        ff_h263_init_rl_inter();
+    init_uni_h263_rl_tab(&ff_rl_intra_aic,  uni_h263_intra_aic_rl_len);
+    init_uni_h263_rl_tab(&ff_h263_rl_inter, uni_h263_inter_rl_len);
 
-        init_uni_h263_rl_tab(&ff_rl_intra_aic,  uni_h263_intra_aic_rl_len);
-        init_uni_h263_rl_tab(&ff_h263_rl_inter, uni_h263_inter_rl_len);
+    init_mv_penalty_and_fcode();
+}
+
+av_cold void ff_h263_encode_init(MpegEncContext *s)
+{
+    static AVOnce init_static_once = AV_ONCE_INIT;
 
-        init_mv_penalty_and_fcode(s);
-    }
     s->me.mv_penalty= mv_penalty; // FIXME exact table for MSMPEG4 & H.263+
 
     s->intra_ac_vlc_length     =s->inter_ac_vlc_length     = uni_h263_inter_rl_len;
@@ -817,6 +819,8 @@  av_cold void ff_h263_encode_init(MpegEncContext *s)
         s->y_dc_scale_table=
         s->c_dc_scale_table= ff_mpeg1_dc_scale_table;
     }
+
+    ff_thread_once(&init_static_once, h263_encode_init_static);
 }
 
 void ff_h263_encode_mba(MpegEncContext *s)
diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index 061081db08..8fb415f42b 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -4786,7 +4786,7 @@  AVCodec ff_h263_encoder = {
     .init           = ff_mpv_encode_init,
     .encode2        = ff_mpv_encode_picture,
     .close          = ff_mpv_encode_end,
-    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
+    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE | FF_CODEC_CAP_INIT_CLEANUP,
     .pix_fmts= (const enum AVPixelFormat[]){AV_PIX_FMT_YUV420P, AV_PIX_FMT_NONE},
     .priv_class     = &h263_class,
 };
@@ -4816,7 +4816,7 @@  AVCodec ff_h263p_encoder = {
     .encode2        = ff_mpv_encode_picture,
     .close          = ff_mpv_encode_end,
     .capabilities   = AV_CODEC_CAP_SLICE_THREADS,
-    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
+    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE | FF_CODEC_CAP_INIT_CLEANUP,
     .pix_fmts       = (const enum AVPixelFormat[]){ AV_PIX_FMT_YUV420P, AV_PIX_FMT_NONE },
     .priv_class     = &h263p_class,
 };
diff --git a/libavcodec/rv10enc.c b/libavcodec/rv10enc.c
index 42316836c5..6add8e4dd1 100644
--- a/libavcodec/rv10enc.c
+++ b/libavcodec/rv10enc.c
@@ -79,7 +79,7 @@  AVCodec ff_rv10_encoder = {
     .init           = ff_mpv_encode_init,
     .encode2        = ff_mpv_encode_picture,
     .close          = ff_mpv_encode_end,
-    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
+    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE | FF_CODEC_CAP_INIT_CLEANUP,
     .pix_fmts       = (const enum AVPixelFormat[]){ AV_PIX_FMT_YUV420P, AV_PIX_FMT_NONE },
     .priv_class     = &rv10_class,
 };
diff --git a/libavcodec/rv20enc.c b/libavcodec/rv20enc.c
index d9d63d4d9c..95f90389bc 100644
--- a/libavcodec/rv20enc.c
+++ b/libavcodec/rv20enc.c
@@ -76,7 +76,7 @@  AVCodec ff_rv20_encoder = {
     .init           = ff_mpv_encode_init,
     .encode2        = ff_mpv_encode_picture,
     .close          = ff_mpv_encode_end,
-    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
+    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE | FF_CODEC_CAP_INIT_CLEANUP,
     .pix_fmts       = (const enum AVPixelFormat[]){ AV_PIX_FMT_YUV420P, AV_PIX_FMT_NONE },
     .priv_class     = &rv20_class,
 };