[vlc-devel] [PATCH 3/8] picture_pool: use the internal condition/lock indirectly
remi at remlab.net
Tue Jul 28 17:02:26 CEST 2020
Le tiistaina 28. heinäkuuta 2020, 8.33.49 EEST Steve Lhomme a écrit :
> >> In pull model it was easy. The pool belonged to the vout so it was not
> >> tied to such lock. It was possible to cancel the pool regardless of who
> >> is waiting. Now that pools are in many decoders (and in many forms) it
> >> becomes tricky to do this. But we need a solution anyway.
> > You can't expect to be able to pass a "cancellable" to each and every
> > place
> > that might sleep. Experience has shown that this just doesn't work.
> Experience is a personal thing. I have never experienced anyone trying
> this before.
> My intuition tells me it's ugly, I agree. The "cancellable" objet can
> only be used by one pool (maybe it would need some attach/detach to
> ensure this). It could be recycled between pools of the same decoder.
I am referring to the project's interrupt framework, what led to that and what
was there before. Specifically, you can't impose a specific mean of waiting, nor
a specific instance of it. And once you fix that, you end up with exactly the
same thing as the interrupt framework, including with all its intrinsic
problems. In particular, you can only really interrupt for final stop/destroy,
even though there are typically other wait scenarii that would need to be
Besides, the picture pool used to be a mandatory interface between the core
and the video output. But nowadays, it's just a convenience for allocating and
recycling main RAM picture buffers. So if it does not suit a given use case,
then there are no reasons to use it. And indeed, opaque decoder surfaces have
always been managed by ad-hoc means, since the picture pool cannot allocate
them. It makes even less sense to use the picture pool for that now than
> > If a module has a private object upon which it sleeps, it is responsible
> > for not locking itself up. For instance, access and access_demux modules
> > are responsible for stopping any asynchronous or threaded activities as
> > needed. A contrario, stream filter and demuxer modules rely on the
> > interrupt framework - because the sleeping happens within the underlying
> > stream that is given to them.
> The picture pool isn't locked during picture_pool_Wait(). So it works as
> The problematic lock is `p_sys->lock` in avcodec/video.c.
In other words, it's a bug in the avcodec plugin. The decoder core and the
pool helpers have nothing to do with it.
> When the picture_pool_Wait() is called, that lock is held. I don't think
> there's a way around this.
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.
And a decoder cannot block the ES output or the video output on pause anyway.
It's not only about pause then stop; it's pause then anything (other than
> That would mean, contrary to what we said, decoders cannot handle their
> own picture pool.
I don't think we meant that decoders must use picture_pool_t. picture_pool_t
is potentially convenient for software decoders. It never was convenient and
probably never will be for hardware decoders whereby it's just a glorified
free-list. You could just as well use a vlc_queue_t or a list and a lock to
implement the picture pool as a *concept*.
If there was a problem with locking in the decoder core, then we'd have a
problem. But this looks like a problem within avcodec here.
More information about the vlc-devel