[FFmpeg-devel] [PATCH] lavfi/WIP: vignette filter.

[Michael Niedermayer]

On Fri, May 10, 2013 at 06:49:22PM +0200, Clément Bœsch wrote:
> On Thu, Apr 04, 2013 at 05:15:29PM +0200, Clément Bœsch wrote:
> [...]
> 
> New version attached.
[...]

> +typedef struct {
> +    const AVClass *class;
> +    const AVPixFmtDescriptor *desc;
> +    int backward;
> +    enum EvalMode { EVAL_MODE_INIT, EVAL_MODE_FRAME, EVAL_MODE_NB } eval_mode;
> +#define DEF_EXPR_FIELDS(name) AVExpr *name##_pexpr; char *name##_expr; double name;
> +    DEF_EXPR_FIELDS(a);
> +    DEF_EXPR_FIELDS(x0);
> +    DEF_EXPR_FIELDS(y0);
> +    double var_values[VAR_NB];

> +    double *fmap;

isnt double overkill ?


[...]
> +static double get_natural_factor(const VignetteContext *s, int x, int y)
> +{
> +    const int xx = (x - s->x0) * s->xscale;
> +    const int yy = (y - s->y0) * s->yscale;
> +    const double dnorm = hypot(xx, yy) / s->dmax;
> +    if (dnorm > 1) {
> +        return 0;
> +    } else {
> +        const double c = cos(s->a * dnorm);
> +        return (c*c)*(c*c); // XXX: do not remove braces, it helps compilers
> +    }
> +}

this looks a bit unflexible, how well does this work for removing
actual vignetting from pictures taken with actual lenses ?


> +
> +#define TS2D(ts)     ((ts) == AV_NOPTS_VALUE ? NAN : (double)(ts))
> +#define TS2T(ts, tb) ((ts) == AV_NOPTS_VALUE ? NAN : (double)(ts) * av_q2d(tb))
> +
> +static void update_context(VignetteContext *s, AVFilterLink *inlink, AVFrame *frame)
> +{
> +    int x, y;
> +    double *dst = s->fmap;
> +    int dst_linesize = s->fmap_linesize;
> +
> +    if (frame) {
> +        s->var_values[VAR_N]   = inlink->frame_count;
> +        s->var_values[VAR_T]   = TS2T(frame->pts, inlink->time_base);
> +        s->var_values[VAR_PTS] = TS2D(frame->pts);
> +    } else {
> +        s->var_values[VAR_N]   = 0;
> +        s->var_values[VAR_T]   = NAN;
> +        s->var_values[VAR_PTS] = NAN;
> +    }
> +
> +    s->a = av_clipf(av_expr_eval(s->a_pexpr, s->var_values, NULL), 0, M_PI_2);
> +    s->x0 = av_expr_eval(s->x0_pexpr, s->var_values, NULL);
> +    s->y0 = av_expr_eval(s->y0_pexpr, s->var_values, NULL);
> +
> +    if (s->backward) {
> +        for (y = 0; y < inlink->h; y++) {
> +            for (x = 0; x < inlink->w; x++)
> +                dst[x] = 1. / get_natural_factor(s, x, y);
> +            dst += dst_linesize;
> +        }
> +    } else {
> +        for (y = 0; y < inlink->h; y++) {
> +            for (x = 0; x < inlink->w; x++)
> +                dst[x] = get_natural_factor(s, x, y);
> +            dst += dst_linesize;
> +        }
> +    }
> +}
> +
> +static int filter_frame(AVFilterLink *inlink, AVFrame *in)
> +{
> +    unsigned x, y;
> +    AVFilterContext *ctx = inlink->dst;
> +    VignetteContext *s = ctx->priv;
> +    AVFilterLink *outlink = inlink->dst->outputs[0];
> +    AVFrame *out;
> +
> +    out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
> +    if (!out) {
> +        av_frame_free(&in);
> +        return AVERROR(ENOMEM);
> +    }
> +    av_frame_copy_props(out, in);
> +
> +    if (s->eval_mode == EVAL_MODE_FRAME)
> +        update_context(s, inlink, in);
> +
> +    if (s->desc->flags & PIX_FMT_RGB) {
> +        uint8_t       *dst = out->data[0];
> +        const uint8_t *src = in ->data[0];
> +        const double *fmap = s->fmap;
> +        const int dst_linesize = out->linesize[0];
> +        const int src_linesize = in ->linesize[0];
> +        const int fmap_linesize = s->fmap_linesize;
> +
> +        for (y = 0; y < inlink->h; y++) {
> +            uint8_t       *dstp = dst;
> +            const uint8_t *srcp = src;
> +
> +            for (x = 0; x < inlink->w; x++, dstp += 3, srcp += 3) {
> +                const double f = fmap[x];
> +
> +                dstp[0] = av_clip_uint8(srcp[0] * f);
> +                dstp[1] = av_clip_uint8(srcp[1] * f);
> +                dstp[2] = av_clip_uint8(srcp[2] * f);

i think with just 8bit this kind of filter is prone to banding
artifacts without dither

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130513/bb5dd6a5/attachment.asc>