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

[Thomas Guillem]

I proposed a new patch with more comments, And I explain why I move the
p_owner->flushing = false; line.

On Tue, Nov 17, 2015, at 17:07, Rémi Denis-Courmont wrote:
> 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/
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel