[FFmpeg-soc] [soc]: r1773 - in libavfilter: Makefile allfilters.h avfilter.c vf_rot90.c

[Vitor Sessak]

Hi,

Bobby Bingham wrote:
> On Sun,  6 Jan 2008 14:19:12 +0100 (CET)
> vitor <subversion at mplayerhq.hu> wrote:
>> +static void draw_slice(AVFilterLink *link, int y, int h)
>> +{
>> +    RotContext *rot = link->dst->priv;
>> +    AVFilterPicRef *in  = link->cur_pic;
>> +    AVFilterPicRef *out = link->dst->outputs[0]->outpic;
>> +    int i, j, plane;
>> +
>> +    /* luma plane */
>> +    for(i = y; i < h; i ++)
>> +        for(j = 0; j < link->w; j ++)
>> +            *(out->data[0] +   j *out->linesize[0] + i) =
>> +                *(in->data[0]+ i * in->linesize[0] + j);
>> +
>> +    /* chroma planes */
>> +    for(plane = 1; plane < 3; plane ++) {
>> +        for(i = y >> rot->vsub; i < h >> rot->vsub; i++) {
>> +            for(j = 0; j < link->w >> rot->hsub; j++)
>> +                *(out->data[plane] +   j *out->linesize[plane] + i) =
>> +                    *(in->data[plane]+ i * in->linesize[plane] + j);
>> +        }
>> +    }
>> +
>> +    avfilter_draw_slice(link->dst->outputs[0], y, h);
>> +}
> 
> This is a bad use of slices.

I admit that I didn't understand the slices concept properly...

> Slices must be a rectangle whose width spans the whole image, and whose
> height is arbitrary.  By transposing a slice, you end up with a valid
> area which spans the height of the image, but not necessarily width.
> 
> Even worse, the last line tells the next filter the dimensions of the
> input slice, which are not the same as the dimensions of your output,
> even assuming your output is valid as a slice.  Run with "-vfilters
> slicify,transpose" (after updating to the latest update I made - the
> auto-inserted scale filters would mask the problem otherwise), or even
> "-vfilters transpose,transpose" to see the result.
> 
> There are a few possible solutions:
> 
> a) Change the concept of slices to allow arbitrary rectangular
> regions.  I had briefly considered this early on, but dismissed it.
> Maybe it's worth considering?

I don't think it's worth the work. Where else would this be useful? 
Also, it's not as cache friendly as the vertical slices...

> b) Add a flags field to input pads, with a flag for "requires entire
> image at once" and auto-insert a buffer filter before filters with this
> flag.  I've been thinking about doing this anyways, because the next
> option (c) is the only one which works currently, and I don't like the
> inconsistency it causes in how you have to implement some filters.

I like this idea. It's really confusing to see some filters processing 
the image in the request_frame() method and others in the draw_slice().

> c) Make the filter process the frame in either request_frame() or in
> end_frame().  This will guarantee that you have the entire frame before
> you start processing it, and you therefore have enough input to
> guarantee a valid slice for output.  See vf_scale or vf_overlay for
> examples.
> 
> Also, could you please set the aspect ratio for the output frames
> correctly.

I will do it as soon as I have some time.

> 
> I haven't looked at vf_rotate yet, but I guess this probably applies
> there as well.

It is, I will fix it too.

BTW, if (a) is implemented, what would be the right fix (other than 
setting the flag) for vf_transpose.c? Just ignore h and y in 
draw_slice() and use the link->h instead?

-Vitor