| Message ID | 20200908211856.16290-10-andreas.rheinhardt@gmail.com |
|---|---|
| State | Accepted |
| Commit | 71daaafa3a7f0e3494b73055bdbb3fd8aa114173 |
| Headers | show |
| Series | [FFmpeg-devel,01/25] avfilter/af_headphone: Don't use uninitialized buffer in log message | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
On Tue, Sep 08, 2020 at 11:18:41PM +0200, Andreas Rheinhardt wrote: > 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.) > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > libavfilter/af_headphone.c | 31 +++++++++---------------------- > 1 file changed, 9 insertions(+), 22 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; > probably ok
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; - - 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); + uint64_t layout = av_get_channel_layout(arg); + + 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;
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.) Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- libavfilter/af_headphone.c | 31 +++++++++---------------------- 1 file changed, 9 insertions(+), 22 deletions(-)