Hi, Nice patch. I've been considering stealing it from mplayer for some time, thanks for beating me to it.
My comments are mostly details, feel free to ignore the ones prefixed with nit: On Sun, 27 Apr 2014 15:37:03 +0200, Alessandro Ghedini <[email protected]> wrote: > diff --git a/libavfilter/af_bs2b.c b/libavfilter/af_bs2b.c > new file mode 100644 > index 0000000..1b7d9fc > --- /dev/null > +++ b/libavfilter/af_bs2b.c > @@ -0,0 +1,214 @@ > +/* > + * This file is part of Libav. > + * > + * Libav is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * Libav 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 > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with Libav; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 > USA > + */ > + > +/** > + * @file > + * Bauer stereo-to-binaural filter > + */ > + > +#include <bs2b.h> > + > +#include "libavutil/channel_layout.h" > +#include "libavutil/common.h" > +#include "libavutil/opt.h" > + > +#include "audio.h" > +#include "avfilter.h" > +#include "formats.h" > +#include "internal.h" > + > +typedef struct Bs2bContext { > + const AVClass *class; > + > + int profile; > + int fcut; > + int feed; > + > + t_bs2bdp bs2bp; > + > + void (*filter)(t_bs2bdp bs2bdp, uint8_t *sample, int n); > +} Bs2bContext; > + > +#define OFFSET(x) offsetof(Bs2bContext, x) > +#define A AV_OPT_FLAG_AUDIO_PARAM > + > +static const AVOption options[] = { > + { "profile", "Apply a pre-defined crossfeed level", > + OFFSET(profile), AV_OPT_TYPE_INT, { .i64 = BS2B_DEFAULT_CLEVEL > }, 0, INT_MAX, A, "profile" }, > + { "default", "default profile", 0, AV_OPT_TYPE_CONST, { .i64 = > BS2B_DEFAULT_CLEVEL }, 0, 0, A, "profile" }, > + { "cmoy", "Chu Moy circuit", 0, AV_OPT_TYPE_CONST, { .i64 = > BS2B_CMOY_CLEVEL }, 0, 0, A, "profile" }, > + { "jmeier", "Jan Meier circuit", 0, AV_OPT_TYPE_CONST, { .i64 = > BS2B_JMEIER_CLEVEL }, 0, 0, A, "profile" }, > + { "fcut", "Set cut frequency (in Hz)", > + OFFSET(fcut), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, BS2B_MAXFCUT, A > }, > + { "feed", "Set feed level (in Hz)", > + OFFSET(feed), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, BS2B_MAXFEED, A > }, > + { NULL }, > +}; > + > +static const AVClass bs2b_class = { > + .class_name = "bs2b filter", > + .item_name = av_default_item_name, > + .option = options, > + .version = LIBAVUTIL_VERSION_INT, > +}; > + > +static av_cold int init(AVFilterContext *ctx) > +{ > + Bs2bContext *bs2b = ctx->priv; > + > + if (!(bs2b->bs2bp = bs2b_open())) > + return AVERROR(ENOMEM); > + > + bs2b_set_level(bs2b->bs2bp, bs2b->profile); > + > + if (bs2b->fcut) > + bs2b_set_level_fcut(bs2b->bs2bp, bs2b->fcut); > + > + if (bs2b->feed) > + bs2b_set_level_feed(bs2b->bs2bp, bs2b->feed); > + > + return 0; > +} > + > +static int query_formats(AVFilterContext *ctx) > +{ > + AVFilterFormats *formats = NULL; > + AVFilterChannelLayouts *layouts; > + > + static const enum AVSampleFormat sample_fmts[] = { > + AV_SAMPLE_FMT_U8, > + AV_SAMPLE_FMT_S16, > + AV_SAMPLE_FMT_S32, > + AV_SAMPLE_FMT_FLT, > + AV_SAMPLE_FMT_DBL, > + AV_SAMPLE_FMT_NONE, > + }; > + > + layouts = ff_all_channel_layouts(); > + ff_add_channel_layout(&layouts, av_get_default_channel_layout(2)); Why is this line here? > + if (!layouts) > + return AVERROR(ENOMEM); > + ff_set_common_channel_layouts(ctx, layouts); > + > + formats = ff_make_format_list(sample_fmts); > + if (!formats) > + return AVERROR(ENOMEM); > + ff_set_common_formats(ctx, formats); > + > + formats = ff_all_samplerates(); > + if (!formats) > + return AVERROR(ENOMEM); > + ff_set_common_samplerates(ctx, formats); > + > + return 0; > +} > + > +static int filter_frame(AVFilterLink *inlink, AVFrame *buf) nit: 'buf' is used all over the code because lavfi used to use a struct called AVFilterBufferRef and I was too lazy to rename everything when I replaced it with AVFrame. Nowadays we usually use 'frame' in new code. > +{ > + int ret; > + AVFrame *out_buf; > + > + Bs2bContext *bs2b = inlink->dst->priv; > + AVFilterLink *outlink = inlink->dst->outputs[0]; > + > + if (av_frame_is_writable(buf)) { > + out_buf = buf; > + } else { > + out_buf = ff_get_audio_buffer(inlink, buf->nb_samples); > + if (!out_buf) > + return AVERROR(ENOMEM); > + ret = av_frame_copy_props(out_buf, buf); > + if (ret < 0) { > + av_frame_free(&out_buf); > + av_frame_free(&buf); > + return ret; > + } > + } > + > + bs2b->filter(bs2b->bs2bp, out_buf->extended_data[0], > out_buf->nb_samples); > + > + if (buf != out_buf) > + av_frame_free(&buf); > + > + return ff_filter_frame(outlink, out_buf); > +} > + > +static int config_output(AVFilterLink *outlink) > +{ > + AVFilterContext *ctx = outlink->src; > + Bs2bContext *bs2b = ctx->priv; > + AVFilterLink *inlink = ctx->inputs[0]; > + > + int srate = inlink->sample_rate; > + > + switch (av_get_packed_sample_fmt(inlink->format)) { av_get_packed_sample_fmt is not needed, you're only declaring packed formats in query_formats(), so you'll never get a planar format here. > + case AV_SAMPLE_FMT_U8: > + bs2b->filter = bs2b_cross_feed_u8; > + break; > + case AV_SAMPLE_FMT_S16: > + bs2b->filter = bs2b_cross_feed_s16; > + break; > + case AV_SAMPLE_FMT_S32: > + bs2b->filter = bs2b_cross_feed_s32; > + break; > + case AV_SAMPLE_FMT_FLT: > + bs2b->filter = bs2b_cross_feed_f; > + break; > + case AV_SAMPLE_FMT_DBL: > + bs2b->filter = bs2b_cross_feed_d; > + break; > + default: > + return AVERROR(ENOSYS); I think AVERROR_BUG is even more appropriate here, since this should never ever happen. > + } > + > + if ((srate < BS2B_MINSRATE) || (srate > BS2B_MAXSRATE)) > + return AVERROR(ENOSYS); > + > + bs2b_set_srate(bs2b->bs2bp, srate); > + > + return 0; > +} > + > +static const AVFilterPad avfilter_af_bs2b_inputs[] = { nit: this overlong naming used all over the code is caused by a script moving the input/output declarations out of AVFilter declaration. You don't have to replicate it, just bsb2_inputs/outputs will do just fine. -- Anton Khirnov _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
