From 71daaafa3a7f0e3494b73055bdbb3fd8aa114173 Mon Sep 17 00:00:00 2001 From: Andreas Rheinhardt Date: Thu, 27 Aug 2020 03:14:02 +0200 Subject: [PATCH] avfilter/af_headphone: Simplify parsing channel mapping string When parsing the channel mapping string (a string containing '|' delimited tokens each of which is supposed to contain a channel name like "FR"), the old code would at each step read up to seven uppercase characters from the input string and give this to av_get_channel_layout() to parse. The returned layout is then checked for being a layout with a single channel set by computing its logarithm. Besides being overtly complicated this also has the drawback of relying on the assumption that every channel name consists of at most seven uppercase letters only; but said assumption is wrong: The abbreviation of the second low frequency channel is LFE2. Furthermore it treats garbage like "FRfoo" as valid channel. This commit changes this by using av_get_channel_layout() directly; furthermore, av_get_channel_layout_nb_channels() (which uses popcount) is used to find out the number of channels instead of the custom code to calculate the logarithm. (As a consequence, certain other formats to specify the channel layouts are now accepted (like the hex versions of av_get_channel_layout()); but this is actually not bad at all.) Reviewed-by: Paul B Mahol Signed-off-by: Andreas Rheinhardt --- libavfilter/af_headphone.c | 29 ++++++++--------------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/libavfilter/af_headphone.c b/libavfilter/af_headphone.c index 8db712e9a0..32939af854 100644 --- a/libavfilter/af_headphone.c +++ b/libavfilter/af_headphone.c @@ -87,26 +87,14 @@ typedef struct HeadphoneContext { uint64_t mapping[64]; } HeadphoneContext; -static int parse_channel_name(char **arg, uint64_t *rchannel, char *buf) +static int parse_channel_name(const char *arg, uint64_t *rchannel) { - int len, i, channel_id = 0; - uint64_t layout, layout0; + uint64_t layout = av_get_channel_layout(arg); - if (sscanf(*arg, "%7[A-Z]%n", buf, &len)) { - layout0 = layout = av_get_channel_layout(buf); - for (i = 32; i > 0; i >>= 1) { - if (layout >= 1LL << i) { - channel_id += i; - layout >>= i; - } - } - if (channel_id >= 64 || layout0 != 1ULL << channel_id) - return AVERROR(EINVAL); - *rchannel = layout0; - *arg += len; - return 0; - } - return AVERROR(EINVAL); + if (av_get_channel_layout_nb_channels(layout) != 1) + return AVERROR(EINVAL); + *rchannel = layout; + return 0; } static void parse_map(AVFilterContext *ctx) @@ -124,15 +112,14 @@ static void parse_map(AVFilterContext *ctx) while ((arg = av_strtok(p, "|", &tokenizer))) { uint64_t out_channel; - char buf[8]; p = NULL; - if (parse_channel_name(&arg, &out_channel, buf)) { + if (parse_channel_name(arg, &out_channel)) { av_log(ctx, AV_LOG_WARNING, "Failed to parse \'%s\' as channel name.\n", arg); continue; } if (used_channels & out_channel) { - av_log(ctx, AV_LOG_WARNING, "Ignoring duplicate channel '%s'.\n", buf); + av_log(ctx, AV_LOG_WARNING, "Ignoring duplicate channel '%s'.\n", arg); continue; } used_channels |= out_channel;