[FFmpeg-devel] [PATCH] Implement hdcd filtering

[Paul B Mahol]

On 3/22/16, Benjamin St <benjaminst123 at gmail.com> wrote:
> This patch applies filtering/decoding for hdcds(see ticket #4441) . The
> filter is heavily based on
> https://github.com/kode54/foo_hdcd/. (Is this ok? Copyright?)
>
> Discuss, Review
>
> Thank you, Benjamin
>

[...]

> +};
> +
> +static uint8_t readaheadtab[1 << 8] = {

const?

> +        0x03, 0x02, 0x01, 0x01, 0x1f, 0x1e, 0x1f, 0x11, 0x1f, 0x1e, 0x1f, 0x1d, 0x1f, 0x1e, 0x1f,
> +        0x10, 0x1f, 0x1e, 0x1f, 0x1d, 0x1f, 0x1e, 0x1f, 0x1c, 0x1f, 0x1e, 0x1f, 0x1d, 0x1f, 0x1e,
> +        0x1f, 0x0f, 0x1f, 0x1e, 0x1f, 0x1d, 0x1f, 0x1e, 0x1f, 0x1c, 0x1f, 0x1e, 0x1f, 0x1d, 0x1f,
> +        0x1e, 0x1f, 0x1b, 0x1f, 0x1e, 0x1f, 0x1d, 0x1f, 0x1e, 0x1f, 0x1c, 0x1f, 0x1e, 0x1f, 0x1d,
> +        0x1f, 0x1e, 0x0e, 0x19, 0x07, 0x1e, 0x1f, 0x1d, 0x1f, 0x1e, 0x1f, 0x1c, 0x1f, 0x1e, 0x1f,
> +        0x1d, 0x1f, 0x1e, 0x1f, 0x1b, 0x1f, 0x1e, 0x1f, 0x1d, 0x1f, 0x1e, 0x1f, 0x1c, 0x1f, 0x1e,

[...]

> +
> +static float gaintab[(15 << 7) + 1] = {

const?

[...]

> +    0.427424, 0.427232, 0.427040, 0.426848, 0.426656, 0.426464, 0.426273, 0.426081, 0.425889, 0.425698, 0.425507, 0.425315, 0.425124, 0.424933, 0.424742,
> +    0.424551, 0.424360, 0.424169, 0.423978, 0.423788, 0.423597, 0.423407, 0.423216, 0.423026, 0.422836, 0.422646, 0.422456, 0.422266, 0.422076, 0.421886,
> +    0.421697
> +};

missing new line.

> +static float gaintabup[(15 << 7) + 1] = {

const?

> +    1.000000, 0.999550, 0.999101, 0.998652, 0.998203, 0.997754, 0.997305, 0.996857, 0.996409, 0.995961, 0.995513, 0.995065, 0.994618, 0.994171, 0.993724,
> +    0.993277, 0.992830, 0.992384, 0.991938, 0.991492, 0.991046, 0.990600, 0.990155, 0.989710, 0.989265, 0.988820, 0.988375, 0.987931, 0.987487, 0.987043,

[...]

> +    0.433230, 0.433035, 0.432841, 0.432646, 0.432452, 0.432257, 0.432063, 0.431869, 0.431674, 0.431480, 0.431286, 0.431092, 0.430899, 0.430705, 0.430511,
> +    0.430318, 0.430124, 0.429931, 0.429737, 0.429544, 0.429351, 0.429158, 0.428965, 0.428772, 0.428579, 0.428387, 0.428194, 0.428002, 0.427809, 0.427617,
> +    0.427424, 0.427232, 0.427040, 0.426848, 0.426656, 0.426464, 0.426273, 0.426081, 0.425889, 0.425698, 0.425507, 0.425315, 0.425124, 0.424933, 0.424742,
> +    0.424551, 0.424360, 0.424169, 0.423978, 0.423788, 0.423597, 0.423407, 0.423216, 0.423026, 0.422836, 0.422646, 0.422456, 0.422266, 0.422076, 0.421886,
> +    0.421697
> +};
> +
> +#if !defined(min)
> +#define min(x, y) ((x) <= (y) ? (x) : (y))

Please use FFMIN()

> +#endif
> +
> +#define GAIN_APPLY(sample, gain) (sample) *= (gain)
> +
> +#define CHANNEL_NUM 2
> +
> +typedef struct {
> +    uint64_t window;
> +    unsigned char readahead, arg, control;
> +    int running_gain;
> +    unsigned sustain, sustain_reset;
> +    int code_counterA;
> +    int code_counterB;
> +    int code_counterC;
> +} hdcd_state_t;
> +
> +typedef struct HDCDContext {
> +    const AVClass *class;
> +    hdcd_state_t state[CHANNEL_NUM];

This will crash if there is >2 channels
Either limit filter to stereo and mono or allocate this differently.

> +} HDCDContext;
> +
> +
> +static const AVOption hdcd_options[] = {
> +     {NULL}

Please remove this if its not going to be used.

> +};

New line.

> +AVFILTER_DEFINE_CLASS(hdcd);
> +
> +#define GAIN_APPLY(sample, gain) (sample) *= (gain)
> +
> +static void hdcd_reset(hdcd_state_t *state, unsigned rate) {

Here and in every other function, { must be on own, separate line like
in every other filter.

> +    state->window = 0;
> +    state->readahead = 32;
> +    state->arg = 0;
> +    state->control = 0;
> +
> +    state->running_gain = 0;
> +
> +    state->sustain = 0;
> +    state->sustain_reset = rate * 10;
> +
> +    state->code_counterA = 0;
> +    state->code_counterB = 0;
> +    state->code_counterC = 0;
> +}
> +

[...]

> +    HDCDContext *s = ctx->priv;
> +
> +    if (inlink->channels > CHANNEL_NUM) {
> +        av_log(ctx, AV_LOG_ERROR, "HDCDs can have a maximum of %d channels\n", CHANNEL_NUM);
> +        return AVERROR(EINVAL);

This is wrong. Either use av_calloc or modify query formats to accepts
only mono/stereo channel layout.

> +    }
> +
> +    for (c = 0; c < inlink->channels; c++) {
> +        hdcd_reset(&s->state[c], inlink->sample_rate);
> +    }
> +
> +    return 0;
> +}
> +
> +static const AVFilterPad avfilter_af_hdcd_inputs[] = {
> +    {
> +        .name         = "default",
> +        .type         = AVMEDIA_TYPE_AUDIO,
> +        .config_props = config_input,
> +        .filter_frame = filter_frame,
> +    },
> +    { NULL }
> +};
> +
> +static const AVFilterPad avfilter_af_hdcd_outputs[] = {
> +    {
> +        .name = "default",
> +        .type = AVMEDIA_TYPE_AUDIO,
> +    },
> +    { NULL }
> +};
> +
> +AVFilter ff_af_hdcd = {
> +    .name          = "hdcd",
> +    .description   = NULL_IF_CONFIG_SMALL("Apply hdcd decoding."),

What is hdcd? Could you put it into this long description?

> +    .priv_size     = sizeof(HDCDContext),
> +    .priv_class    = &hdcd_class,
> +    .uninit        = uninit,
> +    .query_formats = query_formats,
> +    .inputs        = avfilter_af_hdcd_inputs,
> +    .outputs       = avfilter_af_hdcd_outputs,
> +};
> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> index d6145d6..44d9f5f 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -124,6 +124,7 @@ void avfilter_register_all(void)
>      REGISTER_FILTER(VIBRATO,        vibrato,        af);
>      REGISTER_FILTER(VOLUME,         volume,         af);
>      REGISTER_FILTER(VOLUMEDETECT,   volumedetect,   af);
> +    REGISTER_FILTER(HDCD,           hdcd,           af);

Alphabetical order.

>
>      REGISTER_FILTER(AEVALSRC,       aevalsrc,       asrc);
>      REGISTER_FILTER(ANOISESRC,      anoisesrc,      asrc);
> --
> 2.5.0
>