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

[Thomas Guillem]

There was a data-race between any avcodec threads and the pf_decode thread. The
scope of sem wait/post need to be increased. Indeed, dec->fmt_out must be
accessed while being protected, the same for lavc_UpdateVideoFormat() and
decoder_NewPicture().

PS: decoder_QueuePicture() doesn'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.
---
 modules/codec/avcodec/video.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/modules/codec/avcodec/video.c b/modules/codec/avcodec/video.c
index 2232b7a291..fea2a7979b 100644
--- a/modules/codec/avcodec/video.c
+++ b/modules/codec/avcodec/video.c
@@ -1120,6 +1120,7 @@ static int DecodeBlock( decoder_t *p_dec, block_t **pp_block )
                     b_error = true;
                 }
                 av_packet_unref( &pkt );
+                wait_mt( p_sys );
                 break;
             }
 
@@ -1142,6 +1143,7 @@ static int DecodeBlock( decoder_t *p_dec, block_t **pp_block )
         if (unlikely(frame == NULL))
         {
             b_error = true;
+            wait_mt( p_sys );
             break;
         }
 
@@ -1155,12 +1157,13 @@ static int DecodeBlock( decoder_t *p_dec, block_t **pp_block )
             }
             av_frame_free(&frame);
             if( ret == AVERROR_EOF )
+            {
+                wait_mt( p_sys );
                 break;
+            }
         }
         bool not_received_frame = ret;
 
-        wait_mt( p_sys );
-
         if( p_block )
         {
             /* Consumed bytes */
@@ -1172,6 +1175,7 @@ static int DecodeBlock( decoder_t *p_dec, block_t **pp_block )
         if( not_received_frame )
         {
             av_frame_free(&frame);
+            wait_mt( p_sys );
             if( i_used == 0 ) break;
             continue;
         }
@@ -1207,6 +1211,7 @@ static int DecodeBlock( decoder_t *p_dec, block_t **pp_block )
              !p_sys->b_show_corrupted ) )
         {
             av_frame_free(&frame);
+            wait_mt( p_sys );
             continue;
         }
 
@@ -1225,6 +1230,7 @@ static int DecodeBlock( decoder_t *p_dec, block_t **pp_block )
             {
                 b_error = true;
                 av_frame_free(&frame);
+                wait_mt( p_sys );
                 break;
             }
             static_assert( sizeof(p_palette->palette) == AVPALETTE_SIZE,
@@ -1236,6 +1242,7 @@ static int DecodeBlock( decoder_t *p_dec, block_t **pp_block )
             if( decoder_UpdateVideoFormat( p_dec ) )
             {
                 av_frame_free(&frame);
+                wait_mt( p_sys );
                 continue;
             }
         }
@@ -1253,6 +1260,7 @@ static int DecodeBlock( decoder_t *p_dec, block_t **pp_block )
             if( !p_pic )
             {
                 av_frame_free(&frame);
+                wait_mt( p_sys );
                 break;
             }
 
@@ -1261,6 +1269,7 @@ static int DecodeBlock( decoder_t *p_dec, block_t **pp_block )
             {
                 av_frame_free(&frame);
                 picture_Release( p_pic );
+                wait_mt( p_sys );
                 break;
             }
         }
@@ -1275,6 +1284,7 @@ static int DecodeBlock( decoder_t *p_dec, block_t **pp_block )
             if( unlikely(p_pic == NULL) )
             {
                 av_frame_free(&frame);
+                wait_mt( p_sys );
                 break;
             }
         }
@@ -1312,11 +1322,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 */
-- 
2.20.1