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

[Vitor Sessak]

Stefano Sabatini wrote:
> On date Thursday 2009-07-16 01:59:37 +0200, Michael Niedermayer encoded:
>> On Thu, Jul 16, 2009 at 12:49:21AM +0200, Stefano Sabatini wrote:
>>> On date Wednesday 2009-07-15 01:03:12 +0200, Stefano Sabatini encoded:
>>> [...]
>>>> I changed the logic of the patches, now the buffer informations are
>>>> stored in the link rather than in the params.
>>>>
>>>> This allows for simpler API usage, also makes it possible crop/pad
>>>> chains which caused troubles before. I'll provide a better explanation
>>>> with a further patchset, since these ones need to be cleaned up
>>>> /fixed.
>>>>
>>>> Since the last version:
>>>> * fixed ffmpeg.c
>>>> * fixed hflip
>>>>
>>>> Yet to come:
>>>> * fix vflip
>>>> * fix for leaks?
>>>> * add a regression test
>>>> * direct rendering is not implemented, it would be better to post it
>>>>   as a separate patchset since this is hard enough
>>> I added a pix_fmt field to the get_video_buffer, as this information
>>> is not known in the links (i.e. when configuring the chain).
> 
> That wasn't true, my mind was obfuscated when I conceived that thing.
> 
>>> Also added a regression test for the pad.
>>>
>>>> I also suspect the exp_w, exp_h, x, y params in the picref are not really
>>>> required.
>>> Patches updated.
>> would it be possible to split that stuff up a little?
>> (self contained changes with explanations of why its done as is done and
>>  so on)
>>
>> that is assuming you want this reviewed ...
> 
> Uhm, no the patches were not still ready for review but I wanted
> backup / share my progress in the meaningwhile.
> 
> 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.  (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).

Just a question: does it works when linesize is negative?

> * 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:
> 
> static void start_frame(AVFilterLink *link, AVFilterPicRef *picref)
> {
>     PadContext *pad = link->dst->priv;
>     AVFilterPicRef *ref2 = avfilter_ref_pic(picref, ~0);
>     link->dst->outputs[0]->outpic = ref2;
> 
>     ref2->data[0] -=  pad->x             + (pad->y * ref2->linesize[0]);
>     ref2->data[1] -= (pad->x>>pad->hsub) + (pad->y * ref2->linesize[1]>>pad->vsub);
>     ref2->data[2] -= (pad->x>>pad->hsub) + (pad->y * ref2->linesize[2]>>pad->vsub);
>     if (ref2->data[3])
>         ref2->data[3] -= pad->x + pad->y * ref2->linesize[3];
>     ref2->x -= pad->x;
>     ref2->y -= pad->y;
>     ref2->w = pad->out_w;
>     ref2->h = pad->out_h;
> 
>     avfilter_start_frame(link->dst->outputs[0], ref2);
> }
> 
> to work.

Unless I'm misunderstanding something, this forbids, for ex., chainning 
two pad filters. Am I right?

-Vitor