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

[Nicolas George]

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.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121202/5bec7869/attachment.asc>