| Message ID | 20201210111657.2276739-29-andreas.rheinhardt@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Make mpegvideo encoders init-threadsafe | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
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 --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);
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(-)