[vlc-devel] [PATCH] [RFC] decoder: add a Flush callback to decoders/packetizers
Steve Lhomme
robux4 at gmail.com
Thu Nov 19 09:12:42 CET 2015
On Wed, Nov 18, 2015 at 5:23 PM, Thomas Guillem <thomas at gllm.fr> wrote:
>
>
> On Wed, Nov 18, 2015, at 09:33, Steve Lhomme wrote:
>> The flush handling is not passed to the DecoderProcessXXX functions
>> anymore
>>
>> All decoder & packetizer, aout/vout/spu flushing is done in one place.
>> The sout flushing will be part of another patch.
>> --
>> deprecates https://patches.videolan.org/patch/10776/
>> ---
>> include/vlc_codec.h | 1 +
>> src/input/decoder.c | 145
>> ++++++++++++++++++++++++++++++----------------------
>> 2 files changed, 85 insertions(+), 61 deletions(-)
>>
>> diff --git a/include/vlc_codec.h b/include/vlc_codec.h
>> index 1a2baa2..1399d26 100644
>> --- a/include/vlc_codec.h
>> +++ b/include/vlc_codec.h
>> @@ -70,6 +70,7 @@ struct decoder_t
>> block_t * ( * pf_decode_audio )( decoder_t *, block_t **
>> );
>> subpicture_t * ( * pf_decode_sub) ( decoder_t *, block_t **
>> );
>> block_t * ( * pf_packetize ) ( decoder_t *, block_t **
>> );
>> + int ( * pf_flush ) ( decoder_t * );
>>
>> /* Closed Caption (CEA 608/708) extraction.
>> * If set, it *may* be called after pf_decode_video/pf_packetize
>> diff --git a/src/input/decoder.c b/src/input/decoder.c
>> index cacf7f2..b1adac3 100644
>> --- a/src/input/decoder.c
>> +++ b/src/input/decoder.c
>> @@ -134,6 +134,8 @@ struct decoder_owner_sys_t
>> /* */
>> #define DECODER_SPU_VOUT_WAIT_DURATION ((int)(0.200*CLOCK_FREQ))
>>
>> +static int DecoderFlush( decoder_t *p_dec );
>> +
>> /**
>> * Load a decoder module
>> */
>> @@ -148,6 +150,7 @@ static int LoadDecoder( decoder_t *p_dec, bool
>> b_packetizer,
>> p_dec->pf_decode_sub = NULL;
>> p_dec->pf_get_cc = NULL;
>> p_dec->pf_packetize = NULL;
>> + p_dec->pf_flush = DecoderFlush;
>>
>> es_format_Copy( &p_dec->fmt_in, p_fmt );
>> es_format_Init( &p_dec->fmt_out, UNKNOWN_ES, 0 );
>> @@ -224,6 +227,25 @@ static block_t *DecoderBlockFlushNew()
>> return p_null;
>> }
>>
>> +int DecoderFlush( decoder_t *p_dec )
>> +{
>> + block_t *p_flush = DecoderBlockFlushNew();
>> + if( p_flush )
>> + {
>> + if (p_dec->pf_decode_video)
> compare p_dec->fmt_out.i_cat to stay consistent ? (see if(
> p_dec->fmt_out.i_cat == AUDIO_ES )...)
>
>> + p_dec->pf_decode_video( p_dec, &p_flush );
>> + else if (p_dec->pf_decode_audio)
>> + p_dec->pf_decode_audio( p_dec, &p_flush );
>> + else if (p_dec->pf_decode_sub)
>> + p_dec->pf_decode_sub( p_dec, &p_flush );
>> + else if (p_dec->pf_packetize)
>> + p_dec->pf_packetize( p_dec, &p_flush );
>> + else
>> + block_Release( p_flush );
>
> also return VLC_EGENERIC;
>
>> + }
>
> else return VLC_EGENERIC;
>
>> + return VLC_SUCCESS;
>> +}
>> +
>> /*****************************************************************************
>> * Buffers allocation callbacks for the decoders
>> *****************************************************************************/
>> @@ -945,6 +967,13 @@ static void DecoderProcessVideo( decoder_t *p_dec,
>> block_t *p_block, bool b_flus
> Don't compile ? typo ? the b_flush arg should be gone for video too ?
>> {
>> decoder_owner_sys_t *p_owner = (decoder_owner_sys_t
>> *)p_dec->p_owner;
>>
>> + if ( b_flush )
>> + {
>> + if( p_owner->p_vout )
>> + vout_Flush( p_owner->p_vout, VLC_TS_0 );
>> + return;
>> + }
>> +
>> if( p_owner->p_packetizer )
>> {
>> block_t *p_packetized_block;
>> @@ -982,22 +1011,11 @@ static void DecoderProcessVideo( decoder_t *p_dec,
>> block_t *p_block, bool b_flus
>> p_packetized_block = p_next;
>> }
>> }
>> - /* The packetizer does not output a block that tell the decoder
>> to flush
>> - * do it ourself */
>> - if( b_flush )
>> - {
>> - block_t *p_null = DecoderBlockFlushNew();
>> - if( p_null )
>> - DecoderDecodeVideo( p_dec, p_null );
>> - }
>> }
>> else
>> {
>> DecoderDecodeVideo( p_dec, p_block );
>> }
>> -
>> - if( b_flush && p_owner->p_vout )
>> - vout_Flush( p_owner->p_vout, VLC_TS_INVALID+1 );
>> }
>>
>> static void DecoderPlayAudio( decoder_t *p_dec, block_t *p_audio,
>> @@ -1096,7 +1114,7 @@ static void DecoderDecodeAudio( decoder_t *p_dec,
>> block_t *p_block )
>>
>> /* This function process a audio block
>> */
>> -static void DecoderProcessAudio( decoder_t *p_dec, block_t *p_block,
>> bool b_flush )
>> +static void DecoderProcessAudio( decoder_t *p_dec, block_t *p_block )
>> {
>> decoder_owner_sys_t *p_owner = (decoder_owner_sys_t
>> *)p_dec->p_owner;
>>
>> @@ -1134,22 +1152,11 @@ static void DecoderProcessAudio( decoder_t
>> *p_dec, block_t *p_block, bool b_flus
>> p_packetized_block = p_next;
>> }
>> }
>> - /* The packetizer does not output a block that tell the decoder
>> to flush
>> - * do it ourself */
>> - if( b_flush )
>> - {
>> - block_t *p_null = DecoderBlockFlushNew();
>> - if( p_null )
>> - DecoderDecodeAudio( p_dec, p_null );
>> - }
>> }
>> else
>> {
>> DecoderDecodeAudio( p_dec, p_block );
>> }
>> -
>> - if( b_flush && p_owner->p_aout )
>> - aout_DecFlush( p_owner->p_aout, false );
>> }
>>
>> static void DecoderPlaySpu( decoder_t *p_dec, subpicture_t *p_subpic )
>> @@ -1191,7 +1198,7 @@ static void DecoderPlaySpu( decoder_t *p_dec,
>> subpicture_t *p_subpic )
>>
>> /* This function process a subtitle block
>> */
>> -static void DecoderProcessSpu( decoder_t *p_dec, block_t *p_block, bool
>> b_flush )
>> +static void DecoderProcessSpu( decoder_t *p_dec, block_t *p_block )
>> {
>> decoder_owner_sys_t *p_owner = p_dec->p_owner;
>>
>> @@ -1230,17 +1237,6 @@ static void DecoderProcessSpu( decoder_t *p_dec,
>> block_t *p_block, bool b_flush
>> if( p_vout )
>> vlc_object_release( p_vout );
>> }
>> -
>> - if( b_flush && p_owner->p_spu_vout )
>> - {
>> - p_vout = input_resource_HoldVout( p_owner->p_resource );
>> -
>> - if( p_vout && p_owner->p_spu_vout == p_vout )
>> - vout_FlushSubpictureChannel( p_vout, p_owner->i_spu_channel
>> );
>> -
>> - if( p_vout )
>> - vlc_object_release( p_vout );
>> - }
>> }
>>
>> /**
>> @@ -1269,45 +1265,72 @@ static void DecoderProcess( decoder_t *p_dec,
>> block_t *p_block )
>> return;
>> }
>>
>> -#ifdef ENABLE_SOUT
>> - if( p_owner->b_packetizer )
>> - {
>> - if( p_block )
>> - p_block->i_flags &= ~BLOCK_FLAG_CORE_PRIVATE_MASK;
>> + bool b_flush = false;
>>
>> - DecoderProcessSout( p_dec, p_block );
>> - }
>> - else
>> -#endif
>> + if( p_block )
>> {
>> - bool b_flush = false;
>> -
>> - if( p_block )
>> - {
>> - const bool b_flushing = p_owner->i_preroll_end == INT64_MAX;
>> - DecoderUpdatePreroll( &p_owner->i_preroll_end, p_block );
>> + const bool b_flushing = p_owner->i_preroll_end == INT64_MAX;
>> + DecoderUpdatePreroll( &p_owner->i_preroll_end, p_block );
>>
>> - b_flush = !b_flushing && b_flush_request;
>> + b_flush = !b_flushing && b_flush_request;
>
> Here, b_flush_request comes from b_flush_request = p_block &&
> (p_block->i_flags & BLOCK_FLAG_CORE_FLUSH);
>
> It should not, you should pass the b_flush_request boolean directly to
> DecoderProcess function. (see /* TODO: add a flush callback to
> decoder... */)
> You should also remove the BLOCK_FLAG_CORE_FLUSH internal flag.
>
>>
>> - p_block->i_flags &= ~BLOCK_FLAG_CORE_PRIVATE_MASK;
>> - }
>> + p_block->i_flags &= ~BLOCK_FLAG_CORE_PRIVATE_MASK;
>> + }
>>
>> - if( p_dec->fmt_out.i_cat == AUDIO_ES )
>> + if (b_flush)
>> + {
>> + if ( p_block )
>> {
>> - DecoderProcessAudio( p_dec, p_block, b_flush );
>> + block_Release( p_block );
>> + p_block = NULL;
>> }
>> - else if( p_dec->fmt_out.i_cat == VIDEO_ES )
>> +
>> + if ( p_owner->p_packetizer )
>> + p_owner->p_packetizer->pf_flush( p_owner->p_packetizer );
>> + p_dec->pf_flush( p_dec );
>
> both returns not checked.
>
> Maybe pf_flush shouldn't return anything to stay consistent with
> vlc_filter.h ?
> The question is: can decoder/packetizer modules flush fail ?
av_parser cannot be flushed properly. The only way is to recreate an
instance. If it fails the packetizer will be in an unknown state. But
that can be handled at that level.
>> +
>> + if( p_owner->p_vout )
>> + vout_Flush( p_owner->p_vout, VLC_TS_0 );
>> + if( p_owner->p_aout )
>> + aout_DecFlush( p_owner->p_aout, false );
>> + if( p_owner->p_spu_vout )
>> {
>> - DecoderProcessVideo( p_dec, p_block, b_flush );
>> + vout_thread_t *p_vout = input_resource_HoldVout(
>> p_owner->p_resource );
>> +
>> + if( p_vout && p_owner->p_spu_vout == p_vout )
>> + vout_FlushSubpictureChannel( p_vout,
>> p_owner->i_spu_channel );
>> +
>> + if( p_vout )
>> + vlc_object_release( p_vout );
>> }
>> - else if( p_dec->fmt_out.i_cat == SPU_ES )
>> + }
>> + else
>> + {
>> +#ifdef ENABLE_SOUT
>> + if( p_owner->b_packetizer )
>> {
>> - DecoderProcessSpu( p_dec, p_block, b_flush );
>> + DecoderProcessSout( p_dec, p_block );
>> }
>> else
>> +#endif
>> {
>> - msg_Err( p_dec, "unknown ES format" );
>> - p_dec->b_error = true;
>> + if( p_dec->fmt_out.i_cat == AUDIO_ES )
>> + {
>> + DecoderProcessAudio( p_dec, p_block );
>> + }
>> + else if( p_dec->fmt_out.i_cat == VIDEO_ES )
>> + {
>> + DecoderProcessVideo( p_dec, p_block );
>> + }
>> + else if( p_dec->fmt_out.i_cat == SPU_ES )
>> + {
>> + DecoderProcessSpu( p_dec, p_block );
>> + }
>> + else
>> + {
>> + msg_Err( p_dec, "unknown ES format" );
>> + p_dec->b_error = true;
>> + }
>> }
>> }
>> }
I'll redo a patch with your comments and Rémi's in mind. There's room
for cleaning the code even more and factorize some as well. But one
step at a time...
>> --
>> 2.6.2
>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
More information about the vlc-devel
mailing list