[vlc-devel] [PATCH 1/2] avcodec: fix data-races

Thomas Guillem thomas at gllm.fr
Thu Sep 26 11:02:50 CEST 2019


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 );
                 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



More information about the vlc-devel mailing list