[FFmpeg-devel] [PATCH] libavfilter-soc: implement pad filter
Stefano Sabatini
stefano.sabatini-lala
Sun Aug 9 01:25:15 CEST 2009
On date Friday 2009-08-07 01:35:00 +0200, Stefano Sabatini encoded:
> On date Thursday 2009-07-30 06:26:46 +0200, Vitor Sessak encoded:
> > Stefano Sabatini wrote:
> >> On date Monday 2009-07-27 06:15:47 +0200, Vitor Sessak encoded:
> >>> Stefano Sabatini wrote:
> >> [...]
> >>>> 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 believe that should work (not 100% sure though, I shuld double-check
> >> it), note that the vflip filter as it is currently doesn't work for
> >> another reason.
> >
> > There should be some samples that output after decoding negative
> > linesize (I have no example in mind now, unfortunately).
> >
> >>>> * 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?
> >>
> >> Of course padding/cropping chaining is supported, that was a
> >> prerequisite for it to be acceptable, so let me clarify the problem.
> >>
> >> The pad filter takes in input a video buffer, and knows that it is
> >> offeset with respect to the area when it has to write.
> >> So we have:
> >>
> >> [exp_w, exp_h]
> >> +--------------------------------------+
> >> | | | (padx, pady)
> >> |
> >> | +------------------------+ |
> >> | | | |
> >> | | [w, h] | |
> >> | | | |
> >> | +------------------------+ |
> >> +--------------------------------------+
> >
> > I suppose that in this drawing pad{x,y} is stored in
> > AVFilterPicRef.{x,y}, no?
> >
> >> It assumes that the data in the passed reference of the video buffer
> >> points to the top-left corner of the internal area of the padded area,
> >> and so it "moves back" in the new video buffer reference the pointers
> >> to the buffer, so they point to the top-left corner of the padded
> >> area.
> >
> > So it changes AVFilterPicRef.{x,y}.
>
> Yes, actually only the pad and crop filter are meant to change those
> values.
>
> For big-buffer I mean the allocated area, which accounts also for the
> padding area.
>
> The meaning for x, y in the picref are:
>
> * the x,y offsets, with respect to the big-frame buffer area, of the
> data[plane] pointers.
>
> * data[plane] pointers are supposed to contain the pointers to the
> area of the big-buffer when the filter has to write.
>
> The pad and crop filters are special as they change the area in the
> big-buffer when the next filter has to write.
>
> >> The vflip filter on the other hand, puts in the passed buffer
> >> reference the pointers to the bottom-left corner of the input buffer,
> >> and invert the linesizes, which is not what the pad filter assumes.
> >> Negative linesizes shouldn't be a problem, the problem would be with
> >> the pady value which should be changed (also for all the following pad
> >> filters).
> >
> > So, but I'm missing something. It is this last assertion that troubles
> > me. Which field of which struct exactly a filter (or just vflip?) cannot
> > change?
>
> A filter such a vflip filter is not supposed to change the area in the
> big-buffer where the next filter is going to write, so it shouldn't be
> allowed to change data[plane] and the x,y offsets (neither the
> rendering vertical direction of the whole big-buffer, as I'll explain
> below).
>
> > What will fail with the following:
> >
> > pic->data[0] += exp_w*frame->linesize[0] + exp_h;
> > pic->linesize[0] *= -1;
> > [... Same for chroma ...]
> > pic->x = pic->exp_w - pic->x - pic->w;
> > pic->y = pic->exp_h - pic->y - pic->h;
> >
> > ?
> >
> > I suppose I have the right to change pic->x, because the pad filter does
> > that too.
>
> Now there is another problem with the linesize inversion. As the
> current vflip filter works it assumes that the buffer to vflip is the
> whole buffer to display.
>
> Let's consider this filterchain:
> vflip , pad
>
> It would be possible to make the vflip filter to change the y offset,
> but then we would obtain something like this:
>
> +----------------------------------+
> | 8 |
> | 7 |
> | 6 |
> | +----vflip-area------+ 5 |
> | | | | | 4 |
> | | v | v 3 |
> | +--------------------+ 2 |
> | (x,y) 1 |
> +----------------------------------+
>
> while we would rather expect:
>
> +----------------------------------+
> | (x,y) 1 |
> | +----vflip-area------+ 2 |
> | | | | ^ 3 |
> | | v | | 4 |
> | +--------------------+ 5 |
> | 6 |
> | 7 |
> | 8 |
> +----------------------------------+
>
> So even if we change the y offset with respect to the new coordinate
> system, the linesize inversion will cause the whole big-buffer to be
> rendered reverse-wise.
>
> My proposal is to simply make illegal the linesize inversion, as it is
> now it is only useful for the vflip filter, and makes very hard to
> implement a sane behavior with big-buffer areas, and frankly I can't
> imagine any other filter when the linesize inversion may be useful.
>
> The price to pay in this case is that the vflip filter would require a
> memcpy (check the already posted patch which implements that).
>
> Regards.
Ping?
Note also that in this case there is no need to keep x,y and
exp_h,exp_w in the picref, new patchset attached, test it for example
with:
-vfilters "pad= out_w = 3/2 * in_w : out_h= 3 / 2 * in_h : x=10:y=10: color=pink, vflip,
pad= x=10:y=10:out_w = 3/2 * in_w : out_h= 3 / 2 * in_h : color=blue"
Regards.
--
FFmpeg = Fast and Frightening Marvellous Pitiless Enigmatic Gadget
-------------- next part --------------
A non-text attachment was scrubbed...
Name: vflip-plain-ref.patch
Type: text/x-diff
Size: 2815 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090809/fd57593a/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lavfi-recursive-get-video-buffer.patch
Type: text/x-diff
Size: 16322 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090809/fd57593a/attachment-0001.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pad-implement.patch
Type: text/x-diff
Size: 17166 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090809/fd57593a/attachment-0002.patch>
More information about the ffmpeg-devel
mailing list