From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id E75B342419 for ; Tue, 15 Mar 2022 21:24:47 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 49ADE68AEC2; Tue, 15 Mar 2022 23:24:45 +0200 (EET) Received: from mail-oo1-f50.google.com (mail-oo1-f50.google.com [209.85.161.50]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 7248268A4CD for ; Tue, 15 Mar 2022 23:24:39 +0200 (EET) Received: by mail-oo1-f50.google.com with SMTP id w3-20020a4ac183000000b0031d806bbd7eso486705oop.13 for ; Tue, 15 Mar 2022 14:24:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:date:mime-version:user-agent:subject:content-language:to :references:from:in-reply-to:content-transfer-encoding; bh=M0CkfyW+4amtEjWaTlY5lGrvWYrZdfrZ9kdinmbnL/Q=; b=MHS4xs2WUpO7xkrA6uYQ6a7pE/haRWji7oWhRIH1vZBIigl0Sz9sp7FKhE+k7vMgj8 vE5IgykOYASHTn7IG6Sp76EYbaKI9+hHQcLdwZsA9fxjPPM7i5yfVnRk8bwTC5Yw+gKu YhCN3x+ZU7u42N56NTPhDaPvwg8JS/evGg+MyrnuYNXjq9YYnQaM7cT/BrKxLBYFB/1m 1H2Kj4UBScERHo8MM1d00uA7FBJu/yd5a1m5aKxFx11CulTKeJTQhRygNSH0itr7kdRL OhRQy1aXkJPWsmnxCY57vYXHbiL/VcPDpgzyqJEb941Qp4o8yOlQJewYUJi/ZoWqNmeO PRUA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:references:from:in-reply-to :content-transfer-encoding; bh=M0CkfyW+4amtEjWaTlY5lGrvWYrZdfrZ9kdinmbnL/Q=; b=MOg6SNpPranzOWz7F6SPeAalsB2uLAcM5gB9LC5d05nCMmKu+G/7khuIBygEkroE7j AG/oe4sLyXi9sYBkfJW4W4GBnIAhLspnQGcwxVAVX97632bz8rlo8HXHnH/x3q1xHyyb 2yt32+SqWB40N/oitUxcf2FG+ADZfWRHKt45LV9vUe6ff4+W9hmZD/N+ecOfPOHxYFH6 ioUqxMGWyAXlW49yY70Fgvu43zHLDXlD17siiu8gmgJ9MS+Q2bEhF7cX4L3MvS5FACEk OzYUeAppJ9U3Klr2eztzNDo6oD5rE5UiLddzebfy/Bl4PmE0APYa1C1KbrGIsIgUwoI0 4SIg== X-Gm-Message-State: AOAM530goHJBxtC0SD1NsdBupA2fm5K8asyYr/0wnhHlwnhuDQA7hSu3 zct5gympq8052P4EbNG/faXyet38/ZWUsw== X-Google-Smtp-Source: ABdhPJxOxhHr6gYqXK3GP9Dw4T9tB3XsBwrvYigHKYj4JNbicCZ5qoRv0qijN7bOz6qdb1eBYTBHKA== X-Received: by 2002:a05:6870:42cc:b0:dd:a03f:195e with SMTP id z12-20020a05687042cc00b000dda03f195emr753902oah.234.1647379477643; Tue, 15 Mar 2022 14:24:37 -0700 (PDT) Received: from [192.168.0.13] ([186.136.131.95]) by smtp.gmail.com with ESMTPSA id 125-20020a4a1483000000b003175bffb677sm114647ood.2.2022.03.15.14.24.36 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 15 Mar 2022 14:24:36 -0700 (PDT) Message-ID: Date: Tue, 15 Mar 2022 18:24:35 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: <1382f5bd-673f-f429-0ce9-ce39c3f9b586@gmail.com> <20220315211858.26800-1-cus@passwd.hu> <20220315211858.26800-2-cus@passwd.hu> From: James Almer In-Reply-To: <20220315211858.26800-2-cus@passwd.hu> Subject: Re: [FFmpeg-devel] [PATCH v2 2/4] avutil/channel_layout: factorize ambisonic order detection X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On 3/15/2022 6:18 PM, Marton Balint wrote: > Signed-off-by: Marton Balint > --- > libavutil/channel_layout.c | 42 +++++++++++++++++++++++++++----------- > libavutil/channel_layout.h | 1 + > 2 files changed, 31 insertions(+), 12 deletions(-) > > diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c > index c60ccf368f..0069c6257b 100644 > --- a/libavutil/channel_layout.c > +++ b/libavutil/channel_layout.c > @@ -644,29 +644,31 @@ int av_channel_layout_copy(AVChannelLayout *dst, const AVChannelLayout *src) > } > > /** > - * If the custom layout is n-th order standard-order ambisonic, with optional > - * extra non-diegetic channels at the end, write its string description in bp. > - * Return a negative error code on error. > + * If the layout is n-th order standard-order ambisonic, with optional > + * extra non-diegetic channels at the end, return the order. > + * Return a negative error code otherwise. > */ > -static int try_describe_ambisonic(AVBPrint *bp, const AVChannelLayout *channel_layout) > +static int ambisonic_order(const AVChannelLayout *channel_layout) > { > int i, highest_ambi, order; > > highest_ambi = -1; > - if (channel_layout->order == AV_CHANNEL_ORDER_AMBISONIC) > + if (channel_layout->order == AV_CHANNEL_ORDER_AMBISONIC) { > highest_ambi = channel_layout->nb_channels - av_popcount64(channel_layout->u.mask) - 1; > - else { > + } else { nit: Remove these brackets since it's still a single line after the if statement. They bloat the diff for no gain. > const AVChannelCustom *map = channel_layout->u.map; > + av_assert0(channel_layout->order == AV_CHANNEL_ORDER_CUSTOM); > + > for (i = 0; i < channel_layout->nb_channels; i++) { > int is_ambi = CHAN_IS_AMBI(map[i].id); > > /* ambisonic following non-ambisonic */ > if (i > 0 && is_ambi && !CHAN_IS_AMBI(map[i - 1].id)) > - return 0; > + return AVERROR(EINVAL); > > /* non-default ordering */ > if (is_ambi && map[i].id - AV_CHAN_AMBISONIC_BASE != i) > - return 0; > + return AVERROR(EINVAL); > > if (CHAN_IS_AMBI(map[i].id)) > highest_ambi = i; > @@ -674,17 +676,33 @@ static int try_describe_ambisonic(AVBPrint *bp, const AVChannelLayout *channel_l > } > /* no ambisonic channels*/ > if (highest_ambi < 0) > - return 0; > + return AVERROR(EINVAL); > > order = floor(sqrt(highest_ambi)); > /* incomplete order - some harmonics are missing */ > if ((order + 1) * (order + 1) != highest_ambi + 1) > + return AVERROR(EINVAL); > + > + return order; > +} > + > +/** > + * If the custom layout is n-th order standard-order ambisonic, with optional > + * extra non-diegetic channels at the end, write its string description in bp. > + * Return a negative error code on error. > + */ > +static int try_describe_ambisonic(AVBPrint *bp, const AVChannelLayout *channel_layout) > +{ > + int nb_ambi_channels; > + int order = ambisonic_order(channel_layout); > + if (order < 0) > return 0; > > av_bprintf(bp, "ambisonic %d", order); > > /* extra channels present */ > - if (highest_ambi < channel_layout->nb_channels - 1) { > + nb_ambi_channels = (order + 1) * (order + 1); > + if (nb_ambi_channels < channel_layout->nb_channels) { > AVChannelLayout extra = { 0 }; > char buf[128]; > > @@ -696,12 +714,12 @@ static int try_describe_ambisonic(AVBPrint *bp, const AVChannelLayout *channel_l > const AVChannelCustom *map = channel_layout->u.map; > > extra.order = AV_CHANNEL_ORDER_CUSTOM; > - extra.nb_channels = channel_layout->nb_channels - highest_ambi - 1; > + 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[highest_ambi + 1], > + memcpy(extra.u.map, &map[nb_ambi_channels], > sizeof(*extra.u.map) * extra.nb_channels); > } > > diff --git a/libavutil/channel_layout.h b/libavutil/channel_layout.h > index 4dd6614de9..294e8b0773 100644 > --- a/libavutil/channel_layout.h > +++ b/libavutil/channel_layout.h > @@ -27,6 +27,7 @@ > > #include "version.h" > #include "attributes.h" > +#include "avassert.h" Nothing in the header uses it. It'll become an unnecessary dependency every user of this header will have to deal with, so add it to channel_layout.c instead. > > /** > * @file LGTM with the above two changes. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".