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

[Steve Lhomme]

On 2019-09-20 9:31, Thomas Guillem wrote:
> 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.

Aren't we supposed to queue pictures in display order ? In which case we 
have to protect it from other decoder threads trying to queue future 
pictures in our back.

> ---
>   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
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>