[vlc-devel] [PATCH] RFC: avcodec: use a mutex instead of sem
Thomas Guillem
thomas at gllm.fr
Wed Sep 25 15:38:05 CEST 2019
RFC: This commit has been tested with/without va, on some of my samples. It
could needs a more extensive testing. Do you some corner case for me to test ?
Using a mutex instead of a sem allows avcodec callbacks to not wait for a
future pf_decode and pf_flush call.
The mutex protect the pts, the p_va, the dec->fmt_out variables, the
decoder_UpdateVideoFormat() and the decoder_NewPicture() call. Indeed,
decoder_UpdateVideoFormat() is not reentrant and the caller should take care
about serialization.
This fixes the following 3 data-race (happening with/without va, and while
seeking...):
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 | 74 ++++++++++++++++-------------------
1 file changed, 34 insertions(+), 40 deletions(-)
diff --git a/modules/codec/avcodec/video.c b/modules/codec/avcodec/video.c
index 2232b7a291..f68f97eeb6 100644
--- a/modules/codec/avcodec/video.c
+++ b/modules/codec/avcodec/video.c
@@ -66,7 +66,7 @@ typedef struct
const AVCodec *p_codec;
/* Video decoder specific part */
- date_t pts;
+ date_t pts; /* Protected by lock */
/* Closed captions for decoders */
cc_data_t cc;
@@ -102,24 +102,16 @@ typedef struct
bool palette_sent;
/* VA API */
- vlc_va_t *p_va;
+ vlc_va_t *p_va; /* Protected by lock */
enum PixelFormat pix_fmt;
int profile;
int level;
- vlc_sem_t sem_mt;
+ /* Protect dec->fmt_out, decoder_Update*() and decoder_NewPicture()
+ * functions */
+ vlc_mutex_t lock;
} decoder_sys_t;
-static inline void wait_mt(decoder_sys_t *sys)
-{
- vlc_sem_wait(&sys->sem_mt);
-}
-
-static inline void post_mt(decoder_sys_t *sys)
-{
- vlc_sem_post(&sys->sem_mt);
-}
-
/*****************************************************************************
* Local prototypes
*****************************************************************************/
@@ -486,9 +478,7 @@ static int OpenVideoCodec( decoder_t *p_dec )
ctx->active_thread_type = FF_THREAD_SLICE;
}
- post_mt( p_sys );
ret = ffmpeg_OpenCodec( p_dec, ctx, codec );
- wait_mt( p_sys );
if( ret < 0 )
return ret;
@@ -542,7 +532,7 @@ int InitVideoDec( vlc_object_t *obj )
p_sys->p_context = p_context;
p_sys->p_codec = p_codec;
p_sys->p_va = NULL;
- vlc_sem_init( &p_sys->sem_mt, 0 );
+ vlc_mutex_init( &p_sys->lock );
/* ***** Fill p_context with init values ***** */
p_context->codec_tag = ffmpeg_CodecTag( p_dec->fmt_in.i_original_fourcc ?
@@ -687,7 +677,7 @@ int InitVideoDec( vlc_object_t *obj )
/* ***** Open the codec ***** */
if( OpenVideoCodec( p_dec ) < 0 )
{
- vlc_sem_destroy( &p_sys->sem_mt );
+ vlc_mutex_destroy( &p_sys->lock );
free( p_sys );
avcodec_free_context( &p_context );
return VLC_EGENERIC;
@@ -722,11 +712,9 @@ static void Flush( decoder_t *p_dec )
* and workers threads */
decoder_AbortPictures( p_dec, true );
- post_mt( p_sys );
/* do not flush buffers if codec hasn't been opened (theora/vorbis/VC1) */
if( avcodec_is_open( p_context ) )
avcodec_flush_buffers( p_context );
- wait_mt( p_sys );
/* Reset cancel state to false */
decoder_AbortPictures( p_dec, false );
@@ -1071,8 +1059,6 @@ static int DecodeBlock( decoder_t *p_dec, block_t **pp_block )
{
int i_used = 0;
- post_mt( p_sys );
-
if( (p_block && p_block->i_buffer > 0) || b_drain )
{
AVPacket pkt;
@@ -1159,8 +1145,6 @@ static int DecodeBlock( decoder_t *p_dec, block_t **pp_block )
}
bool not_received_frame = ret;
- wait_mt( p_sys );
-
if( p_block )
{
/* Consumed bytes */
@@ -1180,6 +1164,8 @@ static int DecodeBlock( decoder_t *p_dec, block_t **pp_block )
if( p_frame_info->b_eos )
p_sys->b_first_frame = true;
+ vlc_mutex_lock(&p_sys->lock);
+
/* Compute the PTS */
#ifdef FF_API_PKT_PTS
int64_t av_pts = frame->pts == AV_NOPTS_VALUE ? frame->pkt_dts : frame->pts;
@@ -1207,6 +1193,7 @@ static int DecodeBlock( decoder_t *p_dec, block_t **pp_block )
!p_sys->b_show_corrupted ) )
{
av_frame_free(&frame);
+ vlc_mutex_unlock(&p_sys->lock);
continue;
}
@@ -1225,6 +1212,7 @@ static int DecodeBlock( decoder_t *p_dec, block_t **pp_block )
{
b_error = true;
av_frame_free(&frame);
+ vlc_mutex_unlock(&p_sys->lock);
break;
}
static_assert( sizeof(p_palette->palette) == AVPALETTE_SIZE,
@@ -1236,6 +1224,7 @@ static int DecodeBlock( decoder_t *p_dec, block_t **pp_block )
if( decoder_UpdateVideoFormat( p_dec ) )
{
av_frame_free(&frame);
+ vlc_mutex_unlock(&p_sys->lock);
continue;
}
}
@@ -1253,6 +1242,7 @@ static int DecodeBlock( decoder_t *p_dec, block_t **pp_block )
if( !p_pic )
{
av_frame_free(&frame);
+ vlc_mutex_unlock(&p_sys->lock);
break;
}
@@ -1261,6 +1251,7 @@ static int DecodeBlock( decoder_t *p_dec, block_t **pp_block )
{
av_frame_free(&frame);
picture_Release( p_pic );
+ vlc_mutex_unlock(&p_sys->lock);
break;
}
}
@@ -1275,6 +1266,7 @@ static int DecodeBlock( decoder_t *p_dec, block_t **pp_block )
if( unlikely(p_pic == NULL) )
{
av_frame_free(&frame);
+ vlc_mutex_unlock(&p_sys->lock);
break;
}
}
@@ -1312,11 +1304,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;
+ vlc_mutex_unlock(&p_sys->lock);
decoder_QueueVideo( p_dec, p_pic );
}
else
+ {
+ vlc_mutex_unlock(&p_sys->lock);
picture_Release( p_pic );
-
+ }
} while( true );
/* After draining, we need to reset decoder with a flush */
@@ -1369,14 +1364,10 @@ void EndVideoDec( vlc_object_t *obj )
decoder_sys_t *p_sys = p_dec->p_sys;
AVCodecContext *ctx = p_sys->p_context;
- post_mt( p_sys );
-
/* do not flush buffers if codec hasn't been opened (theora/vorbis/VC1) */
if( avcodec_is_open( ctx ) )
avcodec_flush_buffers( ctx );
- wait_mt( p_sys );
-
cc_Flush( &p_sys->cc );
avcodec_free_context( &ctx );
@@ -1384,7 +1375,7 @@ void EndVideoDec( vlc_object_t *obj )
if( p_sys->p_va )
vlc_va_Delete( p_sys->p_va );
- vlc_sem_destroy( &p_sys->sem_mt );
+ vlc_mutex_destroy( &p_sys->lock );
free( p_sys );
}
@@ -1583,12 +1574,12 @@ static int lavc_GetFrame(struct AVCodecContext *ctx, AVFrame *frame, int flags)
}
frame->opaque = NULL;
- wait_mt(sys);
+ vlc_mutex_lock(&sys->lock);
if (sys->p_va == NULL)
{
if (!sys->b_direct_rendering)
{
- post_mt(sys);
+ vlc_mutex_unlock(&sys->lock);
return avcodec_default_get_buffer2(ctx, frame, flags);
}
@@ -1597,18 +1588,22 @@ static int lavc_GetFrame(struct AVCodecContext *ctx, AVFrame *frame, int flags)
* to protect p_dec->fmt_out. */
if (lavc_UpdateVideoFormat(dec, ctx, ctx->pix_fmt, ctx->pix_fmt))
{
- post_mt(sys);
+ vlc_mutex_unlock(&sys->lock);
return -1;
}
}
- post_mt(sys);
if (sys->p_va != NULL)
- return lavc_va_GetFrame(ctx, frame);
+ {
+ int ret = lavc_va_GetFrame(ctx, frame);
+ vlc_mutex_unlock(&sys->lock);
+ 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);
+ vlc_mutex_unlock(&sys->lock);
if (ret)
ret = avcodec_default_get_buffer2(ctx, frame, flags);
return ret;
@@ -1696,7 +1691,7 @@ no_reuse:
}
#endif
- wait_mt(p_sys);
+ vlc_mutex_lock(&p_sys->lock);
static const enum PixelFormat hwfmts[] =
{
@@ -1731,29 +1726,28 @@ no_reuse:
msg_Dbg(p_dec, "trying format %s", dsc ? dsc->name : "unknown");
if (lavc_UpdateVideoFormat(p_dec, p_context, hwfmt, swfmt))
continue; /* Unsupported brand of hardware acceleration */
- post_mt(p_sys);
+ vlc_mutex_unlock(&p_sys->lock);
picture_t *test_pic = decoder_NewPicture(p_dec);
assert(!test_pic || test_pic->format.i_chroma == p_dec->fmt_out.video.i_chroma);
vlc_va_t *va = vlc_va_New(VLC_OBJECT(p_dec), p_context, hwfmt,
&p_dec->fmt_in,
test_pic ? test_pic->p_sys : NULL);
+ vlc_mutex_lock(&p_sys->lock);
if (test_pic)
picture_Release(test_pic);
if (va == NULL)
- {
- wait_mt(p_sys);
continue; /* Unsupported codec profile or such */
- }
p_sys->p_va = va;
p_sys->pix_fmt = hwfmt;
p_context->draw_horiz_band = NULL;
+ vlc_mutex_unlock(&p_sys->lock);
return hwfmt;
}
- post_mt(p_sys);
/* Fallback to default behaviour */
p_sys->pix_fmt = swfmt;
+ vlc_mutex_unlock(&p_sys->lock);
return swfmt;
}
--
2.20.1
More information about the vlc-devel
mailing list