[FFmpeg-devel] [PATCH 1/4] avutil/channel_layout: print channels using av_channel_name_bprint in av_channel_layout_describe_bprint

[FFmpeg-devel] [PATCH 1/4] avutil/channel_layout: print channels using av_channel_name_bprint in av_channel_layout_describe_bprint

[Marton Balint]

This reduces code duplication an allows printing AMBI%d channel names for
custom layouts for non-standard or partial ambisonic layouts.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavutil/channel_layout.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
index 8cc4efe4cf..c60ccf368f 100644
--- a/libavutil/channel_layout.c
+++ b/libavutil/channel_layout.c
@@ -737,14 +737,10 @@ int av_channel_layout_describe_bprint(const AVChannelLayout *channel_layout,
             av_bprintf(bp, "%d channels (", channel_layout->nb_channels);
         for (i = 0; i < channel_layout->nb_channels; i++) {
             enum AVChannel ch = av_channel_layout_channel_from_index(channel_layout, i);
-            const char *channel = get_channel_name(ch);
 
             if (i)
                 av_bprintf(bp, "+");
-            if (channel)
-                av_bprintf(bp, "%s", channel);
-            else
-                av_bprintf(bp, "USR%d", ch);
+            av_channel_name_bprint(bp, ch);
             if (channel_layout->order == AV_CHANNEL_ORDER_CUSTOM &&
                 channel_layout->u.map[i].name[0])
                 av_bprintf(bp, "@%s", channel_layout->u.map[i].name);
-- 
2.31.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 2/4] avutil/channel_layout: factorize ambisonic order detection

[Marton Balint]

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavutil/channel_layout.c | 40 ++++++++++++++++++++++++++------------
 1 file changed, 28 insertions(+), 12 deletions(-)

diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
index c60ccf368f..fb2335a334 100644
--- a/libavutil/channel_layout.c
+++ b/libavutil/channel_layout.c
@@ -644,29 +644,29 @@ 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 if (channel_layout->order == AV_CHANNEL_ORDER_CUSTOM) {
         const AVChannelCustom *map = channel_layout->u.map;
         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 +674,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 +712,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);
         }
 
-- 
2.31.1

_______________________________________________
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".

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

[Marton Balint]

Also use av_channel_layout_bprint directly for describing channel layout for
extra channels.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavutil/channel_layout.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
index fb2335a334..8406089fe0 100644
--- a/libavutil/channel_layout.c
+++ b/libavutil/channel_layout.c
@@ -702,29 +702,19 @@ 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);
     }
 
     return 0;
-- 
2.31.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 4/4] avutil/channel_layout: fix av_channel_layout_describe_bprint with custom and ambisonic channels

[Marton Balint]

bp->len cannot be used to detect if try_describe_ambisonic was successful
because the bprint buffer might contain other data as well.

Also describing an invalid ambisonic layout should not return 0 but
AVERROR(EINVAL) instead, so change try_describe_ambisonic to actually return
error on invalid ambisonics. This also allows us to fix the first issue.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavutil/channel_layout.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
index 8406089fe0..be511dcf68 100644
--- a/libavutil/channel_layout.c
+++ b/libavutil/channel_layout.c
@@ -687,14 +687,14 @@ static int ambisonic_order(const AVChannelLayout *channel_layout)
 /**
  * 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.
+ * Return a negative error code otherwise.
  */
 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;
+        return order;
 
     av_bprintf(bp, "ambisonic %d", order);
 
@@ -736,7 +736,7 @@ int av_channel_layout_describe_bprint(const AVChannelLayout *channel_layout,
     case AV_CHANNEL_ORDER_CUSTOM:
         if (channel_layout->order == AV_CHANNEL_ORDER_CUSTOM) {
             int res = try_describe_ambisonic(bp, channel_layout);
-            if (res < 0 || bp->len)
+            if (res >= 0)
                 return res;
         }
         if (channel_layout->nb_channels)
-- 
2.31.1

_______________________________________________
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".

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

[James Almer]

On 3/15/2022 5:30 PM, Marton Balint wrote:
> Also use av_channel_layout_bprint directly for describing channel layout for
> extra channels.
> 
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>   libavutil/channel_layout.c | 16 +++-------------
>   1 file changed, 3 insertions(+), 13 deletions(-)
> 
> diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
> index fb2335a334..8406089fe0 100644
> --- a/libavutil/channel_layout.c
> +++ b/libavutil/channel_layout.c
> @@ -702,29 +702,19 @@ 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);

Maybe add a comment that you're not calling uninit on extra because it 
doesn't own its AVChannelMap pointer.

> -
> -        av_bprintf(bp, "+%s", buf);
> +        av_bprint_chars(bp, '+', 1);
> +        av_channel_layout_describe_bprint(&extra, bp);
>       }
>   
>       return 0;
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 4/4] avutil/channel_layout: fix av_channel_layout_describe_bprint with custom and ambisonic channels

[James Almer]

On 3/15/2022 5:30 PM, Marton Balint wrote:
> bp->len cannot be used to detect if try_describe_ambisonic was successful
> because the bprint buffer might contain other data as well.
> 
> Also describing an invalid ambisonic layout should not return 0 but
> AVERROR(EINVAL) instead, so change try_describe_ambisonic to actually return
> error on invalid ambisonics. This also allows us to fix the first issue.
> 
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>   libavutil/channel_layout.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
> index 8406089fe0..be511dcf68 100644
> --- a/libavutil/channel_layout.c
> +++ b/libavutil/channel_layout.c
> @@ -687,14 +687,14 @@ static int ambisonic_order(const AVChannelLayout *channel_layout)
>   /**
>    * 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.
> + * Return a negative error code otherwise.
>    */
>   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;
> +        return order;
>   
>       av_bprintf(bp, "ambisonic %d", order);
>   
> @@ -736,7 +736,7 @@ int av_channel_layout_describe_bprint(const AVChannelLayout *channel_layout,
>       case AV_CHANNEL_ORDER_CUSTOM:
>           if (channel_layout->order == AV_CHANNEL_ORDER_CUSTOM) {
>               int res = try_describe_ambisonic(bp, channel_layout);
> -            if (res < 0 || bp->len)
> +            if (res >= 0)
>                   return res;

return 0. The doxy for av_channel_layout_describe_bprint() does not 
define values > 0.

>           }
>           if (channel_layout->nb_channels)
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/4] avutil/channel_layout: print channels using av_channel_name_bprint in av_channel_layout_describe_bprint

[James Almer]

On 3/15/2022 5:30 PM, Marton Balint wrote:
> This reduces code duplication an allows printing AMBI%d channel names for
> custom layouts for non-standard or partial ambisonic layouts.
> 
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>   libavutil/channel_layout.c | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
> index 8cc4efe4cf..c60ccf368f 100644
> --- a/libavutil/channel_layout.c
> +++ b/libavutil/channel_layout.c
> @@ -737,14 +737,10 @@ int av_channel_layout_describe_bprint(const AVChannelLayout *channel_layout,
>               av_bprintf(bp, "%d channels (", channel_layout->nb_channels);
>           for (i = 0; i < channel_layout->nb_channels; i++) {
>               enum AVChannel ch = av_channel_layout_channel_from_index(channel_layout, i);
> -            const char *channel = get_channel_name(ch);
>   
>               if (i)
>                   av_bprintf(bp, "+");
> -            if (channel)
> -                av_bprintf(bp, "%s", channel);
> -            else
> -                av_bprintf(bp, "USR%d", ch);
> +            av_channel_name_bprint(bp, ch);
>               if (channel_layout->order == AV_CHANNEL_ORDER_CUSTOM &&
>                   channel_layout->u.map[i].name[0])
>                   av_bprintf(bp, "@%s", channel_layout->u.map[i].name);

Should be ok.
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 2/4] avutil/channel_layout: factorize ambisonic order detection

[James Almer]

On 3/15/2022 5:30 PM, Marton Balint wrote:
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>   libavutil/channel_layout.c | 40 ++++++++++++++++++++++++++------------
>   1 file changed, 28 insertions(+), 12 deletions(-)
> 
> diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
> index c60ccf368f..fb2335a334 100644
> --- a/libavutil/channel_layout.c
> +++ b/libavutil/channel_layout.c
> @@ -644,29 +644,29 @@ 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 if (channel_layout->order == AV_CHANNEL_ORDER_CUSTOM) {

It's not called for any other order, so this is not needed.

Maybe add an av_assert0() instead.

>           const AVChannelCustom *map = channel_layout->u.map;
>           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 +674,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 +712,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);
>           }

Should be ok.
_______________________________________________
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".

[FFmpeg-devel] [PATCH v2 1/4] avutil/channel_layout: print channels using av_channel_name_bprint in av_channel_layout_describe_bprint

[Marton Balint]

This reduces code duplication an allows printing AMBI%d channel names for
custom layouts for non-standard or partial ambisonic layouts.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavutil/channel_layout.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
index 8cc4efe4cf..c60ccf368f 100644
--- a/libavutil/channel_layout.c
+++ b/libavutil/channel_layout.c
@@ -737,14 +737,10 @@ int av_channel_layout_describe_bprint(const AVChannelLayout *channel_layout,
             av_bprintf(bp, "%d channels (", channel_layout->nb_channels);
         for (i = 0; i < channel_layout->nb_channels; i++) {
             enum AVChannel ch = av_channel_layout_channel_from_index(channel_layout, i);
-            const char *channel = get_channel_name(ch);
 
             if (i)
                 av_bprintf(bp, "+");
-            if (channel)
-                av_bprintf(bp, "%s", channel);
-            else
-                av_bprintf(bp, "USR%d", ch);
+            av_channel_name_bprint(bp, ch);
             if (channel_layout->order == AV_CHANNEL_ORDER_CUSTOM &&
                 channel_layout->u.map[i].name[0])
                 av_bprintf(bp, "@%s", channel_layout->u.map[i].name);
-- 
2.31.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH v2 2/4] avutil/channel_layout: factorize ambisonic order detection

[Marton Balint]

Signed-off-by: Marton Balint <cus@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 {
         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"
 
 /**
  * @file
-- 
2.31.1

_______________________________________________
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".

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

[Marton Balint]

Also use av_channel_layout_bprint directly for describing channel layout for
extra channels.

Signed-off-by: Marton Balint <cus@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;
-- 
2.31.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH v2 4/4] avutil/channel_layout: fix av_channel_layout_describe_bprint with custom and ambisonic channels

[Marton Balint]

bp->len cannot be used to detect if try_describe_ambisonic was successful
because the bprint buffer might contain other data as well.

Also describing an invalid ambisonic layout should not return 0 but
AVERROR(EINVAL) instead, so change try_describe_ambisonic to actually return
error on invalid ambisonics. This also allows us to fix the first issue.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavutil/channel_layout.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
index fb1f72737f..1a141d4a6a 100644
--- a/libavutil/channel_layout.c
+++ b/libavutil/channel_layout.c
@@ -689,14 +689,14 @@ static int ambisonic_order(const AVChannelLayout *channel_layout)
 /**
  * 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.
+ * Return a negative error code otherwise.
  */
 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;
+        return order;
 
     av_bprintf(bp, "ambisonic %d", order);
 
@@ -739,8 +739,8 @@ int av_channel_layout_describe_bprint(const AVChannelLayout *channel_layout,
     case AV_CHANNEL_ORDER_CUSTOM:
         if (channel_layout->order == AV_CHANNEL_ORDER_CUSTOM) {
             int res = try_describe_ambisonic(bp, channel_layout);
-            if (res < 0 || bp->len)
-                return res;
+            if (res >= 0)
+                return 0;
         }
         if (channel_layout->nb_channels)
             av_bprintf(bp, "%d channels (", channel_layout->nb_channels);
-- 
2.31.1

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2 2/4] avutil/channel_layout: factorize ambisonic order detection

[James Almer]

On 3/15/2022 6:18 PM, Marton Balint wrote:
> Signed-off-by: Marton Balint <cus@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.

>           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".

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

[James Almer]

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@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.
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2 4/4] avutil/channel_layout: fix av_channel_layout_describe_bprint with custom and ambisonic channels

[James Almer]

On 3/15/2022 6:18 PM, Marton Balint wrote:
> bp->len cannot be used to detect if try_describe_ambisonic was successful
> because the bprint buffer might contain other data as well.
> 
> Also describing an invalid ambisonic layout should not return 0 but
> AVERROR(EINVAL) instead, so change try_describe_ambisonic to actually return
> error on invalid ambisonics. This also allows us to fix the first issue.
> 
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>   libavutil/channel_layout.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
> index fb1f72737f..1a141d4a6a 100644
> --- a/libavutil/channel_layout.c
> +++ b/libavutil/channel_layout.c
> @@ -689,14 +689,14 @@ static int ambisonic_order(const AVChannelLayout *channel_layout)
>   /**
>    * 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.
> + * Return a negative error code otherwise.
>    */
>   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;
> +        return order;
>   
>       av_bprintf(bp, "ambisonic %d", order);
>   
> @@ -739,8 +739,8 @@ int av_channel_layout_describe_bprint(const AVChannelLayout *channel_layout,
>       case AV_CHANNEL_ORDER_CUSTOM:
>           if (channel_layout->order == AV_CHANNEL_ORDER_CUSTOM) {
>               int res = try_describe_ambisonic(bp, channel_layout);
> -            if (res < 0 || bp->len)
> -                return res;
> +            if (res >= 0)
> +                return 0;
>           }
>           if (channel_layout->nb_channels)
>               av_bprintf(bp, "%d channels (", channel_layout->nb_channels);

LGTM.
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2 2/4] avutil/channel_layout: factorize ambisonic order detection

[Marton Balint]

On Tue, 15 Mar 2022, James Almer wrote:

>
>
> On 3/15/2022 6:18 PM, Marton Balint wrote:
>>  Signed-off-by: Marton Balint <cus@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-request@ffmpeg.org with subject "unsubscribe".

Re: [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@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
_______________________________________________
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".