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

[Rémi Denis-Courmont]

Le perjantaina 31. heinäkuuta 2020, 7.21.38 EEST Steve Lhomme a écrit :
> 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.

Now, probably. In the future? It's still a kludge.

Whether we should explicit or implicitly flush before destroy is just an 
arbitrary conventional choice. The best choice is probably whatever is easier 
for the decoder to handle.

But flushing order *should* be from upstream to downstream. This flushes 
downstream (video output) to work around an upstream (decoder) problem. That's 
what makes it a kludge.

-- 
Реми Дёни-Курмон
http://www.remlab.net/