[FFmpeg-devel] [PATCH] lavfi/gradfun: fix direct writing in buffer.

[Clément Bœsch]

On Sun, Dec 02, 2012 at 03:57:52PM +0100, Nicolas George wrote:
> Le duodi 12 frimaire, an CCXXI, Clément Bœsch a écrit :
> > The (untested) code will now assume src = dst all the time, and we can
> > maybe do some more simplification. The min_perms is here to make sure that
> > a new buffer is allocated when the input is read-only (that was done
> > manually previously).
> > 
> > currently:
> >   if inbuf is RO:
> >     - filter requests a new write buffer
> >     - processing with src ≠ dst
> >   else if inbuf is RW:
> >     - processing with src = dst
> > 
> > after:
> >   if inbuf is RO:
> >     - lavfi requests a new write buffer so a new RW inbuf is passed
> >   - processing with src = dst
> > 
> > Does that make any sense?
> 
> It makes absolute sense, but it is missing the key information: benchmark.
> 
> Consider the hue filter (I did not check whether it is implemented as I will
> state, it is just an example): for planar formats, the U and V planes are
> completely rewritten while the Y plane is passed unchanged.
> 
> If you require WRITE permissions and it is missing, then lavfi will copy Y
> for us in the new buffer, as it should, but it will also copy the whole U
> and V planes, which is completely unnecessary.
> 
> The other way around has the opposite disadvantage: if we always allocate a
> new buffer, then the Y plane needs to be copied always, even if we could
> have reused it.
> 
> The current scheme uses the best of the two worlds: if we have write
> permissions, we use them to avoid copying Y; else, using a new blank buffer
> avoids copying U and V for nothing.
> 
> I have absolutely no idea whether this optimization makes any real
> difference or not. If it was for a new filter, I would not consider
> mandatory to implement it. But in this particular case it is already there,
> removing it without checking would be rather strange.
> 

Ah good point, I missed all of this, thanks for the explanations!

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121207/6599c835/attachment.asc>