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

[Thomas Guillem]

On Mon, Nov 30, 2015, at 19:36, Rémi Denis-Courmont wrote:
> On Monday 30 November 2015 18:53:56 Thomas Guillem wrote:
> > > > > 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 ?
> 
> Why would it not loop? What do you expect from a decoder worker thread
> upon 
> picture allocation failure? Immediate retry... Dropping of all pending
> data, 
> marking of a fatal error and refusal to decode anything ever again... Or 
> anything between those two extremes?
> 
> If the worker thread just goes to sleep until *new* data is queued for 
> decoding, then this works great. But there are no reasons why it would be
> that 
> way: That approach only makes sense when flushing. Yet at that point the 
> picture allocation failed, but flushing has not even started.
> 
> Far more likely, the decoder either retries allocation - leading to a
> real 
> live loop - or discards some unspecified amount of coded data and tries
> to 
> allocate again - leading to a transient live loop.
> 
> > > 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 ?
> 
> Does avcodec_flush_buffers() not wait for pending get_buffer() to return?
> - If it does, then I don´t see how this can apply to avcodec-MT.
> - If does not, then I don´t understand the purpose of the patch in the
> OP.

Yes, avcodec_flush_buffers() waits for pending get_buffer() to return.

I'm out of ideas right now, can we discuss it this week-end?


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