[vlc-devel] [PATCH 3/8] picture_pool: use the internal condition/lock indirectly

[Steve Lhomme]

On 2020-07-30 17:35, Rémi Denis-Courmont wrote:
> Le keskiviikkona 29. heinäkuuta 2020, 8.48.48 EEST Steve Lhomme a écrit :
>> Again, I don't see a solution around this. If the "object" to cancel is
>> not directly accessible by the DecoderThread, in other threads, we can't
>> safely cancel the wait.
> 
> I am not convinced at this point that the problem is intrinsically tied to the
> decoder interface/core. But even if it were, and even if there was no simpler
> alternative (like flushing the vout), the cancellable proposal is essentially a
> constrained reinvention of the interrupt proposal. And I don't think we need
> two mechanisms to do the same thing.
> 
>>> Well the obvious way around is not tot use picture_pool_Wait() or even
>>> picture_pool_t if it does not suit the given use case.
>>
>> va_surface doesn't use picture_pool and yet it waits. mediacodec and
>> VDPAU also have wait loops not using picture pools. All of these are not
>> cancelable.
> 
> Sure, but is it really reasonable to impose a specific mutex and condition
> variable on them and every other future decoders? Interrupts don't have that
> limitation...
> 
> And is stopping the only problematic scenario? Cancellable (and interrupt too)
> cannot handle non-terminal/non-fatal unblocking :(
> 
>> What is the decoder supposed to do on pause ?
> 
> Ideally, I think the decoder is supposed to be notified of the pause by the
> decoder owner (ES output), and *then* the pause notification onto the decoder
> output (video output thread).
> 
> It's entirely up to the decoder not to lock itself up. If it needs to wake its
> internal threads on pause, that's an implementation detail of the
> (hypothetical) decoder pause change callback.
> 
> 
> Of course, it's much easier to do the ugly work-around and flush the video
> output in the decoder's back.

IMO that's the way to go for now and possibly in the future as well. It 
seems normal to flush the pending pictures on Stop/Close. I haven't 
investigated but maybe the flush is done later or closing the vout makes 
this automatic.

In the long run we should have a proper push with cancel model. In lavc 
that may mean having one type of cancelable object that doesn't depend 
on get_format/get_frame changes. In that case a callback called by the 
decoder owner would work.