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

[Thomas Guillem]

On Mon, Nov 30, 2015, at 17:41, Rémi Denis-Courmont wrote:
> 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.

That's the idea I had in mind. (do we agree, that this approach use
decoder_abortPicture(true/false) ?)
A live loop ? But it will be ended when 2/flush is completed, no ?

> 
> 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?

Why would it not apply to avcodec-MT ?
get_buffer2 callbacks can fail. This can be problematic when playing,
but if you flush right after, you don't really care, no ?

For MediaCodec/OMX, there was an hack that forced 31 pictures allocation
when using direct rendering, they generally need less pictures (between
2 and 26).
I tried to remove this hacks, but you can't get the number of output
buffer using android ndk (and it's deprecated in Java since Android L)

> 
> -- 
> 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