[vlc-commits] [Git][videolan/vlc][master] 7 commits: input: decoder: fix flushing when deleting decoder
Steve Lhomme (@robUx4)
gitlab at videolan.org
Fri Mar 24 06:49:36 UTC 2023
Steve Lhomme pushed to branch master at VideoLAN / VLC
Commits:
23a0ea4b by Alexandre Janniaux at 2023-03-24T06:35:39+00:00
input: decoder: fix flushing when deleting decoder
An input_decoder instance can be deleted while its DecoderThread, the
thread calling decoder_t::pf_decode, is still running, meaning that
the deletion request happens asynchronously to the decoding process.
In particular, when the input_decoder instance is being deleted, an
aborting state is signalled to the DecoderThread and the input decoder
joins the thread afterwards.
In order to return and be joined, the DecoderThread needs to finish what
it was doing. In the case of a decoder, where input is paced by output
availability, it's likely that the decoder will get paced and will be
waiting on the decoder_t::pf_decode call. To finalize this call, the
decoder must be provided pictures back from the output, so that the
block from decoder_t::pf_decode can be queued.
At the beginning of the deletion, by setting p_owner->flushing to true,
we also prevent the decoder implementation to queue blocks into the
output (from commit 34a548cc02ce67920efbd7fdaa5af752199236a8). When
flushing the input decoder, the output was also correctly flushed
(tested by 91aabbf0668dffd352f029a10783916d97b80a94) which matches with
how the decoder implementation can be unblocked.
But in the case of closing, output flushing was only happening when the
output was paused, meaning that a deadlock could happen if every
pictures were queued to the output when the decoder would be deleted
while it was decoding another new block.
This issue was reproduced with the Videotoolbox decoder.
- - - - -
13ffce30 by Alexandre Janniaux at 2023-03-24T06:35:39+00:00
stream out: display: add explicit input decoder flush
Ensure the input_decoder is flushed before deletion, so that it's not
stuck waiting on the decoder implementation or the output for the ES.
The end goal is to simplify vlc_input_decoder_Delete to ensure it is
either flushed or drained before being deleted, so that the wanted
behaviour is written in the code and frames are neither dropped when
they should have been played, nor drained during interruption, resulting
in increased response time.
- - - - -
eb34fd0a by Alexandre Janniaux at 2023-03-24T06:35:39+00:00
input: es_out: add explicit input decoder flush
Ensure the input_decoder is flushed before deletion, so that it's not
stuck waiting on the decoder implementation or the output for the ES.
The end goal is to simplify vlc_input_decoder_Delete to ensure it is
either flushed or drained before being deleted, so that the wanted
behaviour is written in the code and frames are neither dropped when
they should have been played, nor drained during interruption, resulting
in increased response time.
- - - - -
cef87510 by Alexandre Janniaux at 2023-03-24T06:35:39+00:00
input: decoder: add explicit input decoder flush
Ensure the input_decoder is flushed before deletion, so that it's not
stuck waiting on the decoder implementation or the output for the ES.
The end goal is to simplify vlc_input_decoder_Delete to ensure it is
either flushed or drained before being deleted, so that the wanted
behaviour is written in the code and frames are neither dropped when
they should have been played, nor drained during interruption, resulting
in increased response time.
- - - - -
5f000b28 by Alexandre Janniaux at 2023-03-24T06:35:39+00:00
vlc_decoder.h: document vlc_input_decoder_Delete
Add documentation for the vlc_input_decoder_Delete function to enforce
usage of vlc_input_decoder_Flush or vlc_input_decoder_Drain before
destruction.
- - - - -
c9d874e7 by Alexandre Janniaux at 2023-03-24T06:35:39+00:00
input: decoder: separate flushing and aborting
No pictures should be queued when aborting the input_decoder via
`vlc_input_decoder_Delete` since it should have been either drained or
queued, but for now the behaviour was conservatively setting flushing to
true in the destructor to ensure the decoder implementation would not
queue anything from there to the output.
Change the check to account for this in release. Future commits will
probably add a debug check to ensure decoder implementations are
conformant to this behaviour and won't queue data from their close
function.
- - - - -
69abb520 by Alexandre Janniaux at 2023-03-24T06:35:39+00:00
input: decoder: remove flush workaround on Delete
Now that flush or drain is done before `vlc_input_decoder_Delete` and
that it has become mandatory, there is no need to flush during delete
preventively.
- - - - -
4 changed files:
- include/vlc_decoder.h
- modules/stream_out/display.c
- src/input/decoder.c
- src/input/es_out.c
Changes:
=====================================
include/vlc_decoder.h
=====================================
@@ -43,7 +43,21 @@ struct vlc_clock_t;
VLC_API vlc_input_decoder_t *
vlc_input_decoder_Create( vlc_object_t *, const es_format_t *,
struct vlc_clock_t *, input_resource_t * ) VLC_USED;
-VLC_API void vlc_input_decoder_Delete( vlc_input_decoder_t * );
+
+/**
+ * Delete an existing vlc_input_decoder_t instance.
+ *
+ * Close the decoder implementation and delete the vlc_input_decoder_t
+ * instance.
+ * The instance must have been drained using vlc_input_decoder_Drain() or
+ * flushed using vlc_input_decoder_Flush() after any previous call to
+ * vlc_input_decoder_Decode() before calling the destructor.
+ *
+ * @param decoder The vlc_input_decoder_t to delete, created from
+ * vlc_input_decoder_Create().
+ */
+VLC_API void vlc_input_decoder_Delete( vlc_input_decoder_t * decoder);
+
VLC_API void vlc_input_decoder_Decode( vlc_input_decoder_t *, block_t *, bool b_do_pace );
VLC_API void vlc_input_decoder_Drain( vlc_input_decoder_t * );
VLC_API void vlc_input_decoder_Flush( vlc_input_decoder_t * );
=====================================
modules/stream_out/display.c
=====================================
@@ -136,6 +136,7 @@ static void Del( sout_stream_t *p_stream, void *id )
(void) p_stream;
sout_stream_id_sys_t *id_sys = id;
+ vlc_input_decoder_Flush(id_sys->dec);
vlc_input_decoder_Delete( id_sys->dec );
sout_ClockDelete( id_sys->clock );
free( id_sys );
=====================================
src/input/decoder.c
=====================================
@@ -1159,7 +1159,7 @@ static void DecoderPlayCc( vlc_input_decoder_t *p_owner, vlc_frame_t *p_cc,
const decoder_cc_desc_t *p_desc )
{
vlc_fifo_Lock(p_owner->p_fifo);
- if (p_owner->flushing)
+ if (p_owner->flushing || p_owner->aborting)
{
vlc_fifo_Unlock(p_owner->p_fifo);
vlc_frame_Release(p_cc);
@@ -1242,7 +1242,7 @@ static int ModuleThread_PlayVideo( vlc_input_decoder_t *p_owner, picture_t *p_pi
return VLC_EGENERIC;
}
- if (p_owner->flushing)
+ if (p_owner->flushing || p_owner->aborting)
{
picture_Release(p_picture);
return VLC_SUCCESS;
@@ -1392,7 +1392,7 @@ static int ModuleThread_PlayAudio( vlc_input_decoder_t *p_owner, vlc_frame_t *p_
return VLC_EGENERIC;
}
- if (p_owner->flushing)
+ if (p_owner->flushing || p_owner->aborting)
{
block_Release(p_audio);
return VLC_SUCCESS;
@@ -2216,44 +2216,15 @@ void vlc_input_decoder_Delete( vlc_input_decoder_t *p_owner )
vlc_fifo_Lock( p_owner->p_fifo );
p_owner->aborting = true;
- p_owner->flushing = true;
p_owner->b_waiting = false;
vlc_fifo_Signal( p_owner->p_fifo );
/* Make sure we aren't waiting/decoding anymore */
vlc_cond_signal( &p_owner->wait_request );
-
- /* If the video output is paused or slow, or if the picture pool size was
- * under-estimated (e.g. greedy video filter, buggy decoder...), the
- * the picture pool may be empty, and the decoder thread or any decoder
- * module worker threads may be stuck waiting for free picture buffers.
- *
- * This unblocks the thread, allowing the decoder module to join all its
- * worker threads (if any) and the decoder thread to terminate. */
- if( p_dec->fmt_in->i_cat == VIDEO_ES && p_owner->p_vout != NULL
- && p_owner->vout_started )
- {
- if( p_owner->paused )
- {
- /* The DecoderThread could be stuck in pf_decode(). This is likely the
- * case with paused asynchronous decoder modules that have a limited
- * input and output pool size. Indeed, with such decoders, you have to
- * release an output buffer to get an input buffer. So, when paused and
- * flushed, the DecoderThread could be waiting for an output buffer to
- * be released (or rendered). In that case, the DecoderThread will
- * never be flushed since it be never leave pf_decode(). To fix this
- * issue, pre-flush the vout from here. The vout will have to be
- * flushed again since the module could be outputting more buffers just
- * after being unstuck. */
-
- vout_FlushAll( p_owner->p_vout );
- }
- }
vlc_fifo_Unlock( p_owner->p_fifo );
if( !vlc_input_decoder_IsSynchronous( p_owner ) )
vlc_join( p_owner->thread, NULL );
-
/* */
if( p_owner->cc.b_supported )
{
@@ -2494,6 +2465,7 @@ int vlc_input_decoder_SetCcState( vlc_input_decoder_t *p_owner, vlc_fourcc_t cod
else if( !p_ccowner->dec.p_module )
{
DecoderUnsupportedCodec( p_dec, &fmt, true );
+ vlc_input_decoder_Flush(p_ccowner);
vlc_input_decoder_Delete(p_ccowner);
vlc_mutex_unlock(&p_owner->cc.lock);
return VLC_EGENERIC;
@@ -2510,7 +2482,11 @@ int vlc_input_decoder_SetCcState( vlc_input_decoder_t *p_owner, vlc_fourcc_t cod
p_owner->cc.pp_decoder[i_channel] = NULL;
if( p_cc )
+ {
+
+ vlc_input_decoder_Flush(p_cc);
vlc_input_decoder_Delete(p_cc);
+ }
}
vlc_mutex_unlock(&p_owner->cc.lock);
return VLC_SUCCESS;
=====================================
src/input/es_out.c
=====================================
@@ -631,7 +631,10 @@ static void EsOutTerminate( es_out_t *out )
foreach_es_then_es_slaves(es)
{
if (es->p_dec != NULL)
+ {
+ vlc_input_decoder_Flush(es->p_dec);
vlc_input_decoder_Delete(es->p_dec);
+ }
EsTerminate(es);
EsRelease(es);
@@ -832,6 +835,7 @@ static int EsOutSetRecord( es_out_t *out, bool b_record, const char *dir_path )
if( !p_es->p_dec_record )
continue;
+ vlc_input_decoder_Flush(p_es->p_dec_record);
vlc_input_decoder_Delete( p_es->p_dec_record );
p_es->p_dec_record = NULL;
}
@@ -2355,6 +2359,7 @@ static void EsOutDestroyDecoder( es_out_t *out, es_out_id_t *p_es )
assert( p_es->p_pgrm );
+ vlc_input_decoder_Flush(p_es->p_dec);
vlc_input_decoder_Delete( p_es->p_dec );
p_es->p_dec = NULL;
if( p_es->p_pgrm->p_master_es_clock == p_es->p_clock )
@@ -2364,6 +2369,7 @@ static void EsOutDestroyDecoder( es_out_t *out, es_out_id_t *p_es )
if( p_es->p_dec_record )
{
+ vlc_input_decoder_Flush(p_es->p_dec_record);
vlc_input_decoder_Delete( p_es->p_dec_record );
p_es->p_dec_record = NULL;
}
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/f7bb59d9f51cc10b25ff86d34a3eff744e60c46e...69abb5200c56e32aea87210460c86d2b75d3682f
--
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/f7bb59d9f51cc10b25ff86d34a3eff744e60c46e...69abb5200c56e32aea87210460c86d2b75d3682f
You're receiving this email because of your account on code.videolan.org.
VideoLAN code repository instance
More information about the vlc-commits
mailing list