On Tue, 15 Mar 2022, James Almer wrote:
On 3/15/2022 6:18 PM, Marton Balint wrote:Signed-off-by: Marton Balint <c...@passwd.hu> --- 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.
Ok.
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.
Ok, I have no idea why I added it to the header... :) Thanks, Marton _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
