On Wed, Aug 24, 2016 at 07:34:22AM -0500, Burt P wrote:
> A new filter that provides HDCD decoding, using libhdcd.
> https://github.com/bp0/libhdcd
> 
> Requires ./configure --enable-libhdcd
> 
> Signed-off-by: Burt P <pbu...@gmail.com>
> ---
>  Changelog                |   1 +
>  configure                |   4 +
>  doc/filters.texi         |  47 +++++++++++
>  libavfilter/Makefile     |   1 +
>  libavfilter/af_hdcd.c    | 199 
> +++++++++++++++++++++++++++++++++++++++++++++++
>  libavfilter/allfilters.c |   1 +
>  6 files changed, 253 insertions(+)
>  create mode 100644 libavfilter/af_hdcd.c

This probably deserves a mention in doc/general.texi and a version bump
for libavfilter.

> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -647,6 +647,53 @@ avconv -i fl -i fr -i fc -i sl -i sr -i lfe 
> -filter_complex
> +
> +When using the filter with wav, note the default encoding for wav is 16-bit,

.. with WAV, note that the default encoding for WAV is 16-bit,

> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -37,6 +37,7 @@ OBJS-$(CONFIG_COMPAND_FILTER)                += af_compand.o
>  OBJS-$(CONFIG_JOIN_FILTER)                   += af_join.o
>  OBJS-$(CONFIG_RESAMPLE_FILTER)               += af_resample.o
>  OBJS-$(CONFIG_VOLUME_FILTER)                 += af_volume.o
> +OBJS-$(CONFIG_HDCD_FILTER)                   += af_hdcd.o

order

> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -60,6 +60,7 @@ void avfilter_register_all(void)
>      REGISTER_FILTER(JOIN,           join,           af);
>      REGISTER_FILTER(RESAMPLE,       resample,       af);
>      REGISTER_FILTER(VOLUME,         volume,         af);
> +    REGISTER_FILTER(HDCD,           hdcd,           af);

order

> --- /dev/null
> +++ b/libavfilter/af_hdcd.c
> @@ -0,0 +1,199 @@
> +
> +#include "libavutil/opt.h"
> +#include "libavutil/avassert.h"
> +#include "libavutil/channel_layout.h"
> +#include "formats.h"
> +#include "avfilter.h"
> +#include "internal.h"
> +#include "audio.h"
> +#include <hdcd/hdcd_simple.h>
> +#include <hdcd/hdcd_decode2.h> /* only for HDCD_ANA_*_DESC defines */

System headers should be placed before local headers.

> +typedef struct HDCDContext {
> +    const AVClass *class;
> +
> +    hdcd_simple_t *shdcd;

The _t namespace is reserved for POSIX, the library should not invade it.

> +static const AVOption hdcd_options[] = {
> +    { "analyze_mode",  "Replace audio with solid tone and signal some 
> processing aspect in the amplitude.",
> +        OFFSET(analyze_mode), AV_OPT_TYPE_INT, { .i64=HDCD_ANA_OFF }, 0, 
> SHDCD_ANA_MAX, A, "analyze_mode"},
> +        { "off", HDCD_ANA_OFF_DESC, 0, AV_OPT_TYPE_CONST, 
> {.i64=SHDCD_ANA_OFF}, 0, 0, A, "analyze_mode" },
> +        { "lle", HDCD_ANA_LLE_DESC, 0, AV_OPT_TYPE_CONST, 
> {.i64=SHDCD_ANA_LLE}, 0, 0, A, "analyze_mode" },
> +        { "pe",  HDCD_ANA_PE_DESC,  0, AV_OPT_TYPE_CONST, 
> {.i64=SHDCD_ANA_PE},  0, 0, A, "analyze_mode" },
> +        { "cdt", HDCD_ANA_CDT_DESC, 0, AV_OPT_TYPE_CONST, 
> {.i64=SHDCD_ANA_CDT}, 0, 0, A, "analyze_mode" },
> +        { "tgm", HDCD_ANA_TGM_DESC, 0, AV_OPT_TYPE_CONST, 
> {.i64=SHDCD_ANA_TGM}, 0, 0, A, "analyze_mode" },
> +        { "pel", SHDCD_ANA_PEL_DESC, 0, AV_OPT_TYPE_CONST, 
> {.i64=SHDCD_ANA_PEL}, 0, 0, A, "analyze_mode" },
> +        { "ltgm", SHDCD_ANA_LTGM_DESC, 0, AV_OPT_TYPE_CONST, 
> {.i64=SHDCD_ANA_LTGM}, 0, 0, A, "analyze_mode" },
> +    {NULL}
> +};

Indentation is off, spaces around = and inside {}.

> +static int filter_frame(AVFilterLink *inlink, AVFrame *in)
> +{
> +    out = ff_get_audio_buffer(outlink, in->nb_samples);
> +    if (!out) {
> +        av_frame_free(&in);
> +        return AVERROR(ENOMEM);
> +    }
> +    result = av_frame_copy_props(out, in);
> +    if (result) {
> +        av_frame_free(&in);
> +        return result;
> +    }

Where does out get freed if av_frame_copy_props() fails?

> +static int query_formats(AVFilterContext *ctx)
> +{
> +    AVFilterFormats *in_formats;
> +    AVFilterFormats *out_formats;
> +    AVFilterFormats *sample_rates = NULL;

nit: merge the lines

> +static av_cold void uninit(AVFilterContext *ctx)
> +{
> +    shdcd_detect_str(s->shdcd, detect_str, sizeof(detect_str));
> +    shdcd_free(s->shdcd);

Why does the library use an shdcd instead of an hdcd prefix?

> +static void af_hdcd_log(const void *priv, const char* fmt, va_list args)

char *fmt

> +    av_log(ctx, AV_LOG_VERBOSE, "Analyze mode: [%d] %s\n",
> +        s->analyze_mode, shdcd_analyze_mode_desc(s->analyze_mode) );

Indentation is off.

Diego
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to