[FFmpeg-devel] [RFC][WIP][PATCH] avfilter: add ITU-R 468-4 noise meter

[Ganesh Ajjanagadde]

On Sun, Dec 6, 2015 at 6:17 PM, Paul B Mahol <onemda at gmail.com> wrote:
> On 12/6/15, Ganesh Ajjanagadde <gajjanag at mit.edu> wrote:
>> On Sun, Dec 6, 2015 at 3:58 PM, Paul B Mahol <onemda at gmail.com> wrote:
[...]
>>
>> Please add a comment with a link: where do these "magic" numbers come from?
>>
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static int filter_frame(AVFilterLink *inlink, AVFrame *in)
>>> +{
>>> +    AVFilterContext *ctx = inlink->dst;
>>> +    ITUR468Context *s = ctx->priv;
>>> +    AVDictionary **metadata;
>>> +    metadata = avpriv_frame_get_metadatap(in);
>>> +    int n, c;
>>> +
>>> +    for (c = 0; c < inlink->channels; c++) {
>>> +        ITUR468Filter *f = &s->filter[c];
>>> +        double *src = (double *)in->extended_data[c];
>>> +        double out, x, zhp, z11, z12, z21, z22, z31, z32, z1, z2;
>>> +
>>> +        zhp = f->zhp;
>>> +        z11 = f->z11;
>>> +        z12 = f->z12;
>>> +        z21 = f->z21;
>>> +        z22 = f->z22;
>>> +        z31 = f->z31;
>>> +        z32 = f->z32;
>>> +        z1  = f->z1;
>>> +        z2  = f->z2;
>>> +
>>> +        for (n = 0; n < in->nb_samples; n++) {
>>> +            x = src[n];
>>> +            zhp += f->whp * (x - zhp) + 1e-20;
>>> +            x -= zhp;
>>> +            x -= f->a11 * z11 + f->a12 * z12;
>>> +            z12 = z11;
>>> +            z11 = x;
>>> +            x -= f->a21 * z21 + f->a22 * z22;
>>> +            z22 = z21;
>>> +            z21 = x;
>>> +            x -= f->a31 * z31 + f->a32 * z32;
>>> +            out = f->b30 * x + f->b31 * z31 + f->b32 * z32;
>>> +            z32 = z31;
>>> +            z31 = x;
>>> +            x = out;
>>> +            x = fabs(x) + 1e-30;
>>
>> All this 1e-20, 1e-30: is is needed for well-conditioning the output
>> or what? A lot of the questions I ask can simply be clarified by
>> adding a one-liner link to the actual source of this filter, which for
>> some obscure reason you seem to not do.
>> How can you expect the reviewer to know the significance of all these
>> magic numbers?
>
> It is from obscure jnoisemeter utility, there is even ladspa plugin
> for it (bobdsp)
>
> Search for ITU-R 468 for more info.
>

Thanks for the reference. Can you please add 1 line saying e.g
"inspired/taken from jnoisemeter:
http://wiki.linuxaudio.org/apps/all/jnoisemeter"? It will help a lot,
not just for me, but for anyone else interested in contributing to it.

I also highly encourage such things for other filters as well.

As for further review, I most likely can't do it for reasons of time.

[...]

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel