[FFmpeg-devel] [PATCH] waveform monitor filter

[Nicolas George]

Le primidi 11 nivôse, an CCXX, Mark Himsley a écrit :
> This is quite an early/raw version of a luminance waveform monitor.
> I'd like to add to it with vector-scope, RGB parade, etc.
> 
> At this stage I'm just looking for some comments from you experienced
> folks whether I'm doing something crazy.
> 
> I've been testing this filter with this, to overlay the waveform
> monitor on top of the original video (thanks to Nicholas for the
> fifo workaround).
> 
> ffmpeg -i input.mov -vf
> "split[one][two];[two]wfm_luma,scale=256:256[wfm];[one]fifo,[wfm]overlay=0:0"
> -aspect 16:9 -vcodec dvvideo -acodec pcm_s16le -ac 2 -y output.mov

I do not really know what a waveform monitor is, I can only comment on a few
points of detail.

Sounds interesting, though. I look forward to reading the docs.

> + * Copyright (c) 2011 Mark Himsley

Too late...

By the way, happy new year to everyone.

> +typedef struct
> +{

The convention is to put the brace on the same line, I believe.

> +    float             gama;

Isn't it gamma?

And do you have a particular reason to use a float rather than a double?

> +    if (ctx->inputs[0]) {

I hope this test is not necessary, because I never did it.

> +    wfm->luma_data = av_malloc(wfm->w * wfm->h * sizeof(wfm->luma_data));
> +    wfm->y_lut = av_malloc((1 + inlink->h) * sizeof(wfm->y_lut));
> +    wfm->a_lut = av_malloc((1 + inlink->h) * sizeof(wfm->a_lut));

Do not forget, in the final version, to check the return values of the
mallocs.

> +    /** calculate 8 bit luminance LUT emulating a waveform monitor display within CCIR-601 space */

I am not sure that comments of that kind deserve doxyfication.

> +        wfm->y_lut[h] = FFMIN(70+pow(wfm->gain*h,1/wfm->gama)*164/pow(inlink->h,1/wfm->gama),235);

Some room to breath may be welcome. And possibly an explanation of what it
does.

> +    memset(wfm->luma_data, 0, wfm->w * wfm->h * sizeof(wfm->luma_data));

sizeof(*luma_data), I suppose?

> +        y = outpicref->data[0] + (wfm->h - dy - 1) * outpicref->linesize[0];
> +        a = outpicref->data[3] + (wfm->h - dy - 1) * outpicref->linesize[0];

linesize[3]?

> +        .rej_perms        = AV_PERM_REUSE2|AV_PERM_PRESERVE,},
> +        { .name = NULL}},

The braces' placement seems a bit odd.

Regards,

-- 
  Nicolas George
-------------- 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/20120101/d09145a6/attachment.asc>