[FFmpeg-devel] [PATCH v2 3/4] avutil/channel_layout: do not copy alloc new map for extra channel layout

[Marton Balint]

On Tue, 15 Mar 2022, James Almer wrote:

> On 3/15/2022 6:18 PM, Marton Balint wrote:
>>  Also use av_channel_layout_bprint directly for describing channel layout
>>  for
>>  extra channels.
>>
>>  Signed-off-by: Marton Balint <cus at passwd.hu>
>>  ---
>>    libavutil/channel_layout.c | 17 ++++-------------
>>    1 file changed, 4 insertions(+), 13 deletions(-)
>>
>>  diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
>>  index 0069c6257b..fb1f72737f 100644
>>  --- a/libavutil/channel_layout.c
>>  +++ b/libavutil/channel_layout.c
>>  @@ -704,29 +704,20 @@ static int try_describe_ambisonic(AVBPrint *bp,
>>  const AVChannelLayout *channel_l
>>        nb_ambi_channels = (order + 1) * (order + 1);
>>        if (nb_ambi_channels < channel_layout->nb_channels) {
>>            AVChannelLayout extra = { 0 };
>>  -        char buf[128];
>>
>>            if (channel_layout->order == AV_CHANNEL_ORDER_AMBISONIC) {
>>                extra.order       = AV_CHANNEL_ORDER_NATIVE;
>>                extra.nb_channels = av_popcount64(channel_layout->u.mask);
>>                extra.u.mask      = channel_layout->u.mask;
>>            } else {
>>  -            const AVChannelCustom *map = channel_layout->u.map;
>>  -
>>                extra.order       = AV_CHANNEL_ORDER_CUSTOM;
>>                extra.nb_channels = channel_layout->nb_channels -
>>                nb_ambi_channels;
>>  -            extra.u.map       = av_calloc(extra.nb_channels,
>>  sizeof(*extra.u.map));
>>  -            if (!extra.u.map)
>>  -                return AVERROR(ENOMEM);
>>  -
>>  -            memcpy(extra.u.map, &map[nb_ambi_channels],
>>  -                   sizeof(*extra.u.map) * extra.nb_channels);
>>  +            extra.u.map       = channel_layout->u.map + nb_ambi_channels;
>>            }
>>    -        av_channel_layout_describe(&extra, buf, sizeof(buf));
>>  -        av_channel_layout_uninit(&extra);
>>  -
>>  -        av_bprintf(bp, "+%s", buf);
>>  +        av_bprint_chars(bp, '+', 1);
>>  +        av_channel_layout_describe_bprint(&extra, bp);
>>  +        /* Not calling uninit here on extra because we don't own the
>>  u.map pointer */
>>        }
>>
>>        return 0;
>

> LGTM.

Thanks, applied the series (although in different order, because it turned 
out that this patch depends on the next).

Thanks,
Marton