[FFmpeg-devel] [PATCH] libavfilter-soc: implement pad filter

Thu Oct 1 01:00:16 CEST 2009

Sorry for the slow reply.

On date Thursday 2009-09-17 02:21:59 +0200, Michael Niedermayer encoded:
[...]
> > Now I'm posting other patches, what I'd like to discuss are these two
> > points:
> > 
> > * I tried both to put the information required for padding (x, y, h,
> >   w, exp_h, exp_w) in the link and pass it in the get_video_buffer()
> >   params, they're pretty equivalent but the second method (as in the
> >   patch) seems more flexible, as the information is not stored
> >   statically in the links, so it doesn't need a reconfiguration when
> >   changing it.  
> 
> yes
> 
> 
> >   (BTW maybe we can as well remove w/h from the link,
> >   that should make simpler to implement variable size filters, I still
> >   have to experiment with this).
> 
> maybe
> 
> > 
> > * I tinkered about the vflip problem much time, and I could'nt find some 
> >   way to keep the previous behavior with the new system.
> >   The new system assumes that the position of the area to pad
> >   (referred by the x/y params in the picref) needs to be invariant
> >   when passing a picref to the next filter.
> >   That's because the pad filter expects this:
> 
> i really honestly must admit that i couldnt figure out after a quick read of
> the mails what problem you have with vflip, maybe thats because there should
> be no problem ...
> 
> let me explain my view, maybe you could tell me where iam wrong or where you
> think there is a problem
> 
> in an abstract view the filters pass get_video_buffer calls in one direction
> and various calls to draw into the image in the other direction.
> 
> a vflip filter can surely flip the reference it gets from get_video_buffer()
> 
> to the right of vflip
> 
> 0--------------------------------
> T
> v-L-->0-----w------>
>       hXXXXXXXXXXXXX
>       vXXXXXXXXXXXXX
> 
> 
> ---------------------------------
> ----linesize--------------------->
> 
> between pad and vflip
> 
> ---------------------------------
> 
>       AXXXXXXXXXXXXX
>       hXXXXXXXXXXXXX
> A-L-->0-----w------>
> T
> |
> 0--------------------------------
> <----linesize--------------------
> 
> left of pad (padding at the top)
> 
> ---------------------------------
> 
>       hXXXXXXXXXXXXX
> A-L-->0-----w------>
> |
> T
> |
> 0--------------------------------
> <----linesize--------------------
> 
> and for drawing instead of get_video_buffer its the exact reverse
> 
> i see no problem

There are two variants of the problem.

1) with no inversion of the reference obtained by the pad filter with
get_video_buffer().

Consider the filterchain: "vflip, pad= x=0 : y=-1 : out_h=in_h + 100"

In this case the vflip filter, in the start frame receives a picref
from the in source, and passes a flipped reference with the origin
placed at the bottom left corner of the input buffer.

So this is the picref in input to the vflip

  [fig. 1]

 +----------+
 |  pad     |
 |   area   |
 |          |
 |          |
 O----------+    
 |    |     |
 |    |     |
 |    |     |
 |    |     |
 |    |     | 
 v    v     |
 +----------+

where O is the origin of the picref passed by start frame.

In output vflip passes this:

   [fig. 2]

 +----------+
 | pad      |
 |  area    |
 |          |
 |          |
 +----------+
 ^    ^     |
 |    |     |
 |    |     |
 |    |     |
 |    |     | 
 |    |     |
 O----------+

Now the pad filter expects the situation of fig. 1, and moves up the
position of the origin by 100 lines, and we get (the linesize is
negative, so the origin is moved down rather than up):

   [fig. 3]

 +----------+
 | pad      |
 |  area    |
 |          |
 |          |
 +----------+
 |    ^     |
 |    |     |
 |    |     |
 |    |     |
 |    |     | 
 |    |     |
 +----------+
 .
 . 
 .
 v
 O

The origin is moved outside the allocated buffer, which will
tipically result in a crash.

2) the buffer obtained by the pad filter with get_video_buffer() is
inverted and passed to the previous filter, as Vitor and IIUC Michael
proposed (check the patch: fix-vflip-logic+vitor.patch).

Consider the filterchain: "crop=0:0:WIDTH:100, vflip"

In this scenario the crop filter will take the top 100 lines of the
image in input.

This is the picref received in input by vflip.c:get_video_buffer():

  [fig. 4]

 O----------+
 | crop     |
 |  area    |
 |          |
 v          |
 +----------+
 |          |
 |          |
 |          |
 |          |
 |          | 
 |          |
 +----------+

The vflip filter moves the origin to the bottom left corner of the
input area, and inverts the linesizes, so this is what
vf_crop.c:get_video_buffer() obtains:

   [fig. 5]

 .
 .
 .
 +----------+
 ^ crop     |
 |  area    |
 |          |
 |          |
 O----------+
 |          |
 |          |
 |          |
 |          |
 |          | 
 |          |
 +----------+

The input filter will write to the top of the crop area, resulting
tipically in a crash.

...

Requesting that the linesize inversion should not be changed, bith
when passing buffers in get_video_buffer() and start_frame()), as
implemented in vflip-plain-ref.patch, avoids these problems.

Maybe there is some way to avoid this constraint, but my question is
if this is really worth to strive for such a solution which adds
complexity to the system, especially considering that the only case
where it seems to make sense to invert a picref is with the vflip
filter.

Patches in attachment are only for reference (that is not for review),
if we agree on this point then I'll discuss each one in a separate
thread.

Regards.
-- 
FFmpeg = Fast Foolish Multimedia Problematic Eretic Gadget
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lavfi-some-regtest.patch
Type: text/x-diff
Size: 3270 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091001/e25b9134/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lavfi-recursive-get-video-buffer.patch
Type: text/x-diff
Size: 16400 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091001/e25b9134/attachment-0001.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: debug-system.patch
Type: text/x-diff
Size: 3893 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091001/e25b9134/attachment-0002.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pad-implement.patch
Type: text/x-diff
Size: 17183 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091001/e25b9134/attachment-0003.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-vflip-logic+vitor.patch
Type: text/x-diff
Size: 1710 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091001/e25b9134/attachment-0004.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: vflip-plain-ref.patch
Type: text/x-diff
Size: 3098 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091001/e25b9134/attachment-0005.patch>