[vlc-devel] [PATCH 1/2] avcodec: fix data-races
Hugo Beauzée-Luyssen
hugo at beauzee.fr
Thu Sep 26 11:16:53 CEST 2019
On Thu, Sep 26, 2019, at 11:02 AM, Thomas Guillem wrote:
> There was various data-races between any avcodec threads and the pf_decode
> thread. The scope of sem wait/post need to be increased. Indeed, dec->fmt_out,
> pts, p_va must be accessed while being protected, the same for
> lavc_UpdateVideoFormat() and decoder_NewPicture().
>
> PS: decoder_QueuePicture() and picture_Release() don't have to be protected,
> it's better to release the sempahore before calling it in order to unlock
> avcodec threads while the picture is being queued.
>
> WARNING: ThreadSanitizer: data race (pid=6962)
> Write of size 4 at 0x7b800002001c by thread T14 (mutexes: write M499,
> write M491):
> #0 date_Change ../../src/misc/mtime.c:83 (libvlccore.so.9+0xcec72)
> #1 lavc_UpdateVideoFormat ../../modules/codec/avcodec/video.c:357
> (libavcodec_plugin.so+0xc50fa)
> #2 lavc_GetFrame ../../modules/codec/avcodec/video.c:1598
> (libavcodec_plugin.so+0xc8d0e)
> #3 get_buffer_internal src/libavcodec/decode.c:1940
> (libavcodec_plugin.so+0x12b4f2)
> #4 ff_get_buffer src/libavcodec/decode.c:1965
> (libavcodec_plugin.so+0x12b4f2)
>
> Previous read of size 4 at 0x7b800002001c by thread T18:
> #0 date_Increment ../../src/misc/mtime.c:91
> (libvlccore.so.9+0xceced)
> #1 interpolate_next_pts ../../modules/codec/avcodec/video.c:798
> (libavcodec_plugin.so+0xc4856)
> #2 DecodeBlock ../../modules/codec/avcodec/video.c:1199
> (libavcodec_plugin.so+0xc7f4e)
> #3 DecodeVideo ../../modules/codec/avcodec/video.c:1357
> (libavcodec_plugin.so+0xc8996)
> #4 DecoderThread_DecodeBlock ../../src/input/decoder.c:1278
> (libvlccore.so.9+0x5cac7)
> #5 DecoderThread_ProcessInput ../../src/input/decoder.c:1400
> (libvlccore.so.9+0x5ca99)
> #6 DecoderThread ../../src/input/decoder.c:1676
> (libvlccore.so.9+0x5cd69)
>
> WARNING: ThreadSanitizer: data race (pid=6962)
> Atomic write of size 8 at 0x7b4c0002fc70 by thread T18:
> #0 __tsan_atomic64_fetch_sub
> ../../../../src/libsanitizer/tsan/tsan_interface_atomic.cc:646
> (libtsan.so.0+0x648dd)
> #1 picture_Release ../../include/vlc_picture.h:203
> (libavcodec_plugin.so+0xc6aa8)
> #2 lavc_ReleaseFrame ../../modules/codec/avcodec/video.c:1461
> (libavcodec_plugin.so+0xc6aa8)
> #3 buffer_replace src/libavutil/buffer.c:120
> (libavcodec_plugin.so+0x999596)
> #4 av_buffer_unref src/libavutil/buffer.c:130
> (libavcodec_plugin.so+0x999596)
> #5 DecoderThread_Flush ../../src/input/decoder.c:1420
> (libvlccore.so.9+0x58c8e)
> #6 DecoderThread ../../src/input/decoder.c:1581
> (libvlccore.so.9+0x5ce24)
>
> Previous write of size 8 at 0x7b4c0002fc70 by thread T12 (mutexes:
> write M494, write M491):
> #0 malloc
> ../../../../src/libsanitizer/tsan/tsan_interceptors.cc:606
> (libtsan.so.0+0x2b1a3)
> #1 malloc
> ../../../../src/libsanitizer/tsan/tsan_interceptors.cc:601
> (libtsan.so.0+0x2b1a3)
> #2 picture_NewPrivate ../../src/misc/picture.c:200
> (libvlccore.so.9+0xd3239)
> #3 picture_NewFromResource ../../src/misc/picture.c:226
> (libvlccore.so.9+0xd338c)
> #4 picture_pool_ClonePicture ../../src/misc/picture_pool.c:109
> (libvlccore.so.9+0xd4ba2)
> #5 picture_pool_Wait ../../src/misc/picture_pool.c:271
> (libvlccore.so.9+0xd5375)
> #6 vout_GetPicture ../../src/video_output/video_output.c:326
> (libvlccore.so.9+0xa9c2f)
> #7 ModuleThread_NewVideoBuffer ../../src/input/decoder.c:605
> (libvlccore.so.9+0x5af28)
> #8 decoder_NewPicture ../../src/input/decoder_helpers.c:93
> (libvlccore.so.9+0x5f8b6)
> #9 lavc_dr_GetFrame ../../modules/codec/avcodec/video.c:1504
> (libavcodec_plugin.so+0xc6b44)
> #10 lavc_GetFrame ../../modules/codec/avcodec/video.c:1611
> (libavcodec_plugin.so+0xc8d47)
> #11 get_buffer_internal src/libavcodec/decode.c:1940
> (libavcodec_plugin.so+0x12b4f2)
> #12 ff_get_buffer src/libavcodec/decode.c:1965
> (libavcodec_plugin.so+0x12b4f2)
>
> WARNING: ThreadSanitizer: data race (pid=7336)
> Read of size 4 at 0x7b8000021020 by thread T13 (mutexes: write M444,
> write M438):
> #0 date_Change ../../src/misc/mtime.c:81 (libvlccore.so.9+0xcec41)
> #1 lavc_UpdateVideoFormat ../../modules/codec/avcodec/video.c:357
> (libavcodec_plugin.so+0xc50fa)
> #2 lavc_GetFrame ../../modules/codec/avcodec/video.c:1598
> (libavcodec_plugin.so+0xc8d0e)
> #3 get_buffer_internal src/libavcodec/decode.c:1940
> (libavcodec_plugin.so+0x12b4f2)
> #4 ff_get_buffer src/libavcodec/decode.c:1965
> (libavcodec_plugin.so+0x12b4f2)
>
> Previous write of size 4 at 0x7b8000021020 by thread T18:
> #0 date_Set ../../include/vlc_tick.h:260
> (libavcodec_plugin.so+0xc7f38)
> #1 DecodeBlock ../../modules/codec/avcodec/video.c:1197
> (libavcodec_plugin.so+0xc7f38)
> #2 DecodeVideo ../../modules/codec/avcodec/video.c:1357
> (libavcodec_plugin.so+0xc8996)
> #3 DecoderThread_DecodeBlock ../../src/input/decoder.c:1278
> (libvlccore.so.9+0x5cac7)
> #4 DecoderThread_ProcessInput ../../src/input/decoder.c:1400
> (libvlccore.so.9+0x5ca99)
> #5 DecoderThread ../../src/input/decoder.c:1676
> (libvlccore.so.9+0x5cd69)
> ---
> modules/codec/avcodec/video.c | 26 ++++++++++++++++++++------
> 1 file changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/modules/codec/avcodec/video.c
> b/modules/codec/avcodec/video.c
> index 2232b7a291..d055325a6e 100644
> --- a/modules/codec/avcodec/video.c
> +++ b/modules/codec/avcodec/video.c
> @@ -1180,6 +1180,8 @@ static int DecodeBlock( decoder_t *p_dec, block_t
> **pp_block )
> if( p_frame_info->b_eos )
> p_sys->b_first_frame = true;
>
> + post_mt( p_sys );
> +
> /* Compute the PTS */
> #ifdef FF_API_PKT_PTS
> int64_t av_pts = frame->pts == AV_NOPTS_VALUE ? frame->pkt_dts
> : frame->pts;
> @@ -1206,6 +1208,7 @@ static int DecodeBlock( decoder_t *p_dec, block_t
> **pp_block )
> ( p_dec->b_frame_drop_allowed && (frame->flags &
> AV_FRAME_FLAG_CORRUPT) &&
> !p_sys->b_show_corrupted ) )
> {
> + wait_mt( p_sys );
> av_frame_free(&frame);
> continue;
> }
> @@ -1223,6 +1226,7 @@ static int DecodeBlock( decoder_t *p_dec, block_t
> **pp_block )
> = malloc( sizeof(video_palette_t) );
> if( !p_palette )
> {
> + wait_mt( p_sys );
Nitpick, but b_error doesn't need to be protected no?
> b_error = true;
> av_frame_free(&frame);
> break;
> @@ -1235,6 +1239,7 @@ static int DecodeBlock( decoder_t *p_dec, block_t
> **pp_block )
> p_dec->fmt_out.video.i_chroma = VLC_CODEC_RGBP;
> if( decoder_UpdateVideoFormat( p_dec ) )
> {
> + wait_mt( p_sys );
> av_frame_free(&frame);
> continue;
> }
> @@ -1252,6 +1257,7 @@ static int DecodeBlock( decoder_t *p_dec, block_t
> **pp_block )
>
> if( !p_pic )
> {
> + wait_mt( p_sys );
> av_frame_free(&frame);
> break;
> }
> @@ -1259,6 +1265,7 @@ static int DecodeBlock( decoder_t *p_dec, block_t
> **pp_block )
> /* Fill picture_t from AVFrame */
> if( lavc_CopyPicture( p_dec, p_pic, frame ) != VLC_SUCCESS
> )
> {
> + wait_mt( p_sys );
> av_frame_free(&frame);
> picture_Release( p_pic );
> break;
> @@ -1274,6 +1281,7 @@ static int DecodeBlock( decoder_t *p_dec, block_t
> **pp_block )
> p_pic = picture_Clone( p_pic );
> if( unlikely(p_pic == NULL) )
> {
> + wait_mt( p_sys );
> av_frame_free(&frame);
> break;
> }
> @@ -1312,11 +1320,14 @@ static int DecodeBlock( decoder_t *p_dec,
> block_t **pp_block )
> if(p_frame_info->b_eos)
> p_pic->b_still = true;
> p_sys->b_first_frame = false;
> + wait_mt( p_sys );
> decoder_QueueVideo( p_dec, p_pic );
> }
> else
> + {
> + wait_mt( p_sys );
> picture_Release( p_pic );
> -
> + }
> } while( true );
>
> /* After draining, we need to reset decoder with a flush */
> @@ -1601,14 +1612,18 @@ static int lavc_GetFrame(struct AVCodecContext
> *ctx, AVFrame *frame, int flags)
> return -1;
> }
> }
> - post_mt(sys);
>
> if (sys->p_va != NULL)
> - return lavc_va_GetFrame(ctx, frame);
> + {
> + int ret = lavc_va_GetFrame(ctx, frame);
> + post_mt(sys);
> + return ret;
> + }
>
> /* Some codecs set pix_fmt only after the 1st frame has been decoded,
> * so we need to check for direct rendering again. */
> int ret = lavc_dr_GetFrame(ctx, frame);
> + post_mt(sys);
> if (ret)
> ret = avcodec_default_get_buffer2(ctx, frame, flags);
> return ret;
> @@ -1740,12 +1755,11 @@ no_reuse:
> test_pic ? test_pic->p_sys : NULL);
> if (test_pic)
> picture_Release(test_pic);
> + wait_mt(p_sys);
> if (va == NULL)
> - {
> - wait_mt(p_sys);
> continue; /* Unsupported codec profile or such */
> - }
>
> + post_mt(p_sys);
> p_sys->p_va = va;
> p_sys->pix_fmt = hwfmt;
> p_context->draw_horiz_band = NULL;
> --
> 2.20.1
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
--
Hugo Beauzée-Luyssen
hugo at beauzee.fr
More information about the vlc-devel
mailing list