diff mbox series

[FFmpeg-devel,28/39] avcodec/mpeg4video: Making initializing RLTable thread-safe

Message ID 20201210111657.2276739-29-andreas.rheinhardt@gmail.com
State Accepted
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
Up until now the RLTable ff_mpeg4_rl_intra was initialized by both mpeg4
decoder and encoder (except the VLCs that are only used by the decoder).
This is an obstacle to making these codecs init-threadsafe, so move
initializing this to a single function that is guarded by a dedicated
AVOnce.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
Those ff_rl_init(&ff_rvlc_rl_inter/intra, ...) calls are actually
superfluous: If these RLTables are in use, the parts initialized by
ff_rl_init() are not used (see mpeg4_decode_block in mpeg4videodec.c).
Yet I have no sample that actually triggers this codepath, so I haven't
done it (rvlc stands for reversible VLC). It would be great if someone
could provide me with samples for this.

 libavcodec/mpeg4video.c    | 14 +++++++++++++-
 libavcodec/mpeg4video.h    |  3 +--
 libavcodec/mpeg4videodec.c |  8 +++++---
 libavcodec/mpeg4videoenc.c |  2 +-
 4 files changed, 20 insertions(+), 7 deletions(-)

Comments

Anton Khirnov April 17, 2021, 1:53 p.m. UTC | #1
Quoting Andreas Rheinhardt (2020-12-10 12:16:46)
> Up until now the RLTable ff_mpeg4_rl_intra was initialized by both mpeg4
> decoder and encoder (except the VLCs that are only used by the decoder).
> This is an obstacle to making these codecs init-threadsafe, so move
> initializing this to a single function that is guarded by a dedicated
> AVOnce.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
> Those ff_rl_init(&ff_rvlc_rl_inter/intra, ...) calls are actually
> superfluous: If these RLTables are in use, the parts initialized by
> ff_rl_init() are not used (see mpeg4_decode_block in mpeg4videodec.c).
> Yet I have no sample that actually triggers this codepath, so I haven't
> done it (rvlc stands for reversible VLC). It would be great if someone
> could provide me with samples for this.
> 
>  libavcodec/mpeg4video.c    | 14 +++++++++++++-
>  libavcodec/mpeg4video.h    |  3 +--
>  libavcodec/mpeg4videodec.c |  8 +++++---
>  libavcodec/mpeg4videoenc.c |  2 +-
>  4 files changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/libavcodec/mpeg4video.h b/libavcodec/mpeg4video.h
> index e919db87a5..74470efb51 100644
> --- a/libavcodec/mpeg4video.h
> +++ b/libavcodec/mpeg4video.h
> @@ -129,6 +129,7 @@ extern const int8_t ff_mpeg4_intra_level[102];
>  extern const int8_t ff_mpeg4_intra_run[102];
>  
>  extern RLTable ff_mpeg4_rl_intra;
> +av_cold void ff_mpeg4_init_rl_intra(void);

Does it make sense to mark the prototype as av_cold?

Beyond that patch looks ok.
diff mbox series

Patch

diff --git a/libavcodec/mpeg4video.c b/libavcodec/mpeg4video.c
index 2aaa9f734c..ffeaf822b2 100644
--- a/libavcodec/mpeg4video.c
+++ b/libavcodec/mpeg4video.c
@@ -20,12 +20,24 @@ 
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+#include "libavutil/thread.h"
+
 #include "mpegutils.h"
 #include "mpegvideo.h"
 #include "mpeg4video.h"
 #include "mpeg4data.h"
 
-uint8_t ff_mpeg4_static_rl_table_store[3][2][2 * MAX_RUN + MAX_LEVEL + 3];
+static av_cold void mpeg4_init_rl_intra(void)
+{
+    static uint8_t mpeg4_rl_intra_table[2][2 * MAX_RUN + MAX_LEVEL + 3];
+    ff_rl_init(&ff_mpeg4_rl_intra, mpeg4_rl_intra_table);
+}
+
+av_cold void ff_mpeg4_init_rl_intra(void)
+{
+    static AVOnce init_static_once = AV_ONCE_INIT;
+    ff_thread_once(&init_static_once, mpeg4_init_rl_intra);
+}
 
 int ff_mpeg4_get_video_packet_prefix_length(MpegEncContext *s)
 {
diff --git a/libavcodec/mpeg4video.h b/libavcodec/mpeg4video.h
index e919db87a5..74470efb51 100644
--- a/libavcodec/mpeg4video.h
+++ b/libavcodec/mpeg4video.h
@@ -129,6 +129,7 @@  extern const int8_t ff_mpeg4_intra_level[102];
 extern const int8_t ff_mpeg4_intra_run[102];
 
 extern RLTable ff_mpeg4_rl_intra;
+av_cold void ff_mpeg4_init_rl_intra(void);
 
 /* Note this is identical to the intra rvlc except that it is reordered. */
 extern RLTable ff_rvlc_rl_inter;
@@ -180,8 +181,6 @@  int ff_mpeg4_frame_end(AVCodecContext *avctx, const uint8_t *buf, int buf_size);
  */
 int ff_mpeg4_set_direct_mv(MpegEncContext *s, int mx, int my);
 
-extern uint8_t ff_mpeg4_static_rl_table_store[3][2][2 * MAX_RUN + MAX_LEVEL + 3];
-
 #if 0 //3IV1 is quite rare and it slows things down a tiny bit
 #define IS_3IV1 s->codec_tag == AV_RL32("3IV1")
 #else
diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c
index a6e29447b0..5b8236872f 100644
--- a/libavcodec/mpeg4videodec.c
+++ b/libavcodec/mpeg4videodec.c
@@ -3386,9 +3386,11 @@  av_cold void ff_mpeg4videodec_static_init(void) {
     static int done = 0;
 
     if (!done) {
-        ff_rl_init(&ff_mpeg4_rl_intra, ff_mpeg4_static_rl_table_store[0]);
-        ff_rl_init(&ff_rvlc_rl_inter, ff_mpeg4_static_rl_table_store[1]);
-        ff_rl_init(&ff_rvlc_rl_intra, ff_mpeg4_static_rl_table_store[2]);
+        static uint8_t mpeg4_rvlc_rl_tables[2][2][2 * MAX_RUN + MAX_LEVEL + 3];
+
+        ff_mpeg4_init_rl_intra();
+        ff_rl_init(&ff_rvlc_rl_inter, mpeg4_rvlc_rl_tables[0]);
+        ff_rl_init(&ff_rvlc_rl_intra, mpeg4_rvlc_rl_tables[1]);
         INIT_FIRST_VLC_RL(ff_mpeg4_rl_intra, 554);
         INIT_VLC_RL(ff_rvlc_rl_inter, 1072);
         INIT_FIRST_VLC_RL(ff_rvlc_rl_intra, 1072);
diff --git a/libavcodec/mpeg4videoenc.c b/libavcodec/mpeg4videoenc.c
index b3fa910c64..ca1c0dfd9c 100644
--- a/libavcodec/mpeg4videoenc.c
+++ b/libavcodec/mpeg4videoenc.c
@@ -1288,7 +1288,7 @@  static av_cold int encode_init(AVCodecContext *avctx)
 
         init_uni_dc_tab();
 
-        ff_rl_init(&ff_mpeg4_rl_intra, ff_mpeg4_static_rl_table_store[0]);
+        ff_mpeg4_init_rl_intra();
 
         init_uni_mpeg4_rl_tab(&ff_mpeg4_rl_intra, uni_mpeg4_intra_rl_bits, uni_mpeg4_intra_rl_len);
         init_uni_mpeg4_rl_tab(&ff_h263_rl_inter, uni_mpeg4_inter_rl_bits, uni_mpeg4_inter_rl_len);