[vlc-devel] [PATCH] fps: clone the extra pictures we send

[Alexandre Janniaux]

Hi,

On Wed, Oct 07, 2020 at 07:41:26AM +0200, Steve Lhomme wrote:
> On 2020-10-06 16:40, Rémi Denis-Courmont wrote:
> > Le tiistaina 6. lokakuuta 2020, 15.08.35 EEST Steve Lhomme a écrit :
> > > On 2020-10-06 13:53, Steve Lhomme wrote:
> > > > The same picture is supposed to be filtered or rendered at a different
> > > > date, but the source pixels should be the same. The content is not going
> > > > to be changed in the pictures we output from this filter. Even blenders
> > > > take care of this in their source picture to avoid modifying pictures
> > > > used by the decoder as reference frames.
> > >
> > > It means we can use the fps even for GPU sources.
> >
> > We can already do that. picture_Copy() calls picture_CopyPixels() which calls
> > the context copy callback. There's effectively no differences between
> > picture_Clone() and picture_Copy() for opaque pictures.
>
> And this has already been pointed out in the past that this is wrong. A
> clone and a copy are not the same for CPU pictures. It must not be the same
> for GPU pictures.
>
> And doing a copy here is also the wrong thing to do.

There's no right or wrong here. From an historical perspective
the .copy callback doing a refcount is the wrong thing to do
but most GPU resources like those we use are read only anyway,
while we don't queue them back to the decoder until all the
read references are consumed, and copying makes almost no
difference for the picture themselves and is harder to write
for GPU buffers.

There's a big difference for picture producer though. Since we
have no feedback of how many pictures can be used by each part
of the pipeline, not really copying and holding on pictures
means that we can stall the pipeline, so it's «not supported»
in the current state.

Thus, both points are legitimate and both needs to be tackled,
hence Rémi's point on the needed refactor for this.

Regards,
--
Alexandre Janniaux
Videolabs