[FFmpeg-devel] [PATCH 2/4] pan: add channel mapping capability.
Nicolas George
nicolas.george at normalesup.org
Wed Jan 18 15:05:09 CET 2012
Le nonidi 29 nivôse, an CCXX, Clément Bœsch a écrit :
> From: Clément Bœsch <clement.boesch at smartjog.com>
>
> ---
> doc/filters.texi | 43 +++++++++++++++++++++
> libavfilter/af_pan.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 142 insertions(+), 1 deletions(-)
>
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 3c9f554..7d24389 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -315,6 +315,9 @@ Ported from SoX.
> Mix channels with specific gain levels. The filter accepts the output
> channel layout followed by a set of channels definitions.
>
> +This filter is also designed to remap efficiently the channels of an audio
> +stream.
> +
> The filter accepts parameters of the form:
> "@var{l}:@var{outdef}:@var{outdef}:..."
>
> @@ -342,6 +345,8 @@ If the `=' in a channel specification is replaced by `<', then the gains for
> that specification will be renormalized so that the total is 1, thus
> avoiding clipping noise.
>
> + at subsection Mixing examples
> +
> For example, if you want to down-mix from stereo to mono, but with a bigger
> factor for the left channel:
> @example
> @@ -358,6 +363,44 @@ Note that @command{ffmpeg} integrates a default down-mix (and up-mix) system
> that should be preferred (see "-ac" option) unless you have very specific
> needs.
>
> + at subsection Remapping examples
> +
> +The channel remapping will be effective if, and only if:
> +
> + at itemize
> + at item gain coefficients are zeroes or ones,
> + at item only one input per channel output,
> + at item the number of output channels is supported by libswresample.
Maybe say the value of the maximum number of channels?
> + at end itemize
> +
> +If all these conditions are satisfied, the filter will notice the user ("Pure
Notify, not notice.
> +channel mapping detected"), and use an optimized and lossless method to do the
> +remapping.
> +
> +For example, if you have a 5.1 source and want a stereo audio stream by
> +dropping the extra channels:
> + at example
> +pan="stereo: c0=FL : c1=FR"
> + at end example
> +
> +Given the same source, you can also switch front left and front right channels
> +and keep the input channel layout:
> + at example
> +pan="5.1: c0=c1 : c1=c0 : c2=c2 : c3=c3 : c4=c4 : c5=c5"
> + at end example
> +
> +If the input is a stereo audio stream, you can mute the front left channel (and
> +still keep the stereo channel layout) with:
> + at example
> +pan="stereo:c1=c1"
> + at end example
> +
> +Still with a stereo audio stream input, you can copy the right channel in both
> +front left and right:
> + at example
> +pan="stereo: c0=FR : c1=FR"
> + at end example
> +
> @section silencedetect
>
> Detect silence in an audio stream.
> diff --git a/libavfilter/af_pan.c b/libavfilter/af_pan.c
> index add14b0..37831da 100644
> --- a/libavfilter/af_pan.c
> +++ b/libavfilter/af_pan.c
> @@ -30,6 +30,8 @@
> #include <stdio.h>
> #include "libavutil/audioconvert.h"
> #include "libavutil/avstring.h"
> +#include "libavutil/opt.h"
> +#include "libswresample/swresample.h"
> #include "avfilter.h"
> #include "internal.h"
>
> @@ -46,6 +48,14 @@ typedef struct {
> int need_renumber;
> int nb_input_channels;
> int nb_output_channels;
> +
> + int pure_gains;
pure_remap would be more accurate, no?
> + void (*filter_samples)(AVFilterLink *, AVFilterBufferRef *);
> +
> + /* channel mapping specific */
> + int ch[SWR_CH_MAX];
channel_map?
> + struct SwrContext *swr;
> + int sample_rate;
> } PanContext;
>
> static int parse_channel_name(char **arg, int *rchannel, int *rnamed)
> @@ -93,6 +103,7 @@ static av_cold int init(AVFilterContext *ctx, const char *args0, void *opaque)
> char *arg, *arg0, *tokenizer, *args = av_strdup(args0);
> int out_ch_id, in_ch_id, len, named;
> int nb_in_channels[2] = { 0, 0 }; // number of unnamed and named input channels
> + int output_ch_has_gain[MAX_CHANNELS] = { 0 };
> double gain;
>
> if (!args0) {
> @@ -111,6 +122,11 @@ static av_cold int init(AVFilterContext *ctx, const char *args0, void *opaque)
> }
> pan->nb_output_channels = av_get_channel_layout_nb_channels(pan->out_channel_layout);
>
> + /* assume pure channel re-mapping if the number of output channels is
> + * supported by libswresample */
> + if (pan->nb_output_channels < SWR_CH_MAX)
<=
> + pan->pure_gains = 1;
> +
> /* parse channel specifications */
> while ((arg = arg0 = av_strtok(NULL, ":", &tokenizer))) {
> /* channel name */
> @@ -162,6 +178,19 @@ static av_cold int init(AVFilterContext *ctx, const char *args0, void *opaque)
> "Can not mix named and numbered channels\n");
> return AVERROR(EINVAL);
> }
> + /* check if libswresample channel remapping can still be applied */
> + if (pan->pure_gains) {
> + /* channel mapping is effective only if 0% or 100% of a channel is
> + * selected... */
> + if (gain != 0. && gain != 1.) {
> + pan->pure_gains = 0;
> + } else if (gain == 1.) {
> + /* ...and if the output channel is only composed of one input */
> + if (output_ch_has_gain[out_ch_id])
> + pan->pure_gains = 0;
> + output_ch_has_gain[out_ch_id] = 1;
> + }
> + }
I would be much happier if it was completely separated from the parsing
loop. It would also avoid all the cumbersome boolean variables. Even better
in a separate function:
static int is_pure_remap(PanContext *pan)
{
int i, j, nb_ch;
for (i = 0; i < MAX_CHANNELS; i++) {
nb_ch = 0;
for (j = 0; j < MAX_CHANNELS; j++) {
if (pan->gain.d[i][j]) {
if (pan->gain.d[i][j] != 1.0)
return 0;
if (++nb_ch > 1)
return 0;
}
}
}
return 1;
}
> pan->gain.d[out_ch_id][in_ch_id] = gain;
> if (!*arg)
> break;
> @@ -179,6 +208,9 @@ static av_cold int init(AVFilterContext *ctx, const char *args0, void *opaque)
> return 0;
> }
>
> +static void filter_samples_channel_mapping(AVFilterLink *inlink, AVFilterBufferRef *insamples);
> +static void filter_samples_panning (AVFilterLink *inlink, AVFilterBufferRef *insamples);
> +
> static int query_formats(AVFilterContext *ctx)
> {
> PanContext *pan = ctx->priv;
> @@ -186,11 +218,19 @@ static int query_formats(AVFilterContext *ctx)
> AVFilterLink *outlink = ctx->outputs[0];
> AVFilterFormats *formats;
>
> + if (pan->pure_gains) {
> + /* libswr supports any sample and packing formats */
> + avfilter_set_common_sample_formats(ctx, avfilter_make_all_formats(AVMEDIA_TYPE_AUDIO));
> + avfilter_set_common_packing_formats(ctx, avfilter_make_all_packing_formats());
> + pan->filter_samples = filter_samples_channel_mapping;
> + } else {
> const enum AVSampleFormat sample_fmts[] = {AV_SAMPLE_FMT_S16, -1};
> const int packing_fmts[] = {AVFILTER_PACKED, -1};
>
> avfilter_set_common_sample_formats (ctx, avfilter_make_format_list(sample_fmts));
> avfilter_set_common_packing_formats(ctx, avfilter_make_format_list(packing_fmts));
> + pan->filter_samples = filter_samples_panning;
> + }
>
> // inlink supports any channel layout
> formats = avfilter_make_all_channel_layouts();
> @@ -222,6 +262,19 @@ static int config_props(AVFilterLink *link)
> }
> }
> }
> + // gains are pures, init the channel mapping array
Pure, no plural mark.
> + if (pan->pure_gains) {
> + for (i = 0; i < pan->nb_output_channels; i++) {
> + int ch_id = -1;
> + for (j = 0; j < pan->nb_input_channels; j++) {
> + if (pan->gain.d[i][j]) {
> + ch_id = j;
> + break;
> + }
> + }
> + pan->ch[i] = ch_id;
> + }
> + } else {
> // renormalize
> for (i = 0; i < pan->nb_output_channels; i++) {
> if (!((pan->need_renorm >> i) & 1))
> @@ -239,6 +292,7 @@ static int config_props(AVFilterLink *link)
> for (j = 0; j < pan->nb_input_channels; j++)
> pan->gain.d[i][j] /= t;
> }
> + }
> // summary
> for (i = 0; i < pan->nb_output_channels; i++) {
> cur = buf;
> @@ -249,6 +303,15 @@ static int config_props(AVFilterLink *link)
> }
> av_log(ctx, AV_LOG_INFO, "o%d = %s\n", i, buf);
> }
> + // add channel mapping summary if possible
> + if (pan->pure_gains) {
> + av_log(ctx, AV_LOG_INFO, "Pure channel mapping detected:");
> + for (i = 0; i < pan->nb_output_channels; i++)
> + if (pan->ch[i] < 0) av_log(ctx, AV_LOG_INFO, " M");
> + else av_log(ctx, AV_LOG_INFO, " %d", pan->ch[i]);
> + av_log(ctx, AV_LOG_INFO, "\n");
> + return 0;
> + }
> // convert to integer
> for (i = 0; i < pan->nb_output_channels; i++) {
> for (j = 0; j < pan->nb_input_channels; j++) {
> @@ -261,8 +324,37 @@ static int config_props(AVFilterLink *link)
> return 0;
> }
>
> +static void filter_samples_channel_mapping(AVFilterLink *inlink, AVFilterBufferRef *insamples)
> +{
> + int n = insamples->audio->nb_samples;
> + PanContext *pan = inlink->dst->priv;
> + AVFilterLink * const outlink = inlink->dst->outputs[0];
The " * " is not coherent with the rest of the style.
> + AVFilterBufferRef *outsamples = avfilter_get_audio_buffer(outlink, AV_PERM_WRITE, n);
> + AVFilterBufferRefAudioProps *in = insamples->audio;
> + AVFilterBufferRefAudioProps *out = outsamples->audio;
>
> -static void filter_samples(AVFilterLink *inlink, AVFilterBufferRef *insamples)
> + if (!pan->sample_rate || pan->sample_rate != in->sample_rate) {
Can you explain why it is necessary? In other words, is the sample rate
supposed to be able to change? And if so, do we really need to take it into
account: surely, for pure remapping, the sample rate is not relevant?
> + pan->sample_rate = in->sample_rate;
> + pan->swr = swr_alloc_set_opts(pan->swr,
> + out->channel_layout, outsamples->format, pan->sample_rate,
> + in ->channel_layout, insamples ->format, pan->sample_rate,
> + 0, 0);
> + if (!pan->swr)
> + return;
If swr_alloc_set_opts fails, this stops the filter for that round, but for
the next round, pan->sample_rate will have been set, and the code will skip
to the call to swr_convert with pan->swr null.
Simple fix: set pan->sample_rate only after everything has succeeded.
Better fix: allocate pan->swr in config_props, and return an explicit error
if it fails.
> + av_opt_set_int(pan->swr, "icl", pan->out_channel_layout, 0);
> + av_opt_set_int(pan->swr, "uch", pan->nb_output_channels, 0);
> + swr_set_channel_mapping(pan->swr, pan->ch);
> + if (swr_init(pan->swr) < 0)
> + return;
Same problem here of course.
> + }
> +
> + swr_convert(pan->swr, outsamples->data, n, insamples->data, n);
> +
> + avfilter_filter_samples(outlink, outsamples);
> + avfilter_unref_buffer(insamples);
> +}
> +
> +static void filter_samples_panning(AVFilterLink *inlink, AVFilterBufferRef *insamples)
> {
> PanContext *const pan = inlink->dst->priv;
> int i, o, n = insamples->audio->nb_samples;
> @@ -289,6 +381,12 @@ static void filter_samples(AVFilterLink *inlink, AVFilterBufferRef *insamples)
> avfilter_unref_buffer(insamples);
> }
>
> +static void filter_samples(AVFilterLink *inlink, AVFilterBufferRef *insamples)
> +{
> + PanContext * const pan = inlink->dst->priv;
" * " again.
> + pan->filter_samples(inlink, insamples);
> +}
> +
It's hard to see in the diff, but I believe some of the code could be
factored out of filter_samples_*: probably, at least,
avfilter_get_audio_buffer, avfilter_filter_samples, avfilter_unref_buffer.
> AVFilter avfilter_af_pan = {
> .name = "pan",
> .description = NULL_IF_CONFIG_SMALL("Remix channels with coefficients (panning)"),
Did you not forget some kind of swr_free?
Regards,
--
Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120118/a61f66c2/attachment.asc>
More information about the ffmpeg-devel
mailing list