[vlc-devel] [RFC PATCH] decoder: Trying to fix the fmt_in locking issue
Thomas Guillem
thomas at gllm.fr
Thu Sep 3 17:09:09 CEST 2015
This RFC patch follows the discussion we had with Rémi in the "[PATCH 01/11]
decoder: add decoder_OnOutputReady" thread.
I'm starting with the point 2/ (fmt_in) since I'm more familiar with it.
First, I don't know if it's possible to have fmt_in in read only. Indeed, if we
choose that solution, we'll need to pass the new fmt from the packetizer by an
other way (by adding a new pkt_fmt in decoder_t ?) and we'll need to patch all
decoder modules.
My solution: add 2 new functions: decoder_FmtInLock and decoder_FmtInUnlock.
Theses functions are used to lock/unlock the fmt_in when it's accessed from an
other thread than the DecoderThread.
Asynchronous decoder modules will have to lock fmt_in before accessing it from
an other thread (and also before calling decoder_NewPicture and
decoder_Update*Format). The fmt_in will also be locked by the DecoderThread
when it's modified by the packetizer.
The fmt_in won't be locked in pf_decode* callbacks, and synchronous decoder
modules won't have to lock it since it can only be modified from the same
thread. Obviously, fmt_in should stay read_only for decoder modules.
For avcodec, fmt_in should only be locked from the get_buffer2 and the
get_format callbacks.
I don't know if b_async is really useful for now, we can always lock fmt_in
even with sync decoders.
Maybe we can rename decoder_FmtInLock to a more generic decoder_Lock that will
be used for fmt_out and others functions.
---
include/vlc_codec.h | 5 +++++
modules/codec/avcodec/video.c | 6 ++++++
src/input/decoder.c | 24 ++++++++++++++++++++++++
src/libvlccore.sym | 2 ++
4 files changed, 37 insertions(+)
diff --git a/include/vlc_codec.h b/include/vlc_codec.h
index 0c43978..5194d12 100644
--- a/include/vlc_codec.h
+++ b/include/vlc_codec.h
@@ -68,6 +68,8 @@ struct decoder_t
/* Tell the decoder if it is allowed to drop frames */
bool b_frame_drop_allowed;
+ bool b_async;
+
/* */
picture_t * ( * pf_decode_video )( decoder_t *, block_t ** );
block_t * ( * pf_decode_audio )( decoder_t *, block_t ** );
@@ -241,6 +243,9 @@ VLC_API mtime_t decoder_GetDisplayDate( decoder_t *, mtime_t ) VLC_USED;
*/
VLC_API int decoder_GetDisplayRate( decoder_t * ) VLC_USED;
+VLC_API void decoder_FmtInLock( decoder_t * );
+VLC_API void decoder_FmtInUnlock( decoder_t * );
+
/** @} */
/** @} */
#endif /* _VLC_CODEC_H */
diff --git a/modules/codec/avcodec/video.c b/modules/codec/avcodec/video.c
index 7e05a5f..e07b869 100644
--- a/modules/codec/avcodec/video.c
+++ b/modules/codec/avcodec/video.c
@@ -497,6 +497,7 @@ int InitVideoDec( decoder_t *p_dec, AVCodecContext *p_context,
if ( p_dec->fmt_in.i_codec == VLC_CODEC_VP9 )
p_dec->b_need_packetized = true;
+ p_dec->b_async = i_thread_count > 1;
return VLC_SUCCESS;
}
@@ -1080,7 +1081,9 @@ static int lavc_GetFrame(struct AVCodecContext *ctx, AVFrame *frame, int flags)
}
/* The semaphore protects updates to fmt_out */
+ decoder_FmtInLock(dec);
pic = ffmpeg_NewPictBuf(dec, ctx);
+ decoder_FmtInUnlock(dec);
post_mt(sys);
if (pic == NULL)
return -1;
@@ -1130,6 +1133,7 @@ static enum PixelFormat ffmpeg_GetFormat( AVCodecContext *p_context,
wait_mt(p_sys);
+ decoder_FmtInLock(p_dec);
for( size_t i = 0; pi_fmt[i] != AV_PIX_FMT_NONE; i++ )
{
enum PixelFormat hwfmt = pi_fmt[i];
@@ -1160,6 +1164,7 @@ static enum PixelFormat ffmpeg_GetFormat( AVCodecContext *p_context,
continue;
}
+ decoder_FmtInUnlock(p_dec);
post_mt(p_sys);
if (va->description != NULL)
@@ -1174,6 +1179,7 @@ static enum PixelFormat ffmpeg_GetFormat( AVCodecContext *p_context,
return pi_fmt[i];
}
+ decoder_FmtInUnlock(p_dec);
post_mt(p_sys);
/* Fallback to default behaviour */
return swfmt;
diff --git a/src/input/decoder.c b/src/input/decoder.c
index 0fad3f9..bdf38c9 100644
--- a/src/input/decoder.c
+++ b/src/input/decoder.c
@@ -87,6 +87,7 @@ struct decoder_owner_sys_t
/* Lock for communication with decoder thread */
vlc_mutex_t lock;
+ vlc_mutex_t fmt_in_lock;
vlc_cond_t wait_request;
vlc_cond_t wait_acknowledge;
vlc_cond_t wait_fifo; /* TODO: merge with wait_acknowledge */
@@ -594,6 +595,20 @@ int decoder_GetDisplayRate( decoder_t *p_dec )
return p_dec->pf_get_display_rate( p_dec );
}
+/* decoder_FmtInLock
+ */
+void decoder_FmtInLock( decoder_t *p_dec )
+{
+ if( p_dec->b_async )
+ vlc_mutex_lock( &p_dec->p_owner->fmt_in_lock );
+}
+/* decoder_UnlockFmtIn
+ */
+void decoder_FmtInUnlock( decoder_t *p_dec )
+{
+ if( p_dec->b_async )
+ vlc_mutex_unlock( &p_dec->p_owner->fmt_in_lock );
+}
static bool DecoderWaitUnblock( decoder_t *p_dec )
{
@@ -764,6 +779,7 @@ static void DecoderProcessSout( decoder_t *p_dec, block_t *p_block )
DecoderUpdateFormatLocked( p_dec );
vlc_mutex_unlock( &p_owner->lock );
+ decoder_FmtInLock( p_dec );
p_owner->fmt.i_group = p_dec->fmt_in.i_group;
p_owner->fmt.i_id = p_dec->fmt_in.i_id;
if( p_dec->fmt_in.psz_language )
@@ -772,6 +788,7 @@ static void DecoderProcessSout( decoder_t *p_dec, block_t *p_block )
p_owner->fmt.psz_language =
strdup( p_dec->fmt_in.psz_language );
}
+ decoder_FmtInUnlock( p_dec );
p_owner->p_sout_input =
sout_InputNew( p_owner->p_sout, &p_owner->fmt );
@@ -998,6 +1015,7 @@ static void DecoderProcessVideo( decoder_t *p_dec, block_t *p_block, bool b_flus
while( (p_packetized_block =
p_packetizer->pf_packetize( p_packetizer, p_block ? &p_block : NULL )) )
{
+ decoder_FmtInLock( p_dec );
if( p_packetizer->fmt_out.i_extra && !p_dec->fmt_in.i_extra )
{
es_format_Clean( &p_dec->fmt_in );
@@ -1017,6 +1035,7 @@ static void DecoderProcessVideo( decoder_t *p_dec, block_t *p_block, bool b_flus
p_dec->fmt_in.video.i_sar_den=
p_packetizer->fmt_out.video.i_sar_den;
}
+ decoder_FmtInUnlock( p_dec );
if( p_packetizer->pf_get_cc )
DecoderGetCc( p_dec, p_packetizer );
@@ -1178,11 +1197,13 @@ static void DecoderProcessAudio( decoder_t *p_dec, block_t *p_block, bool b_flus
while( (p_packetized_block =
p_packetizer->pf_packetize( p_packetizer, p_block ? &p_block : NULL )) )
{
+ decoder_FmtInLock( p_dec );
if( p_packetizer->fmt_out.i_extra && !p_dec->fmt_in.i_extra )
{
es_format_Clean( &p_dec->fmt_in );
es_format_Copy( &p_dec->fmt_in, &p_packetizer->fmt_out );
}
+ decoder_FmtInUnlock( p_dec );
while( p_packetized_block )
{
@@ -1481,6 +1502,7 @@ static decoder_t * CreateDecoder( vlc_object_t *p_parent,
return NULL;
p_dec->b_frame_drop_allowed = true;
+ p_dec->b_async = false;
p_dec->pf_decode_audio = NULL;
p_dec->pf_decode_video = NULL;
p_dec->pf_decode_sub = NULL;
@@ -1545,6 +1567,7 @@ static decoder_t * CreateDecoder( vlc_object_t *p_parent,
}
vlc_mutex_init( &p_owner->lock );
+ vlc_mutex_init( &p_owner->fmt_in_lock );
vlc_cond_init( &p_owner->wait_request );
vlc_cond_init( &p_owner->wait_acknowledge );
vlc_cond_init( &p_owner->wait_fifo );
@@ -1709,6 +1732,7 @@ static void DeleteDecoder( decoder_t * p_dec )
vlc_cond_destroy( &p_owner->wait_fifo );
vlc_cond_destroy( &p_owner->wait_acknowledge );
vlc_cond_destroy( &p_owner->wait_request );
+ vlc_mutex_destroy( &p_owner->fmt_in_lock );
vlc_mutex_destroy( &p_owner->lock );
vlc_object_release( p_dec );
diff --git a/src/libvlccore.sym b/src/libvlccore.sym
index 372b312..01a4c1d 100644
--- a/src/libvlccore.sym
+++ b/src/libvlccore.sym
@@ -72,6 +72,8 @@ date_Increment
date_Init
date_Move
date_Set
+decoder_FmtInLock
+decoder_FmtInUnlock
decoder_GetDisplayDate
decoder_GetDisplayRate
decoder_GetInputAttachments
--
2.1.4
More information about the vlc-devel
mailing list