On date Monday 2009-05-25 15:47:59 +0200, [email protected] encoded:
>
> Hi,
>
> ----- "Stefano Sabatini" <[email protected]> wrote:
> > I believe pow *is* supported out of the box by the eval code.
>
> It is not enabled in the parse_primary(Parser *p) function.
>
> > Docs and example go to doc/vfilters.texi.
>
> Removed them.
Uh, what about to move them to doc/vfilters.texi?
> > enum VarName {
> > ...
> > VARS_NB
> > };
> >
> > is better.
>
> Done.
>
> >
> > And a PTS var may be useful too.
> >
>
> Working on that.
>
> > > +static const char *applyfn_symbols[POV_NULL+1]={
> > ^^
> > Why +1?
>
> I copied the code from vf_setpts, didn't gave it second thought.
>
> > > + ApplyfnContext *applyfn = ctx->priv;
> > > + char Y_expr[256], U_expr[256], V_expr[256], A_expr[256];
> > > + const char *error;
> >
> > Weird indent.
> >
>
> Fixed.
>
> > > + sscanf(args, "%255[^:]:%255[^:]:%255[^:]:%255[^:]", Y_expr, U_expr,
> > > V_expr, A_expr)
> >
> > I believe here it would be nice to use av_set_options_string, check
> > vf_pad.c in the archive for an usage example.
>
> Done.
>
> > > + applyfn->value_V = ff_parse(V_expr, applyfn_symbols,
> > > + NULL, NULL, NULL, NULL, &error);
> > > + if (!applyfn->value_V )
> > > + av_log(ctx, AV_LOG_ERROR,
> > > + "init() cannot parse V expression due to %s\n",
> > > error);
> > > +
> > > + applyfn->value_A = ff_parse(A_expr, applyfn_symbols,
> > > + NULL, NULL, NULL, NULL, &error);
> > > + if (!applyfn->value_A )
> > > + av_log(ctx, AV_LOG_ERROR,
> > > + "init() cannot parse A expression due to %s\n",
> > > error);
> > This can be factorized (check again vf_pad.c to see how), maybe even a
> > macro may be used.
>
> I created a macro for it, I'm not sure if it is okay according to
> FFmpeg coding standards.
>
> > > +static void draw_slice(AVFilterLink *link, int y, int h)
> > > +{
> > > + ApplyfnContext *applyfn = link->dst->priv;
> > > + AVFilterPicRef *in = link->cur_pic;
> > > + AVFilterPicRef *out = link->dst->outputs[0]->outpic;
> > > + uint8_t *inrow_0, *outrow_0, *alpharow, *outrow_1, *outrow_2,
> > > *inrow_1, *inrow_2;
> > > + int i, j, i_sub, j_sub, outrow_1_off, outrow_2_off, inrow_1_off,
> > > inrow_2_off;
> > > +
> > > + applyfn->values[POV_N] += 1.0;
> >
> > Ouch, this is wrong, as draw_slice() is called for every slice(), and
> > you don't know how many slices there are. This update should be done
> > in start_frame() / end_frame() (same for W and H, since they're not
> > supposed to change between a slice and another one).
> >
>
> Moved the code to start_frame.
>
> For some functions it would be much easier to normalize the YUV values
> to [0.0..1.0][-0.5..0.5][-0.5..0.5] beforehand, would it be an addition
> if this were to be an option?
Yes, I believe so. Maybe simply adding some vars for the normalized
values will be fine.
> Thanks for the feedback!
>
> Zeger Knops
> Index: vf_applyfn.c
[...]
> +static void draw_slice(AVFilterLink *link, int y, int h)
> +{
> + ApplyfnContext *applyfn = link->dst->priv;
> + AVFilterPicRef *in = link->cur_pic;
> + AVFilterPicRef *out = link->dst->outputs[0]->outpic;
> + uint8_t *inrow_0, *outrow_0, *alpharow, *outrow_1, *outrow_2, *inrow_1,
> *inrow_2;
> + int i, j, i_sub, j_sub, outrow_1_off, outrow_2_off, inrow_1_off,
> inrow_2_off;
> +
> + inrow_0 = in-> data[0] + y * in-> linesize[0];
> + outrow_0 = out->data[0] + y * out->linesize[0];
> +
> + outrow_1_off = (y >> applyfn->vsub) * out->linesize[1];
> + outrow_2_off = (h >> applyfn->vsub ) * out->linesize[2] * ( link->w >>
> applyfn->hsub ) * (y >> applyfn->vsub);
> + inrow_1_off = (y >> applyfn->vsub) * in-> linesize[1];
> + inrow_2_off = (h >> applyfn->vsub ) * in-> linesize[2] * ( link->w >>
> applyfn->hsub ) * (y >> applyfn->vsub);
> +
> + outrow_1 = outrow_1_off + out->data[1];
> + outrow_2 = outrow_2_off + out->data[2];
> + inrow_1 = outrow_1_off + in-> data[1];
> + inrow_2 = outrow_2_off + in-> data[2];
Some nits:
> + if ( out->data[3] )
I believe (out->data[3]) style is preferred.
> + alpharow = out->data[3] + y * out->linesize[3];
> +
> + for(i = 0; i < h; i ++) {
> + applyfn->var_values[y] = i;
> + i_sub = i >> applyfn->hsub;
> + for(j = 0; j < link->w; j ++)
> + {
> + j_sub = j >> applyfn->hsub;
Not K&R style (for brace not on the same line as the for), and weird
indent.
> + applyfn->var_values[x] = j;
> +
> + applyfn->var_values[Y] = inrow_0[j];
> + applyfn->var_values[U] = inrow_1[j_sub];
> + applyfn->var_values[V] = inrow_2[j_sub];
> +
> + applyfn->var_values[A] = out->data[3] ? alpharow[j] : 0;
> +
> + outrow_0[j] = ff_parse_eval(applyfn->Y_evalexpr,
> applyfn->var_values, applyfn);
> + outrow_1[j_sub] = ff_parse_eval(applyfn->U_evalexpr,
> applyfn->var_values, applyfn);
> + outrow_2[j_sub] = ff_parse_eval(applyfn->V_evalexpr,
> applyfn->var_values, applyfn);
> +
> + if ( out->data[3] )
^^^^^^^^^^^^^^^^
> + alpharow[j] = ff_parse_eval(applyfn->A_evalexpr,
> applyfn->var_values, applyfn);
> +
> + }
> + inrow_0 += in-> linesize[0];
> + outrow_0 += out->linesize[0];
> +
> + if ( out->data[3] )
^^^^^^^^^^^^^^^^
> + alpharow += out->linesize[3];
> +
> + inrow_1 = inrow_1_off + in-> data[1] + i_sub * in-> linesize[1];
> + inrow_2 = inrow_2_off + in-> data[2] + i_sub * in-> linesize[2];
> + outrow_1 = outrow_1_off + out->data[1] + i_sub * out->linesize[1];
> + outrow_2 = outrow_2_off + out->data[2] + i_sub * out->linesize[2];
> + }
> +
> + avfilter_draw_slice(link->dst->outputs[0], y, h);
> +}
> +
> +AVFilter avfilter_vf_applyfn =
> +{
> + .name = "applyfn",
> + .init = init,
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.
If you don't mind consider to change accordingly the name of the
filter (but await for others devs opinion, Vitor?).
Regards.
_______________________________________________
FFmpeg-soc mailing list
[email protected]
https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-soc
