[FFmpeg-soc] [RFC] Apply function to pixel filter: vf_applyfn

[Vitor Sessak]

Hi!

Thanks for the patch and sorry for the delay for reviewing.

Zeger Knops wrote:
> On Wed, 2009-05-27 at 18:44 +0200, Diego Biurrun wrote:
>> On Wed, May 27, 2009 at 05:49:37PM +0200, Zeger Knops wrote:
>>>> Not K&R style (for brace not on the same line as the for), and weird
>>>> indent.
>>> Updated to K&R style.
>> Clearly not.
>>
>> Diego
> 
> Working on that. There was also a problem with the previous patch, I
> have attached a new patch which replaces the previous.

[...]

>> > Rethinking at it, I wrote a very similar filter named vf_eval.c but
>> > which worked in the RGB colorspace.
>> > 
>> > Maybe it would be less confusing for the users to give to this filters
>> > some related names, for example:
>> > rgbeval and yuveval,
>> > or
>> > yuvapplyfn and rgbapplyfn.

I'm not against it, but obviously both filters should share 99% of the 
code (no duplication)....

[...]

> +        for(j = 0; j < link->w; j ++) {
> +            j_sub = j >> applyfn->hsub;
> +            applyfn->var_values[x] = j;
> +
> +            x_eval = (uint16_t) ff_parse_eval(applyfn->x_evalexpr, applyfn->var_values, applyfn) % link->w;
> +            y_eval = (uint16_t) ff_parse_eval(applyfn->y_evalexpr, applyfn->var_values, applyfn) % h;

I prefer to give a error in case it is beyond the picture (and let the 
user do the modulus op if he want to)

> The filter has been updated to support remapping of x and y, so
> flipping the video can be done with applyfn="x_exp=W-x" or
> apply="x_exp=H-y". A cheesy water effect is achieved with
> applyfn="x_exp=x+10*sin((y/H)*PI*10+(N/4))". 

> +            x_eval = (uint16_t) ff_parse_eval(applyfn->x_evalexpr, applyfn->var_values, applyfn) % link->w;
> +            y_eval = (uint16_t) ff_parse_eval(applyfn->y_evalexpr, applyfn->var_values, applyfn) % h;
> +
> +            applyfn->var_values[Y] = *(inrow_0_off +  y_eval                   * in-> linesize[0] +  x_eval);
> +            applyfn->var_values[U] = *(inrow_1_off + (y_eval >> applyfn->hsub) * in-> linesize[1] + (x_eval >> applyfn->hsub));
> +            applyfn->var_values[V] = *(inrow_2_off + (y_eval >> applyfn->hsub) * in-> linesize[2] + (x_eval >> applyfn->hsub));

This will play very badly with slices (you can not read beyond the slice 
you are rendering. Try your example above using "slicify=8,applyfn=..." 
as filter chain. In the other hand, this is a interesting feature, so I 
think it justify making the filter not slice-based.

> +    if (out->data[3])
> +    {

> +    if(out) {


Inconsistent indentation (if you are wondering, in ffmpeg the later is 
preferred).

> +static void end_frame(AVFilterLink *link)
> +{
> +    avfilter_end_frame(link->dst->outputs[0]);
> +}

I'm not sure this is needed.

-Vitor