On 12/6/15, Ganesh Ajjanagadde <gajja...@mit.edu> wrote:
> On Sun, Dec 6, 2015 at 3:58 PM, Paul B Mahol <one...@gmail.com> wrote:
>> Signed-off-by: Paul B Mahol <one...@gmail.com>
>> ---
>> libavfilter/Makefile | 1 +
>> libavfilter/af_itur468.c | 223
>> +++++++++++++++++++++++++++++++++++++++++++++++
>> libavfilter/allfilters.c | 1 +
>> 3 files changed, 225 insertions(+)
>> create mode 100644 libavfilter/af_itur468.c
>>
>> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
>> index 8884d1d..2a053d0 100644
>> --- a/libavfilter/Makefile
>> +++ b/libavfilter/Makefile
>> @@ -76,6 +76,7 @@ OBJS-$(CONFIG_EQUALIZER_FILTER) +=
>> af_biquads.o
>> OBJS-$(CONFIG_EXTRASTEREO_FILTER) += af_extrastereo.o
>> OBJS-$(CONFIG_FLANGER_FILTER) += af_flanger.o
>> generate_wave_table.o
>> OBJS-$(CONFIG_HIGHPASS_FILTER) += af_biquads.o
>> +OBJS-$(CONFIG_ITUR468_FILTER) += af_itur468.o
>> OBJS-$(CONFIG_JOIN_FILTER) += af_join.o
>> OBJS-$(CONFIG_LADSPA_FILTER) += af_ladspa.o
>> OBJS-$(CONFIG_LOWPASS_FILTER) += af_biquads.o
>> diff --git a/libavfilter/af_itur468.c b/libavfilter/af_itur468.c
>> new file mode 100644
>> index 0000000..9b0eddc
>> --- /dev/null
>> +++ b/libavfilter/af_itur468.c
>> @@ -0,0 +1,223 @@
>> +/*
>> + * Copyright (c) 2010 Fons Adriaensen <f...@kokkinizita.net>
>> + *
>> + * This file is part of FFmpeg.
>> + *
>> + * FFmpeg is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * FFmpeg is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> along
>> + * with FFmpeg; if not, write to the Free Software Foundation, Inc.,
>> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
>> + */
>> +
>> +#include <float.h>
>> +#include <math.h>
>> +
>> +#include "libavutil/avassert.h"
>> +#include "libavutil/avstring.h"
>> +#include "libavutil/channel_layout.h"
>> +#include "libavutil/dict.h"
>> +#include "libavutil/xga_font_data.h"
>> +#include "libavutil/opt.h"
>> +#include "audio.h"
>> +#include "avfilter.h"
>> +#include "formats.h"
>> +#include "internal.h"
>> +
>> +typedef struct ITUR468Filter {
>> + double whp;
>> + double a11, a12;
>> + double a21, a22;
>> + double a31, a32;
>> + double b30, b31, b32;
>> + double zhp;
>> + double z11, z12;
>> + double z21, z22;
>> + double z31, z32;
>> +
>> + double a1, b1;
>> + double a2, b2;
>> + double z1, z2;
>> +} ITUR468Filter;
>> +
>> +typedef struct {
>> + const AVClass *class;
>> +
>> + ITUR468Filter *filter;
>> +} ITUR468Context;
>> +
>> +static int config_output(AVFilterLink *outlink)
>> +{
>> + AVFilterContext *ctx = outlink->src;
>> + AVFilterLink *inlink = ctx->inputs[0];
>> + ITUR468Context *s = ctx->priv;
>> + int c;
>> +
>> + s->filter = av_calloc(inlink->channels, sizeof(*s->filter));
>> + if (!s->filter)
>> + return AVERROR(ENOMEM);
>> +
>> + for (c = 0; c < inlink->channels; c++) {
>> + ITUR468Filter *f = &s->filter[c];
>> +
>> + f->whp = 3.8715217e-01;
>> + f->a11 = -8.4163201e-01;
>> + f->a12 = 3.0498350e-01;
>> + f->a21 = -6.5680242e-01;
>> + f->a22 = 2.3733993e-01;
>> + f->a31 = -3.3843556e-01;
>> + f->a32 = 4.3756709e-01;
>> + f->b30 = 9.8607997e-01;
>> + f->b31 = 5.4846389e-01;
>> + f->b32 = -8.2465158e-02;
>> + f->a1 = 670. / inlink->sample_rate;
>> + f->b1 = 3.5 / inlink->sample_rate;
>> + f->a2 = 6.6 / inlink->sample_rate;
>> + f->b2 = 0.65 / inlink->sample_rate;
>
> 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.
>
> With the information you provide, all I can do is say "whatever,
> another filter obtained from somewhere on the internet that is being
> imported into FFmpeg".
>
>> + z1 -= z1 * f->b1;
>> + if (x > z1)
>> + z1 += f->a1 * (x - z1);
>> +
>> + z2 -= z2 * f->b2;
>> + if (z1 > z2)
>> + z2 += f->a2 * (z1 - z2);
>> + }
>> +
>> + f->zhp = zhp;
>> + f->z11 = z11;
>> + f->z12 = z12;
>> + f->z21 = z21;
>> + f->z22 = z22;
>> + f->z31 = z31;
>> + f->z32 = z32;
>> + f->z1 = z1;
>> + f->z2 = z2;
>> + if (metadata) {
>> + uint8_t value[128];
>> + uint8_t key[128];
>> +
>> + snprintf(key, sizeof(key), "lavfi.itur468.%d.noise", c + 1);
>> + snprintf(value, sizeof(value), "%f", 1.1453 * f->z2);
>> + av_dict_set(metadata, key, value, 0);
>> + }
>> + }
>> +
>> + return ff_filter_frame(ctx->outputs[0], in);
>> +}
>> +
>> +static int query_formats(AVFilterContext *ctx)
>> +{
>> + AVFilterFormats *formats;
>> + AVFilterChannelLayouts *layouts;
>> + AVFilterLink *inlink = ctx->inputs[0];
>> + AVFilterLink *outlink = ctx->outputs[0];
>> + int ret;
>> +
>> + static const enum AVSampleFormat sample_fmts[] = {
>> AV_SAMPLE_FMT_DBLP, AV_SAMPLE_FMT_NONE };
>> + static const int input_srate[] = {48000, -1};
>> +
>> + formats = ff_make_format_list(sample_fmts);
>> + if ((ret = ff_formats_ref(formats, &inlink->out_formats)) < 0 ||
>> + (ret = ff_formats_ref(formats, &outlink->in_formats)) < 0)
>> + return ret;
>> +
>> + layouts = ff_all_channel_counts();
>> + if ((ret = ff_channel_layouts_ref(layouts,
>> &inlink->out_channel_layouts)) < 0 ||
>> + (ret = ff_channel_layouts_ref(layouts,
>> &outlink->in_channel_layouts)) < 0)
>> + return ret;
>> +
>> + formats = ff_make_format_list(input_srate);
>> + if ((ret = ff_formats_ref(formats, &inlink->out_samplerates)) < 0 ||
>> + (ret = ff_formats_ref(formats, &outlink->in_samplerates)) < 0)
>> + return ret;
>
> Still leaky, I hope you recognize it.
>
> I have provided illustrations for easily resolving this, see the CID
> patchset.
>
> By the way, what is the RFC/WIP for: FFmpeg seems to basically accept
> any filter under the sun, regardless of importance/utility/review
> status?
> I have no idea.
>
> On a positive note: thanks for doing real arithmetic.
>
> [...]
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
