[FFmpeg-devel] [PATCH] frei0r wrapper

Michael Niedermayer michaelni
Sun Sep 5 15:43:23 CEST 2010


On Mon, Aug 30, 2010 at 08:32:01PM +0200, Stefano Sabatini wrote:
[...]

> +
> +where @var{filter_path} is the path to the frei0r effect to load, and
> + at var{param1}, @var{param2}, ... , @var{paramN} is a list of values
> +separated by ":" which specify the parameters for the frei0r effect.
> +
> +frei0r filters are usually installed in @file{/usr/lib/frei0r-1/}.
> +
> +A frei0r effect parameter can be a boolean (whose values are specified
> +with "true" and "false"), a double, a color (specified by the syntax
> + at var{R} @var{G} @var{B}), a position (specified by the syntax
> + at var{x} @var{y}) and a string.
> +
> +The number and kind of parameters depend on the loaded effect. If an
> +effect parameter is not specified the default value is set.
> +
> +Follows some example:
> + at example
> +# apply the distort0r effect, set the first two double parameters
> +frei0r=/usr/lib/frei0r-1/distort0r.so:0.5:0.01

requirering full pathes is lame when all filters are installed in like
one place


[...]
> +static int set_param(AVFilterContext *ctx, f0r_param_info_t info, int index, char *param)
> +{
> +    Frei0rContext *frei0r = ctx->priv;
> +    void *v;

> +    long double ld;

what are you trying to do?


> +    double d;
> +    f0r_param_color_t col;
> +    f0r_param_position_t pos;
> +    f0r_set_param_value_f f0r_set_param_value;
> +
> +    if (!(f0r_set_param_value = load_sym(ctx, "f0r_set_param_value")))
> +        return AVERROR(EINVAL);
> +

> +    switch (info.type) {
> +    case F0R_PARAM_BOOL:

> +    {

is that way of placing it consistent?

    
> +        if      (!strcmp(param, "true" )) d = 1.0;
> +        else if (!strcmp(param, "false")) d = 0.0;
> +        else goto fail;

we should allow something shorter than true false


> +        v = (void *)(&d);

the cast looks unneeded


> +    }
> +    break;
> +
> +    case F0R_PARAM_DOUBLE:
> +    {
> +        char *tail;
> +        ld = strtold(param, &tail);
> +        if (*tail || ld > DBL_MAX || ld < -DBL_MAX)
> +            goto fail;
> +        d = ld;
> +        v = (void *)(&d);
> +    }
> +    break;
> +

> +    case F0R_PARAM_COLOR:
> +    {
> +        if (sscanf(param, "%f %f %f", &col.r, &col.g, &col.b) != 3)
> +            goto fail;
> +        v = (void *)(&col);

dont the spaces need escaping on the command line?

[...]
> +        switch (info.type) {
> +        case F0R_PARAM_BOOL:
> +            v = (void *)(&d);
> +            f0r_get_param_value(frei0r->instance, v, i);
> +            av_log(ctx, AV_LOG_INFO, "value:'%s'\n", d == 1.0 ? "true" : "false");
> +            break;
> +        case F0R_PARAM_DOUBLE:
> +            v = (void *)(&d);
> +            f0r_get_param_value(frei0r->instance, v, i);
> +            av_log(ctx, AV_LOG_INFO, "value:'%f'\n", d);
> +            break;
> +        case F0R_PARAM_COLOR:
> +            v = (void *)(&col);
> +            f0r_get_param_value(frei0r->instance, v, i);
> +            av_log(ctx, AV_LOG_INFO, "value:'%f %f %f'\n", col.r, col.g, col.b);
> +            break;
> +        case F0R_PARAM_POSITION:
> +            v = (void *)(&pos);
> +            f0r_get_param_value(frei0r->instance, v, i);
> +            av_log(ctx, AV_LOG_INFO, "value:'%lf %lf'\n", pos.x, pos.y);
> +            break;
> +        default: /* F0R_PARAM_STRING */
> +            v = (void *)s;
> +            f0r_get_param_value(frei0r->instance, v, i);
> +            av_log(ctx, AV_LOG_INFO, "value:'%s'\n", s);
> +            break;
> +        }

what is this good for?


[...]
> +static av_cold void uninit(AVFilterContext *ctx)
> +{
> +    Frei0rContext *frei0r = ctx->priv;
> +    f0r_destruct_f f0r_destruct;
> +    f0r_deinit_f f0r_deinit;
> +
> +    if ((f0r_destruct = dlsym(frei0r->dll, "f0r_destruct")))
> +        f0r_destruct(frei0r->instance);

all the dlsym shoudl be done in "init()" IMHO

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100905/b5e7009c/attachment.pgp>



More information about the ffmpeg-devel mailing list