[vlc-devel] [RFC PATCH 1/4] decoder: don't wait for flush to complete

[Rémi Denis-Courmont]

Le 2015-11-17 19:03, Rémi Denis-Courmont a écrit :
> Le 2015-11-17 18:55, Thomas Guillem a écrit :
>> ---
>>  src/input/decoder.c | 23 +----------------------
>>  1 file changed, 1 insertion(+), 22 deletions(-)
>>
>> diff --git a/src/input/decoder.c b/src/input/decoder.c
>> index cacf7f2..3e371ca 100644
>> --- a/src/input/decoder.c
>> +++ b/src/input/decoder.c
>> @@ -110,7 +110,6 @@ struct decoder_owner_sys_t
>>
>>      /* Flushing */
>>      bool flushing;
>> -    bool flushed;
>>      bool b_draining;
>>      atomic_bool drained;
>>      bool b_idle;
>> @@ -1341,9 +1340,6 @@ static void *DecoderThread( void *p_data )
>>              if( unlikely(dummy == NULL) )
>>                  msg_Err( p_dec, "cannot flush" );
>>
>> -            /* Owner is buggy if it queues data while flushing */
>> -            assert( vlc_fifo_IsEmpty( p_owner->p_fifo ) );
>> -            p_owner->flushing = false;
>
> Why do you move this line? ^^
>
>>              vlc_fifo_Unlock( p_owner->p_fifo );
>>
>>              /* Flush the decoder (and the output) */
>> @@ -1351,12 +1347,8 @@ static void *DecoderThread( void *p_data )
>>
>>              vlc_fifo_Lock( p_owner->p_fifo );
>>              vlc_restorecancel( canc );
>> +            p_owner->flushing = false;
>
> So what if another flush request occurred after the vlc_fifo_Unlock() 
> above.

Well actually, it might work: the owner will clear the FIFO (again) 
before anything was dequeued and there is no need to flush a second time 
in a row. But this is sufficiently nonobvious to deserve source code 
comments.

>
>>
>> -            /* Owner is supposed to wait for flush to complete.
>> -             * TODO: It might be possible to remove this 
>> restriction. */
>> -            assert( vlc_fifo_IsEmpty( p_owner->p_fifo ) );
>> -            p_owner->flushed = true;
>> -            vlc_cond_signal( &p_owner->wait_fifo );
>>              continue;
>>          }
>>
>> @@ -1486,7 +1478,6 @@ static decoder_t * CreateDecoder( vlc_object_t
>> *p_parent,
>>      p_owner->b_has_data = false;
>>
>>      p_owner->flushing = false;
>> -    p_owner->flushed = true;
>>      p_owner->b_draining = false;
>>      atomic_init( &p_owner->drained, false );
>>      p_owner->b_idle = false;
>> @@ -1818,7 +1809,6 @@ void input_DecoderDecode( decoder_t *p_dec,
>> block_t *p_block, bool b_do_pace )
>>              vlc_fifo_WaitCond( p_owner->p_fifo, &p_owner->wait_fifo 
>> );
>>      }
>>
>> -    p_owner->flushed = false;
>>      vlc_fifo_QueueUnlocked( p_owner->p_fifo, p_block );
>>      vlc_fifo_Unlock( p_owner->p_fifo );
>>  }
>> @@ -1874,13 +1864,6 @@ void input_DecoderFlush( decoder_t *p_dec )
>>
>>      vlc_fifo_Lock( p_owner->p_fifo );
>>
>> -    /* Don't flush if already flushed */
>> -    if( p_owner->flushed )
>> -    {
>> -        vlc_fifo_Unlock( p_owner->p_fifo );
>> -        return;
>> -    }
>> -
>>      /* Empty the fifo */
>>      block_ChainRelease( vlc_fifo_DequeueAllUnlocked( 
>> p_owner->p_fifo ) );
>>      p_owner->flushing = true;
>> @@ -1893,10 +1876,6 @@ void input_DecoderFlush( decoder_t *p_dec )
>>
>>      vlc_fifo_Signal( p_owner->p_fifo );
>>
>> -    /* Monitor for flush end */
>> -    while( !p_owner->flushed )
>> -        vlc_fifo_WaitCond( p_owner->p_fifo, &p_owner->wait_fifo );
>> -
>>      vlc_fifo_Unlock( p_owner->p_fifo );
>>  }

-- 
Rémi Denis-Courmont
http://www.remlab.net/