[vlc-devel] [PATCH] decoder: flush the vout before flushing the decoder

[Rémi Denis-Courmont]

On Monday 30 November 2015 11:41:25 Thomas Guillem wrote:
> > > +    /* see vout_Flush comment in input_DecoderDelete() */
> > 
> > Uh? How? If the decoder gets stuck decoding as in that comment, then
> > this code cannot be reached until the decoder gets unstuck. So that
> > seems like a catch-22. AFAIK, if the decoder locks up, then the only
> > avenue of recovery is to stop.
> 
> Oh, yes this patch works only if decoder module wait for a new picture
> from an other thread.

pf_flush() will be invoked and the decoder module can implement it however 
appropriate. And after that, vout_Flush() will be called in short order 
anyway. So I don´t really see the point, is there?

> As a better fix, I could put the following at the end of
> input_DecoderFlush
> 
> vlc_mutex_lock( &p_owner->lock );
> /* see vout_Flush comment in input_DecoderDelete() */
> if( p_owner->p_vout )
>     vout_Flush( p_owner->p_vout, VLC_TS_INVALID+1 );
> vlc_mutex_unlock( &p_owner->lock );
> 
> > If alternatively the decoder gets stuck flushing, then either the
> > decoder plug-in:
> > - has a bug,
> > - needs to cancel pending picture allocations *within* pf_flush, or
> 
> Or add decoder_abortPicture called from pf_flush to cancel thread
> waiting for a picture as I said in a previous mail.

This approach is not viable because it is racy and worker threads could get 
stuck again between step 1 and 2 retrying to allocate:
1) force all already pending picture allocations to fail,
2) flush.
(An opportunistic vout_Flush() has essentially the same supposedly fatal 
limitation.)

This other approach is also dangerous because it can cause a live loop in 
worker threads:
1) force all picture allocations to fail until further notice,
2) flush,
3) undo step 1.

decoder_abortPicture() could be useful for a specific class of threaded 
decoder, that would support this pattern where cancellation and flushing is 
atomic:
1) start flushing in non-blocking manner,
2) cancel pending picture allocation on worker threads,
3) finish flushing.

Yet even then, a threaded decoder implementation could flush while some or all 
worker threads are blocked in allocation step. This is really implementation-
dependent.

So overall I believe that decoder_abortPicture() would only be sufficient 
*and* necessary under constrained assumptions. Notably, I believe that it 
would not apply well to avcodec-MT, and I don´t know any other threaded 
decoder - maybe Android MediaCodec and/or OMX-IL?

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