* [FFmpeg-devel] [PATCH 2/4] avutil/channel_layout: factorize ambisonic order detection
2022-03-15 20:30 [FFmpeg-devel] [PATCH 1/4] avutil/channel_layout: print channels using av_channel_name_bprint in av_channel_layout_describe_bprint Marton Balint
@ 2022-03-15 20:30 ` Marton Balint
2022-03-15 20:49 ` James Almer
2022-03-15 20:30 ` [FFmpeg-devel] [PATCH 3/4] avutil/channel_layout: do not copy alloc new map for extra channel layout Marton Balint
` (2 subsequent siblings)
3 siblings, 1 reply; 17+ messages in thread
From: Marton Balint @ 2022-03-15 20:30 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: 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".
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [FFmpeg-devel] [PATCH 2/4] avutil/channel_layout: factorize ambisonic order detection
2022-03-15 20:30 ` [FFmpeg-devel] [PATCH 2/4] avutil/channel_layout: factorize ambisonic order detection Marton Balint
@ 2022-03-15 20:49 ` James Almer
0 siblings, 0 replies; 17+ messages in thread
From: James Almer @ 2022-03-15 20:49 UTC (permalink / raw)
To: ffmpeg-devel
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".
^ permalink raw reply [flat|nested] 17+ messages in thread
* [FFmpeg-devel] [PATCH 3/4] avutil/channel_layout: do not copy alloc new map for extra channel layout
2022-03-15 20:30 [FFmpeg-devel] [PATCH 1/4] avutil/channel_layout: print channels using av_channel_name_bprint in av_channel_layout_describe_bprint Marton Balint
2022-03-15 20:30 ` [FFmpeg-devel] [PATCH 2/4] avutil/channel_layout: factorize ambisonic order detection Marton Balint
@ 2022-03-15 20:30 ` Marton Balint
2022-03-15 20:39 ` James Almer
2022-03-15 20:30 ` [FFmpeg-devel] [PATCH 4/4] avutil/channel_layout: fix av_channel_layout_describe_bprint with custom and ambisonic channels Marton Balint
2022-03-15 20:44 ` [FFmpeg-devel] [PATCH 1/4] avutil/channel_layout: print channels using av_channel_name_bprint in av_channel_layout_describe_bprint James Almer
3 siblings, 1 reply; 17+ messages in thread
From: Marton Balint @ 2022-03-15 20:30 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: 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".
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [FFmpeg-devel] [PATCH 3/4] avutil/channel_layout: do not copy alloc new map for extra channel layout
2022-03-15 20:30 ` [FFmpeg-devel] [PATCH 3/4] avutil/channel_layout: do not copy alloc new map for extra channel layout Marton Balint
@ 2022-03-15 20:39 ` James Almer
0 siblings, 0 replies; 17+ messages in thread
From: James Almer @ 2022-03-15 20:39 UTC (permalink / raw)
To: ffmpeg-devel
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".
^ permalink raw reply [flat|nested] 17+ messages in thread
* [FFmpeg-devel] [PATCH 4/4] avutil/channel_layout: fix av_channel_layout_describe_bprint with custom and ambisonic channels
2022-03-15 20:30 [FFmpeg-devel] [PATCH 1/4] avutil/channel_layout: print channels using av_channel_name_bprint in av_channel_layout_describe_bprint Marton Balint
2022-03-15 20:30 ` [FFmpeg-devel] [PATCH 2/4] avutil/channel_layout: factorize ambisonic order detection Marton Balint
2022-03-15 20:30 ` [FFmpeg-devel] [PATCH 3/4] avutil/channel_layout: do not copy alloc new map for extra channel layout Marton Balint
@ 2022-03-15 20:30 ` Marton Balint
2022-03-15 20:42 ` James Almer
2022-03-15 20:44 ` [FFmpeg-devel] [PATCH 1/4] avutil/channel_layout: print channels using av_channel_name_bprint in av_channel_layout_describe_bprint James Almer
3 siblings, 1 reply; 17+ messages in thread
From: Marton Balint @ 2022-03-15 20:30 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: 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".
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [FFmpeg-devel] [PATCH 4/4] avutil/channel_layout: fix av_channel_layout_describe_bprint with custom and ambisonic channels
2022-03-15 20:30 ` [FFmpeg-devel] [PATCH 4/4] avutil/channel_layout: fix av_channel_layout_describe_bprint with custom and ambisonic channels Marton Balint
@ 2022-03-15 20:42 ` James Almer
0 siblings, 0 replies; 17+ messages in thread
From: James Almer @ 2022-03-15 20:42 UTC (permalink / raw)
To: ffmpeg-devel
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".
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/4] avutil/channel_layout: print channels using av_channel_name_bprint in av_channel_layout_describe_bprint
2022-03-15 20:30 [FFmpeg-devel] [PATCH 1/4] avutil/channel_layout: print channels using av_channel_name_bprint in av_channel_layout_describe_bprint Marton Balint
` (2 preceding siblings ...)
2022-03-15 20:30 ` [FFmpeg-devel] [PATCH 4/4] avutil/channel_layout: fix av_channel_layout_describe_bprint with custom and ambisonic channels Marton Balint
@ 2022-03-15 20:44 ` James Almer
2022-03-15 21:18 ` [FFmpeg-devel] [PATCH v2 " Marton Balint
3 siblings, 1 reply; 17+ messages in thread
From: James Almer @ 2022-03-15 20:44 UTC (permalink / raw)
To: ffmpeg-devel
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".
^ permalink raw reply [flat|nested] 17+ messages in thread* [FFmpeg-devel] [PATCH v2 1/4] avutil/channel_layout: print channels using av_channel_name_bprint in av_channel_layout_describe_bprint
2022-03-15 20:44 ` [FFmpeg-devel] [PATCH 1/4] avutil/channel_layout: print channels using av_channel_name_bprint in av_channel_layout_describe_bprint James Almer
@ 2022-03-15 21:18 ` Marton Balint
2022-03-15 21:18 ` [FFmpeg-devel] [PATCH v2 2/4] avutil/channel_layout: factorize ambisonic order detection Marton Balint
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: Marton Balint @ 2022-03-15 21:18 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: 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".
^ permalink raw reply [flat|nested] 17+ messages in thread* [FFmpeg-devel] [PATCH v2 2/4] avutil/channel_layout: factorize ambisonic order detection
2022-03-15 21:18 ` [FFmpeg-devel] [PATCH v2 " Marton Balint
@ 2022-03-15 21:18 ` Marton Balint
2022-03-15 21:24 ` James Almer
2022-03-15 21:18 ` [FFmpeg-devel] [PATCH v2 3/4] avutil/channel_layout: do not copy alloc new map for extra channel layout Marton Balint
2022-03-15 21:18 ` [FFmpeg-devel] [PATCH v2 4/4] avutil/channel_layout: fix av_channel_layout_describe_bprint with custom and ambisonic channels Marton Balint
2 siblings, 1 reply; 17+ messages in thread
From: Marton Balint @ 2022-03-15 21:18 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: 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".
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [FFmpeg-devel] [PATCH v2 2/4] avutil/channel_layout: factorize ambisonic order detection
2022-03-15 21:18 ` [FFmpeg-devel] [PATCH v2 2/4] avutil/channel_layout: factorize ambisonic order detection Marton Balint
@ 2022-03-15 21:24 ` James Almer
2022-03-15 21:31 ` Marton Balint
0 siblings, 1 reply; 17+ messages in thread
From: James Almer @ 2022-03-15 21:24 UTC (permalink / raw)
To: ffmpeg-devel
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".
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [FFmpeg-devel] [PATCH v2 2/4] avutil/channel_layout: factorize ambisonic order detection
2022-03-15 21:24 ` James Almer
@ 2022-03-15 21:31 ` Marton Balint
0 siblings, 0 replies; 17+ messages in thread
From: Marton Balint @ 2022-03-15 21:31 UTC (permalink / raw)
To: FFmpeg development discussions and patches
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".
^ permalink raw reply [flat|nested] 17+ messages in thread
* [FFmpeg-devel] [PATCH v2 3/4] avutil/channel_layout: do not copy alloc new map for extra channel layout
2022-03-15 21:18 ` [FFmpeg-devel] [PATCH v2 " Marton Balint
2022-03-15 21:18 ` [FFmpeg-devel] [PATCH v2 2/4] avutil/channel_layout: factorize ambisonic order detection Marton Balint
@ 2022-03-15 21:18 ` Marton Balint
2022-03-15 21:26 ` James Almer
2022-03-15 21:18 ` [FFmpeg-devel] [PATCH v2 4/4] avutil/channel_layout: fix av_channel_layout_describe_bprint with custom and ambisonic channels Marton Balint
2 siblings, 1 reply; 17+ messages in thread
From: Marton Balint @ 2022-03-15 21:18 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: 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".
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [FFmpeg-devel] [PATCH v2 3/4] avutil/channel_layout: do not copy alloc new map for extra channel layout
2022-03-15 21:18 ` [FFmpeg-devel] [PATCH v2 3/4] avutil/channel_layout: do not copy alloc new map for extra channel layout Marton Balint
@ 2022-03-15 21:26 ` James Almer
2022-03-15 22:28 ` Marton Balint
0 siblings, 1 reply; 17+ messages in thread
From: James Almer @ 2022-03-15 21:26 UTC (permalink / raw)
To: ffmpeg-devel
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".
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [FFmpeg-devel] [PATCH v2 3/4] avutil/channel_layout: do not copy alloc new map for extra channel layout
2022-03-15 21:26 ` James Almer
@ 2022-03-15 22:28 ` Marton Balint
0 siblings, 0 replies; 17+ messages in thread
From: Marton Balint @ 2022-03-15 22:28 UTC (permalink / raw)
To: FFmpeg development discussions and patches
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".
^ permalink raw reply [flat|nested] 17+ messages in thread
* [FFmpeg-devel] [PATCH v2 4/4] avutil/channel_layout: fix av_channel_layout_describe_bprint with custom and ambisonic channels
2022-03-15 21:18 ` [FFmpeg-devel] [PATCH v2 " Marton Balint
2022-03-15 21:18 ` [FFmpeg-devel] [PATCH v2 2/4] avutil/channel_layout: factorize ambisonic order detection Marton Balint
2022-03-15 21:18 ` [FFmpeg-devel] [PATCH v2 3/4] avutil/channel_layout: do not copy alloc new map for extra channel layout Marton Balint
@ 2022-03-15 21:18 ` Marton Balint
2022-03-15 21:28 ` James Almer
2 siblings, 1 reply; 17+ messages in thread
From: Marton Balint @ 2022-03-15 21:18 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: 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".
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [FFmpeg-devel] [PATCH v2 4/4] avutil/channel_layout: fix av_channel_layout_describe_bprint with custom and ambisonic channels
2022-03-15 21:18 ` [FFmpeg-devel] [PATCH v2 4/4] avutil/channel_layout: fix av_channel_layout_describe_bprint with custom and ambisonic channels Marton Balint
@ 2022-03-15 21:28 ` James Almer
0 siblings, 0 replies; 17+ messages in thread
From: James Almer @ 2022-03-15 21:28 UTC (permalink / raw)
To: ffmpeg-devel
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".
^ permalink raw reply [flat|nested] 17+ messages in thread