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

[Rémi Denis-Courmont]

Le keskiviikkona 7. lokakuuta 2020, 10.24.55 EEST Steve Lhomme a écrit :
> On 2020-10-07 7:41, 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.
> 
> BTW, using picture_Copy is almost never a good idea.

It's mostly unavoidable for in-place filters, not only blend (though you could 
consider blend as an in-place filter, at least conceptually).

Unless the picture is already dereferenced explicitly by the decoder, we 
either know that it cannot be modified, or do not know if it can be modified.

> To avoid unnecessary copies we could introduce the concept of
> mutable/immutable pictures.

That won't really solve anything at this point. We don't have a case of read-
only picture buffer so far. What can be done is a reference counter on the 
picture buffer for copy-on-write. But I fear in practice it would almost always 
do a copy because the picture buffer is most often still referenced by the 
decoder or something else.

-- 
レミ・デニ-クールモン
http://www.remlab.net/