[FFmpeg-devel] [PATCH 1/5] avformat/mov_chan: do not assume channels are in native order

[FFmpeg-devel] [PATCH 1/5] avformat/mov_chan: do not assume channels are in native order

[Marton Balint]

Existing code could have caused wrong channel order signalling or reduced
channel count if a channel designation appeared multiple times. This is
actually an old bug, but the conversion to the new channel layout API made it
visible, because now the code overrides the proper channel count with the one
calculated from the mask.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavformat/mov_chan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/mov_chan.c b/libavformat/mov_chan.c
index f3d51899e1..5cb2de3820 100644
--- a/libavformat/mov_chan.c
+++ b/libavformat/mov_chan.c
@@ -530,7 +530,7 @@ int ff_mov_read_chan(AVFormatContext *s, AVIOContext *pb, AVStream *st,
         size -= 20;
         if (layout_tag == 0) {
             uint64_t mask_incr = mov_get_channel_mask(label);
-            if (mask_incr == 0) {
+            if (mask_incr == 0 || mask_incr <= label_mask) {
                 label_mask = 0;
                 break;
             }
-- 
2.35.3

_______________________________________________
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/5] avformat/mov_chan: never override number of channels based on chan atom

[Marton Balint]

The channel designation metadata should not override the number of channels.
Let's warn the user if it is inconsistent, and keep the channel layout
unspecified.

Before the conversion to the channel layout API the code only set the mask, but
never overridden the channel count, so this restores the old behaviour.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavformat/mov_chan.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/libavformat/mov_chan.c b/libavformat/mov_chan.c
index 5cb2de3820..6b206745b4 100644
--- a/libavformat/mov_chan.c
+++ b/libavformat/mov_chan.c
@@ -544,8 +544,13 @@ int ff_mov_read_chan(AVFormatContext *s, AVIOContext *pb, AVStream *st,
         mask = mov_get_channel_layout(layout_tag, bitmap);
 
     if (mask) {
-        av_channel_layout_uninit(&st->codecpar->ch_layout);
-        av_channel_layout_from_mask(&st->codecpar->ch_layout, mask);
+        if (!st->codecpar->ch_layout.nb_channels || av_popcount64(mask) == st->codecpar->ch_layout.nb_channels) {
+            av_channel_layout_uninit(&st->codecpar->ch_layout);
+            av_channel_layout_from_mask(&st->codecpar->ch_layout, mask);
+        } else {
+            av_log(s, AV_LOG_WARNING, "ignoring channel layout with %d channels because the real number of channels is %d\n",
+                   av_popcount64(mask), st->codecpar->ch_layout.nb_channels);
+        }
     }
     avio_skip(pb, size - 12);
 
-- 
2.35.3

_______________________________________________
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/5] avutil/channel_layout: add av_channel_layout_from_custom()

[Marton Balint]

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 doc/APIchanges             |  3 +++
 libavutil/channel_layout.c | 20 ++++++++++++++++++++
 libavutil/channel_layout.h | 13 +++++++++++++
 libavutil/version.h        |  4 ++--
 4 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index e477ed78e0..8e8498f803 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -2,6 +2,9 @@ The last version increases of all libraries were on 2023-02-09
 
 API changes, most recent first:
 
+2024-02-xx - xxxxxxxxxx - lavu 58.37.100 - channel_layout.h
+  Add av_channel_layout_from_custom().
+
 2023-11-xx - xxxxxxxxxx - lavfi 9.16.100 - buffersink.h buffersrc.h
   Add av_buffersink_get_colorspace and av_buffersink_get_color_range.
   Add AVBufferSrcParameters.color_space and AVBufferSrcParameters.color_range.
diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
index b59d798f29..0810d32bf6 100644
--- a/libavutil/channel_layout.c
+++ b/libavutil/channel_layout.c
@@ -398,6 +398,26 @@ int av_get_standard_channel_layout(unsigned index, uint64_t *layout,
 FF_ENABLE_DEPRECATION_WARNINGS
 #endif
 
+int av_channel_layout_from_custom(AVChannelLayout *channel_layout, int nb_channels)
+{
+    AVChannelCustom *map;
+
+    if (nb_channels <= 0)
+        return AVERROR(EINVAL);
+
+    map = av_calloc(nb_channels, sizeof(*channel_layout->u.map));
+    if (!map)
+        return AVERROR(ENOMEM);
+    for (int i = 0; i < nb_channels; i++)
+        map[i].id = AV_CHAN_UNKNOWN;
+
+    channel_layout->order       = AV_CHANNEL_ORDER_CUSTOM;
+    channel_layout->nb_channels = nb_channels;
+    channel_layout->u.map       = map;
+
+    return 0;
+}
+
 int av_channel_layout_from_mask(AVChannelLayout *channel_layout,
                                 uint64_t mask)
 {
diff --git a/libavutil/channel_layout.h b/libavutil/channel_layout.h
index 8dc1a91401..37629ab5d2 100644
--- a/libavutil/channel_layout.h
+++ b/libavutil/channel_layout.h
@@ -617,6 +617,19 @@ void av_channel_description_bprint(struct AVBPrint *bp, enum AVChannel channel_i
  */
 enum AVChannel av_channel_from_string(const char *name);
 
+/**
+ * Initialize a custom channel layout with the specified number of channels.
+ * The designation of all channels will be unknown.
+ *
+ * @param channel_layout the layout structure to be initialized
+ * @param nb_channels the number of channels
+ *
+ * @return 0 on success
+ *         AVERROR(EINVAL) if the number of channels <= 0
+ *         AVERROR(ENOMEM) if the channel map could not be allocated
+ */
+int av_channel_layout_from_custom(AVChannelLayout *channel_layout, int nb_channels);
+
 /**
  * Initialize a native channel layout from a bitmask indicating which channels
  * are present.
diff --git a/libavutil/version.h b/libavutil/version.h
index 772c4e209c..3b38f8f5da 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,8 +79,8 @@
  */
 
 #define LIBAVUTIL_VERSION_MAJOR  58
-#define LIBAVUTIL_VERSION_MINOR  36
-#define LIBAVUTIL_VERSION_MICRO 101
+#define LIBAVUTIL_VERSION_MINOR  37
+#define LIBAVUTIL_VERSION_MICRO 100
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
                                                LIBAVUTIL_VERSION_MINOR, \
-- 
2.35.3

_______________________________________________
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/5] avutil/channel_layout: add av_channel_layout_from_custom()

[Anton Khirnov]

Quoting Marton Balint (2024-01-30 00:27:53)
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  doc/APIchanges             |  3 +++
>  libavutil/channel_layout.c | 20 ++++++++++++++++++++
>  libavutil/channel_layout.h | 13 +++++++++++++
>  libavutil/version.h        |  4 ++--
>  4 files changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index e477ed78e0..8e8498f803 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2023-02-09
>  
>  API changes, most recent first:
>  
> +2024-02-xx - xxxxxxxxxx - lavu 58.37.100 - channel_layout.h
> +  Add av_channel_layout_from_custom().

The name seems misleading to me, there is no 'custom' from which a
channel layout is derived.

It should be something like av_channel_layout_custom_init() instead.

> +/**
> + * Initialize a custom channel layout with the specified number of channels.
> + * The designation of all channels will be unknown.
> + *
> + * @param channel_layout the layout structure to be initialized
> + * @param nb_channels the number of channels
> + *
> + * @return 0 on success
> + *         AVERROR(EINVAL) if the number of channels <= 0
> + *         AVERROR(ENOMEM) if the channel map could not be allocated
> + */

It should say that this is a convenience helper, so people don't think
this is the only way to build custom layouts.
It should also explicitly say it allocates the channel map array.

-- 
Anton Khirnov
_______________________________________________
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/5] avutil/channel_layout: add av_channel_layout_from_custom()

[Marton Balint]

On Thu, 1 Feb 2024, Anton Khirnov wrote:

> Quoting Marton Balint (2024-01-30 00:27:53)
>> Signed-off-by: Marton Balint <cus@passwd.hu>
>> ---
>>  doc/APIchanges             |  3 +++
>>  libavutil/channel_layout.c | 20 ++++++++++++++++++++
>>  libavutil/channel_layout.h | 13 +++++++++++++
>>  libavutil/version.h        |  4 ++--
>>  4 files changed, 38 insertions(+), 2 deletions(-)
>>
>> diff --git a/doc/APIchanges b/doc/APIchanges
>> index e477ed78e0..8e8498f803 100644
>> --- a/doc/APIchanges
>> +++ b/doc/APIchanges
>> @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2023-02-09
>>
>>  API changes, most recent first:
>>
>> +2024-02-xx - xxxxxxxxxx - lavu 58.37.100 - channel_layout.h
>> +  Add av_channel_layout_from_custom().
>
> The name seems misleading to me, there is no 'custom' from which a
> channel layout is derived.
>
> It should be something like av_channel_layout_custom_init() instead.

The other initializer functions follow the av_channel_layout_from_* 
pattern. But I can rename of course if that is preferred.

>
>> +/**
>> + * Initialize a custom channel layout with the specified number of channels.
>> + * The designation of all channels will be unknown.
>> + *
>> + * @param channel_layout the layout structure to be initialized
>> + * @param nb_channels the number of channels
>> + *
>> + * @return 0 on success
>> + *         AVERROR(EINVAL) if the number of channels <= 0
>> + *         AVERROR(ENOMEM) if the channel map could not be allocated
>> + */
>
> It should say that this is a convenience helper, so people don't think
> this is the only way to build custom layouts.
> It should also explicitly say it allocates the channel map array.

Ok, will send v2.

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 3/5] avutil/channel_layout: add av_channel_layout_from_custom()

[Anton Khirnov]

Quoting Marton Balint (2024-02-01 21:01:37)
> 
> 
> On Thu, 1 Feb 2024, Anton Khirnov wrote:
> 
> > Quoting Marton Balint (2024-01-30 00:27:53)
> >> Signed-off-by: Marton Balint <cus@passwd.hu>
> >> ---
> >>  doc/APIchanges             |  3 +++
> >>  libavutil/channel_layout.c | 20 ++++++++++++++++++++
> >>  libavutil/channel_layout.h | 13 +++++++++++++
> >>  libavutil/version.h        |  4 ++--
> >>  4 files changed, 38 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/doc/APIchanges b/doc/APIchanges
> >> index e477ed78e0..8e8498f803 100644
> >> --- a/doc/APIchanges
> >> +++ b/doc/APIchanges
> >> @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2023-02-09
> >>
> >>  API changes, most recent first:
> >>
> >> +2024-02-xx - xxxxxxxxxx - lavu 58.37.100 - channel_layout.h
> >> +  Add av_channel_layout_from_custom().
> >
> > The name seems misleading to me, there is no 'custom' from which a
> > channel layout is derived.
> >
> > It should be something like av_channel_layout_custom_init() instead.
> 
> The other initializer functions follow the av_channel_layout_from_* 
> pattern. But I can rename of course if that is preferred.

All av_foo_from_x() take x and build foo from it, but this function does
not.

-- 
Anton Khirnov
_______________________________________________
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/5] avutil/channel_layout: add av_channel_layout_retype()

[Marton Balint]

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 doc/APIchanges             |  3 +++
 libavutil/channel_layout.c | 48 ++++++++++++++++++++++++++++++++++++++
 libavutil/channel_layout.h | 11 +++++++++
 libavutil/version.h        |  2 +-
 4 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index 8e8498f803..ce1e816fa5 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -2,6 +2,9 @@ The last version increases of all libraries were on 2023-02-09
 
 API changes, most recent first:
 
+2024-02-xx - xxxxxxxxxx - lavu 58.38.100 - channel_layout.h
+  Add av_channel_layout_retype().
+
 2024-02-xx - xxxxxxxxxx - lavu 58.37.100 - channel_layout.h
   Add av_channel_layout_from_custom().
 
diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
index 0810d32bf6..cc8c208080 100644
--- a/libavutil/channel_layout.c
+++ b/libavutil/channel_layout.c
@@ -1036,3 +1036,51 @@ uint64_t av_channel_layout_subset(const AVChannelLayout *channel_layout,
 
     return ret;
 }
+
+int av_channel_layout_retype(AVChannelLayout *channel_layout, enum AVChannelOrder order)
+{
+    if (!av_channel_layout_check(channel_layout))
+        return AVERROR(EINVAL);
+
+    if (channel_layout->order == order)
+        return 0;
+
+    switch (order) {
+    case AV_CHANNEL_ORDER_UNSPEC: {
+        int nb_channels = channel_layout->nb_channels;
+        av_channel_layout_uninit(channel_layout);
+        channel_layout->order       = AV_CHANNEL_ORDER_UNSPEC;
+        channel_layout->nb_channels = nb_channels;
+        return 0;
+        }
+    case AV_CHANNEL_ORDER_CUSTOM: {
+        AVChannelLayout custom = { 0 };
+        int ret = av_channel_layout_from_custom(&custom, channel_layout->nb_channels);
+        if (ret < 0)
+            return ret;
+        if (channel_layout->order != AV_CHANNEL_ORDER_UNSPEC)
+            for (int i = 0; i < channel_layout->nb_channels; i++)
+                custom.u.map[i].id = av_channel_layout_channel_from_index(channel_layout, i);
+        av_channel_layout_uninit(channel_layout);
+        *channel_layout = custom;
+        return 0;
+        }
+    case AV_CHANNEL_ORDER_NATIVE:
+        if (channel_layout->order == AV_CHANNEL_ORDER_CUSTOM) {
+            uint64_t mask = 0;
+            for (int i = 0; i < channel_layout->nb_channels; i++) {
+                enum AVChannel ch = channel_layout->u.map[i].id;
+                if (ch >= 0 && ch < 63 && mask < (1ULL << ch))
+                    mask |= (1ULL << ch);
+                else
+                    return 1;
+            }
+            av_channel_layout_uninit(channel_layout);
+            return av_channel_layout_from_mask(channel_layout, mask);
+        } else {
+            return 1;
+        }
+    default:
+        return 1;
+    }
+}
diff --git a/libavutil/channel_layout.h b/libavutil/channel_layout.h
index 37629ab5d2..7e27a00d39 100644
--- a/libavutil/channel_layout.h
+++ b/libavutil/channel_layout.h
@@ -817,6 +817,17 @@ int av_channel_layout_check(const AVChannelLayout *channel_layout);
  */
 int av_channel_layout_compare(const AVChannelLayout *chl, const AVChannelLayout *chl1);
 
+/**
+ * Try changing the AVChannelOrder of a channel layout.
+ *
+ * @param channel_layout channel layout which will be changed
+ * @param order the desired channel layout order
+ * @return 0 on success or if the channel layout is already in the desired order
+ *         1 if using the desired order is not possible for the specified layout
+ *         AVERROR code on error
+ */
+int av_channel_layout_retype(AVChannelLayout *channel_layout, enum AVChannelOrder order);
+
 /**
  * @}
  */
diff --git a/libavutil/version.h b/libavutil/version.h
index 3b38f8f5da..cebf4a0acd 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@
  */
 
 #define LIBAVUTIL_VERSION_MAJOR  58
-#define LIBAVUTIL_VERSION_MINOR  37
+#define LIBAVUTIL_VERSION_MINOR  38
 #define LIBAVUTIL_VERSION_MICRO 100
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
-- 
2.35.3

_______________________________________________
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/5] avutil/channel_layout: add av_channel_layout_retype()

[Marton Balint]

v2: add conversion from custom layout to ambisonic

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 doc/APIchanges             |  3 ++
 libavutil/channel_layout.c | 74 ++++++++++++++++++++++++++++++++++++++
 libavutil/channel_layout.h | 11 ++++++
 libavutil/version.h        |  2 +-
 4 files changed, 89 insertions(+), 1 deletion(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index 8e8498f803..ce1e816fa5 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -2,6 +2,9 @@ The last version increases of all libraries were on 2023-02-09
 
 API changes, most recent first:
 
+2024-02-xx - xxxxxxxxxx - lavu 58.38.100 - channel_layout.h
+  Add av_channel_layout_retype().
+
 2024-02-xx - xxxxxxxxxx - lavu 58.37.100 - channel_layout.h
   Add av_channel_layout_from_custom().
 
diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
index 0810d32bf6..056b7f7c10 100644
--- a/libavutil/channel_layout.c
+++ b/libavutil/channel_layout.c
@@ -1036,3 +1036,77 @@ uint64_t av_channel_layout_subset(const AVChannelLayout *channel_layout,
 
     return ret;
 }
+
+static int64_t masked_description(AVChannelLayout *channel_layout, int start_channel)
+{
+    uint64_t mask = 0;
+    for (int i = start_channel; i < channel_layout->nb_channels; i++) {
+        enum AVChannel ch = channel_layout->u.map[i].id;
+        if (ch >= 0 && ch < 63 && mask < (1ULL << ch))
+            mask |= (1ULL << ch);
+        else
+            return AVERROR(EINVAL);
+    }
+    return mask;
+}
+
+int av_channel_layout_retype(AVChannelLayout *channel_layout, enum AVChannelOrder order)
+{
+    if (!av_channel_layout_check(channel_layout))
+        return AVERROR(EINVAL);
+
+    if (channel_layout->order == order)
+        return 0;
+
+    switch (order) {
+    case AV_CHANNEL_ORDER_UNSPEC: {
+        int nb_channels = channel_layout->nb_channels;
+        av_channel_layout_uninit(channel_layout);
+        channel_layout->order       = AV_CHANNEL_ORDER_UNSPEC;
+        channel_layout->nb_channels = nb_channels;
+        return 0;
+        }
+    case AV_CHANNEL_ORDER_CUSTOM: {
+        AVChannelLayout custom = { 0 };
+        int ret = av_channel_layout_from_custom(&custom, channel_layout->nb_channels);
+        if (ret < 0)
+            return ret;
+        if (channel_layout->order != AV_CHANNEL_ORDER_UNSPEC)
+            for (int i = 0; i < channel_layout->nb_channels; i++)
+                custom.u.map[i].id = av_channel_layout_channel_from_index(channel_layout, i);
+        av_channel_layout_uninit(channel_layout);
+        *channel_layout = custom;
+        return 0;
+        }
+    case AV_CHANNEL_ORDER_NATIVE:
+        if (channel_layout->order == AV_CHANNEL_ORDER_CUSTOM) {
+            int64_t mask = masked_description(channel_layout, 0);
+            if (mask < 0)
+                return 1;
+            av_channel_layout_uninit(channel_layout);
+            return av_channel_layout_from_mask(channel_layout, mask);
+        } else {
+            return 1;
+        }
+    case AV_CHANNEL_ORDER_AMBISONIC:
+        if (channel_layout->order == AV_CHANNEL_ORDER_CUSTOM) {
+            int64_t mask;
+            int nb_channels = channel_layout->nb_channels;
+            int order = ambisonic_order(channel_layout);
+            if (order < 0)
+                return 1;
+            mask = masked_description(channel_layout, (order + 1) * (order + 1));
+            if (mask < 0)
+                return 1;
+            av_channel_layout_uninit(channel_layout);
+            channel_layout->order       = AV_CHANNEL_ORDER_AMBISONIC;
+            channel_layout->nb_channels = nb_channels;
+            channel_layout->u.mask      = mask;
+            return 0;
+        } else {
+            return 1;
+        }
+    default:
+        return 1;
+    }
+}
diff --git a/libavutil/channel_layout.h b/libavutil/channel_layout.h
index 37629ab5d2..7e27a00d39 100644
--- a/libavutil/channel_layout.h
+++ b/libavutil/channel_layout.h
@@ -817,6 +817,17 @@ int av_channel_layout_check(const AVChannelLayout *channel_layout);
  */
 int av_channel_layout_compare(const AVChannelLayout *chl, const AVChannelLayout *chl1);
 
+/**
+ * Try changing the AVChannelOrder of a channel layout.
+ *
+ * @param channel_layout channel layout which will be changed
+ * @param order the desired channel layout order
+ * @return 0 on success or if the channel layout is already in the desired order
+ *         1 if using the desired order is not possible for the specified layout
+ *         AVERROR code on error
+ */
+int av_channel_layout_retype(AVChannelLayout *channel_layout, enum AVChannelOrder order);
+
 /**
  * @}
  */
diff --git a/libavutil/version.h b/libavutil/version.h
index 3b38f8f5da..cebf4a0acd 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@
  */
 
 #define LIBAVUTIL_VERSION_MAJOR  58
-#define LIBAVUTIL_VERSION_MINOR  37
+#define LIBAVUTIL_VERSION_MINOR  38
 #define LIBAVUTIL_VERSION_MICRO 100
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
-- 
2.35.3

_______________________________________________
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/5] avutil/channel_layout: add av_channel_layout_retype()

[Anton Khirnov]

Quoting Marton Balint (2024-02-01 00:01:36)
> diff --git a/libavutil/channel_layout.h b/libavutil/channel_layout.h
> index 37629ab5d2..7e27a00d39 100644
> --- a/libavutil/channel_layout.h
> +++ b/libavutil/channel_layout.h
> @@ -817,6 +817,17 @@ int av_channel_layout_check(const AVChannelLayout *channel_layout);
>   */
>  int av_channel_layout_compare(const AVChannelLayout *chl, const AVChannelLayout *chl1);
>  
> +/**
> + * Try changing the AVChannelOrder of a channel layout.

What exactly is the rule for when the change succeeds or not? I would
expect it to be when all the channels can be represented in the new
order, but that is not the case for conversion to unspec.

> + *
> + * @param channel_layout channel layout which will be changed
> + * @param order the desired channel layout order
> + * @return 0 on success or if the channel layout is already in the desired order
> + *         1 if using the desired order is not possible for the specified layout

AVERROR(ENOSYS) seems more consistent to me

-- 
Anton Khirnov
_______________________________________________
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/5] avutil/channel_layout: add av_channel_layout_retype()

[Marton Balint]

On Thu, 1 Feb 2024, Anton Khirnov wrote:

> Quoting Marton Balint (2024-02-01 00:01:36)
>> diff --git a/libavutil/channel_layout.h b/libavutil/channel_layout.h
>> index 37629ab5d2..7e27a00d39 100644
>> --- a/libavutil/channel_layout.h
>> +++ b/libavutil/channel_layout.h
>> @@ -817,6 +817,17 @@ int av_channel_layout_check(const AVChannelLayout *channel_layout);
>>   */
>>  int av_channel_layout_compare(const AVChannelLayout *chl, const AVChannelLayout *chl1);
>>
>> +/**
>> + * Try changing the AVChannelOrder of a channel layout.
>
> What exactly is the rule for when the change succeeds or not? I would
> expect it to be when all the channels can be represented in the new
> order, but that is not the case for conversion to unspec.

Yes, you are right. Converting to unspec indeed makes you lose the 
channel designations, so the conversion will not be lossless. On the other 
hand, when you specify UNSPEC as a target, you don't actually expect to 
keep the designations, so what is the point of returning an error...

I think this is one of those cases when both behaviour (always doing the 
conversion, or returning a failure in case the source order is not already 
unspec) can make sense. We have to decide though if a custom layout with 
all channels as UNKNOWN can be losslessly converted to UNSPEC layout or 
not. And if yes, then would not that conflict with 
av_channel_layout_channel_from_index() which returns AV_CHAN_NONE and not 
AV_CHAN_UNKNOWN for UNSPEC layouts...

So if you have a preference, I can change this (and document it), I don't 
particularly feel strongly about any of the two approaches. Probably there 
is no bad choce as long as it is properly documented.

>
>> + *
>> + * @param channel_layout channel layout which will be changed
>> + * @param order the desired channel layout order
>> + * @return 0 on success or if the channel layout is already in the desired order
>> + *         1 if using the desired order is not possible for the specified layout
>
> AVERROR(ENOSYS) seems more consistent to me

By using a positive result all negative return values can be considered 
serious errors which have to be propagated back to the user.

In the next patch I try to simplify a custom channel layout:

+        ret = av_channel_layout_retype(ch_layout, AV_CHANNEL_ORDER_NATIVE);
+        if (ret < 0)
+            goto out;

I can do simply this, because I don't care if the simplification was 
successful.

Regards,
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 4/5] avutil/channel_layout: add av_channel_layout_retype()

[Anton Khirnov]

Quoting Marton Balint (2024-02-01 21:36:31)
> > What exactly is the rule for when the change succeeds or not? I would
> > expect it to be when all the channels can be represented in the new
> > order, but that is not the case for conversion to unspec.
> 
> Yes, you are right. Converting to unspec indeed makes you lose the 
> channel designations, so the conversion will not be lossless. On the other 
> hand, when you specify UNSPEC as a target, you don't actually expect to 
> keep the designations, so what is the point of returning an error...
> 
> I think this is one of those cases when both behaviour (always doing the 
> conversion, or returning a failure in case the source order is not already 
> unspec) can make sense. We have to decide though if a custom layout with 
> all channels as UNKNOWN can be losslessly converted to UNSPEC layout or 
> not. And if yes, then would not that conflict with 
> av_channel_layout_channel_from_index() which returns AV_CHAN_NONE and not 
> AV_CHAN_UNKNOWN for UNSPEC layouts...

Huh, that might actually considered a bug, returning UNKNOWN certainly
makes more sense to me.

> >
> >> + *
> >> + * @param channel_layout channel layout which will be changed
> >> + * @param order the desired channel layout order
> >> + * @return 0 on success or if the channel layout is already in the desired order
> >> + *         1 if using the desired order is not possible for the specified layout
> >
> > AVERROR(ENOSYS) seems more consistent to me
> 
> By using a positive result all negative return values can be considered 
> serious errors which have to be propagated back to the user.
> 
> In the next patch I try to simplify a custom channel layout:
> 
> +        ret = av_channel_layout_retype(ch_layout, AV_CHANNEL_ORDER_NATIVE);
> +        if (ret < 0)
> +            goto out;
> 
> I can do simply this, because I don't care if the simplification was 
> successful.

IMO policy like what errors are to be considered serious should be up to
the caller. If you asked it to get a certain order and it failed to
deliver, then I'd consider that an error state.

-- 
Anton Khirnov
_______________________________________________
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/5] avutil/channel_layout: add av_channel_layout_retype()

[James Almer]

On 1/29/2024 8:27 PM, Marton Balint wrote:
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>   doc/APIchanges             |  3 +++
>   libavutil/channel_layout.c | 48 ++++++++++++++++++++++++++++++++++++++
>   libavutil/channel_layout.h | 11 +++++++++
>   libavutil/version.h        |  2 +-
>   4 files changed, 63 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 8e8498f803..ce1e816fa5 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2023-02-09
>   
>   API changes, most recent first:
>   
> +2024-02-xx - xxxxxxxxxx - lavu 58.38.100 - channel_layout.h
> +  Add av_channel_layout_retype().
> +
>   2024-02-xx - xxxxxxxxxx - lavu 58.37.100 - channel_layout.h
>     Add av_channel_layout_from_custom().
>   
> diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
> index 0810d32bf6..cc8c208080 100644
> --- a/libavutil/channel_layout.c
> +++ b/libavutil/channel_layout.c
> @@ -1036,3 +1036,51 @@ uint64_t av_channel_layout_subset(const AVChannelLayout *channel_layout,
>   
>       return ret;
>   }
> +
> +int av_channel_layout_retype(AVChannelLayout *channel_layout, enum AVChannelOrder order)
> +{
> +    if (!av_channel_layout_check(channel_layout))
> +        return AVERROR(EINVAL);
> +
> +    if (channel_layout->order == order)
> +        return 0;
> +
> +    switch (order) {
> +    case AV_CHANNEL_ORDER_UNSPEC: {
> +        int nb_channels = channel_layout->nb_channels;
> +        av_channel_layout_uninit(channel_layout);
> +        channel_layout->order       = AV_CHANNEL_ORDER_UNSPEC;
> +        channel_layout->nb_channels = nb_channels;
> +        return 0;
> +        }
> +    case AV_CHANNEL_ORDER_CUSTOM: {
> +        AVChannelLayout custom = { 0 };
> +        int ret = av_channel_layout_from_custom(&custom, channel_layout->nb_channels);
> +        if (ret < 0)
> +            return ret;
> +        if (channel_layout->order != AV_CHANNEL_ORDER_UNSPEC)
> +            for (int i = 0; i < channel_layout->nb_channels; i++)
> +                custom.u.map[i].id = av_channel_layout_channel_from_index(channel_layout, i);
> +        av_channel_layout_uninit(channel_layout);
> +        *channel_layout = custom;
> +        return 0;
> +        }
> +    case AV_CHANNEL_ORDER_NATIVE:
> +        if (channel_layout->order == AV_CHANNEL_ORDER_CUSTOM) {
> +            uint64_t mask = 0;
> +            for (int i = 0; i < channel_layout->nb_channels; i++) {
> +                enum AVChannel ch = channel_layout->u.map[i].id;
> +                if (ch >= 0 && ch < 63 && mask < (1ULL << ch))
> +                    mask |= (1ULL << ch);
> +                else
> +                    return 1;
> +            }
> +            av_channel_layout_uninit(channel_layout);
> +            return av_channel_layout_from_mask(channel_layout, mask);
> +        } else {
> +            return 1;
> +        }
> +    default:
> +        return 1;
> +    }
> +}

I wrote a function like this some time ago, but i lost the patch by 
accident during a migration.

The way i approached it was making the return codes reflect if the 
conversion was lossless or not, as in, custom -> native is lossless only 
if channels have no custom names (and possible only if the ids are 
within UINT64_MAX and in order). Anything to Unspec is always lossy, etc.

Custom -> Ambi: Possible only if it contains ambi channels. Lossless.
Custom -> Native: Possible only if has no ambi channels and all ids < 64 
and in order. Lossy if it has custom names, otherwise lossless.
Ambi -> Custom: Lossless.
Ambi -> Native: Not possible.
Native -> Custom: Lossless.
Native -> Ambi: Not possible.
Any -> Unspec: Possible but lossy.

So 0 for lossless, 1 for lossy, ENOSYS for not possible.
_______________________________________________
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/5] avutil/channel_layout: add av_channel_layout_retype()

[Anton Khirnov]

Quoting James Almer (2024-02-02 13:56:31)
> I wrote a function like this some time ago, but i lost the patch by 
> accident during a migration.
> 
> The way i approached it was making the return codes reflect if the 
> conversion was lossless or not, as in, custom -> native is lossless only 
> if channels have no custom names (and possible only if the ids are 
> within UINT64_MAX and in order).

> Anything to Unspec is always lossy, etc.

Unless the source is custom with every channel unknown.

> Custom -> Ambi: Possible only if it contains ambi channels. Lossless.
> Custom -> Native: Possible only if has no ambi channels and all ids < 64 
> and in order. Lossy if it has custom names, otherwise lossless.
> Ambi -> Custom: Lossless.
> Ambi -> Native: Not possible.
> Native -> Custom: Lossless.
> Native -> Ambi: Not possible.
> Any -> Unspec: Possible but lossy.
> 
> So 0 for lossless, 1 for lossy, ENOSYS for not possible.

That sounds good to me.

Might also have a flags argument that forces lossless, and for future
extensions.

-- 
Anton Khirnov
_______________________________________________
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/5] avutil/channel_layout: add av_channel_layout_retype()

[James Almer]

On 2/3/2024 7:38 AM, Anton Khirnov wrote:
> Quoting James Almer (2024-02-02 13:56:31)
>> I wrote a function like this some time ago, but i lost the patch by
>> accident during a migration.
>>
>> The way i approached it was making the return codes reflect if the
>> conversion was lossless or not, as in, custom -> native is lossless only
>> if channels have no custom names (and possible only if the ids are
>> within UINT64_MAX and in order).
> 
>> Anything to Unspec is always lossy, etc.
> 
> Unless the source is custom with every channel unknown.

True, and Unspec -> Custom is possible by doing the inverse.

> 
>> Custom -> Ambi: Possible only if it contains ambi channels. Lossless.
>> Custom -> Native: Possible only if has no ambi channels and all ids < 64
>> and in order. Lossy if it has custom names, otherwise lossless.
>> Ambi -> Custom: Lossless.
>> Ambi -> Native: Not possible.
>> Native -> Custom: Lossless.
>> Native -> Ambi: Not possible.
>> Any -> Unspec: Possible but lossy.
>>
>> So 0 for lossless, 1 for lossy, ENOSYS for not possible.
> 
> That sounds good to me.
> 
> Might also have a flags argument that forces lossless, and for future
> extensions.

Agree.
_______________________________________________
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/5] avutil/channel_layout: add av_channel_layout_custom_init()

[Marton Balint]

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 doc/APIchanges             |  3 +++
 libavutil/channel_layout.c | 20 ++++++++++++++++++++
 libavutil/channel_layout.h | 17 +++++++++++++++++
 libavutil/version.h        |  4 ++--
 4 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index 1f5724324a..cdb9b6a458 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -2,6 +2,9 @@ The last version increases of all libraries were on 2023-02-09
 
 API changes, most recent first:
 
+2024-02-xx - xxxxxxxxxx - lavu 58.37.100 - channel_layout.h
+  Add av_channel_layout_custom_init().
+
 2024-02-04 - xxxxxxxxxx - lavc 60.39.100 - packet.h
   Add AV_PKT_DATA_AMBIENT_VIEWING_ENVIRONMENT.
 
diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
index b59d798f29..40e31e9d12 100644
--- a/libavutil/channel_layout.c
+++ b/libavutil/channel_layout.c
@@ -398,6 +398,26 @@ int av_get_standard_channel_layout(unsigned index, uint64_t *layout,
 FF_ENABLE_DEPRECATION_WARNINGS
 #endif
 
+int av_channel_layout_custom_init(AVChannelLayout *channel_layout, int nb_channels)
+{
+    AVChannelCustom *map;
+
+    if (nb_channels <= 0)
+        return AVERROR(EINVAL);
+
+    map = av_calloc(nb_channels, sizeof(*channel_layout->u.map));
+    if (!map)
+        return AVERROR(ENOMEM);
+    for (int i = 0; i < nb_channels; i++)
+        map[i].id = AV_CHAN_UNKNOWN;
+
+    channel_layout->order       = AV_CHANNEL_ORDER_CUSTOM;
+    channel_layout->nb_channels = nb_channels;
+    channel_layout->u.map       = map;
+
+    return 0;
+}
+
 int av_channel_layout_from_mask(AVChannelLayout *channel_layout,
                                 uint64_t mask)
 {
diff --git a/libavutil/channel_layout.h b/libavutil/channel_layout.h
index 8dc1a91401..dcc320cbfe 100644
--- a/libavutil/channel_layout.h
+++ b/libavutil/channel_layout.h
@@ -617,6 +617,23 @@ void av_channel_description_bprint(struct AVBPrint *bp, enum AVChannel channel_i
  */
 enum AVChannel av_channel_from_string(const char *name);
 
+/**
+ * Initialize a custom channel layout with the specified number of channels.
+ * The channel map will be allocated and the designation of all channels will
+ * be set to AV_CHAN_UNKNOWN.
+ *
+ * This is only a convenience helper function, a custom channel layout can also
+ * be constructed without using this.
+ *
+ * @param channel_layout the layout structure to be initialized
+ * @param nb_channels the number of channels
+ *
+ * @return 0 on success
+ *         AVERROR(EINVAL) if the number of channels <= 0
+ *         AVERROR(ENOMEM) if the channel map could not be allocated
+ */
+int av_channel_layout_custom_init(AVChannelLayout *channel_layout, int nb_channels);
+
 /**
  * Initialize a native channel layout from a bitmask indicating which channels
  * are present.
diff --git a/libavutil/version.h b/libavutil/version.h
index 772c4e209c..3b38f8f5da 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,8 +79,8 @@
  */
 
 #define LIBAVUTIL_VERSION_MAJOR  58
-#define LIBAVUTIL_VERSION_MINOR  36
-#define LIBAVUTIL_VERSION_MICRO 101
+#define LIBAVUTIL_VERSION_MINOR  37
+#define LIBAVUTIL_VERSION_MICRO 100
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
                                                LIBAVUTIL_VERSION_MINOR, \
-- 
2.35.3

_______________________________________________
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/5] avutil/channel_layout: add av_channel_layout_retype()

[Marton Balint]

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 doc/APIchanges             |   3 ++
 libavutil/channel_layout.c | 106 +++++++++++++++++++++++++++++++++++++
 libavutil/channel_layout.h |  40 ++++++++++++++
 libavutil/version.h        |   2 +-
 4 files changed, 150 insertions(+), 1 deletion(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index cdb9b6a458..221fea30c2 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -2,6 +2,9 @@ The last version increases of all libraries were on 2023-02-09
 
 API changes, most recent first:
 
+2024-02-xx - xxxxxxxxxx - lavu 58.38.100 - channel_layout.h
+  Add av_channel_layout_retype().
+
 2024-02-xx - xxxxxxxxxx - lavu 58.37.100 - channel_layout.h
   Add av_channel_layout_custom_init().
 
diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
index 40e31e9d12..7f51c076dc 100644
--- a/libavutil/channel_layout.c
+++ b/libavutil/channel_layout.c
@@ -1036,3 +1036,109 @@ uint64_t av_channel_layout_subset(const AVChannelLayout *channel_layout,
 
     return ret;
 }
+
+static int64_t masked_description(AVChannelLayout *channel_layout, int start_channel)
+{
+    uint64_t mask = 0;
+    for (int i = start_channel; i < channel_layout->nb_channels; i++) {
+        enum AVChannel ch = channel_layout->u.map[i].id;
+        if (ch >= 0 && ch < 63 && mask < (1ULL << ch))
+            mask |= (1ULL << ch);
+        else
+            return AVERROR(EINVAL);
+    }
+    return mask;
+}
+
+static int has_channel_names(AVChannelLayout *channel_layout)
+{
+    if (channel_layout->order != AV_CHANNEL_ORDER_CUSTOM)
+        return 0;
+    for (int i = 0; i < channel_layout->nb_channels; i++)
+        if (channel_layout->u.map[i].name[0])
+            return 1;
+    return 0;
+}
+
+int av_channel_layout_retype(AVChannelLayout *channel_layout, enum AVChannelOrder order, int flags)
+{
+    int allow_lossy = !(flags & AV_CHANNEL_LAYOUT_RETYPE_FLAG_LOSSLESS);
+    int lossy;
+
+    if (!av_channel_layout_check(channel_layout))
+        return AVERROR(EINVAL);
+
+    if (channel_layout->order == order)
+        return 0;
+
+    switch (order) {
+    case AV_CHANNEL_ORDER_UNSPEC: {
+        int nb_channels = channel_layout->nb_channels;
+        if (channel_layout->order == AV_CHANNEL_ORDER_CUSTOM) {
+            lossy = 0;
+            for (int i = 0; i < nb_channels; i++) {
+                if (channel_layout->u.map[i].id != AV_CHAN_UNKNOWN || channel_layout->u.map[i].name[0]) {
+                    lossy = 1;
+                    break;
+                }
+            }
+        } else {
+            lossy = 1;
+        }
+        if (!lossy || allow_lossy) {
+            av_channel_layout_uninit(channel_layout);
+            channel_layout->order       = AV_CHANNEL_ORDER_UNSPEC;
+            channel_layout->nb_channels = nb_channels;
+            return lossy;
+        }
+        return AVERROR(ENOSYS);
+        }
+    case AV_CHANNEL_ORDER_NATIVE:
+        if (channel_layout->order == AV_CHANNEL_ORDER_CUSTOM) {
+            int64_t mask = masked_description(channel_layout, 0);
+            if (mask < 0)
+                return AVERROR(ENOSYS);
+            lossy = has_channel_names(channel_layout);
+            if (!lossy || allow_lossy) {
+                av_channel_layout_uninit(channel_layout);
+                av_channel_layout_from_mask(channel_layout, mask);
+                return lossy;
+            }
+        }
+        return AVERROR(ENOSYS);
+    case AV_CHANNEL_ORDER_CUSTOM: {
+        AVChannelLayout custom = { 0 };
+        int ret = av_channel_layout_custom_init(&custom, channel_layout->nb_channels);
+        if (ret < 0)
+            return ret;
+        if (channel_layout->order != AV_CHANNEL_ORDER_UNSPEC)
+            for (int i = 0; i < channel_layout->nb_channels; i++)
+                custom.u.map[i].id = av_channel_layout_channel_from_index(channel_layout, i);
+        av_channel_layout_uninit(channel_layout);
+        *channel_layout = custom;
+        return 0;
+        }
+    case AV_CHANNEL_ORDER_AMBISONIC:
+        if (channel_layout->order == AV_CHANNEL_ORDER_CUSTOM) {
+            int64_t mask;
+            int nb_channels = channel_layout->nb_channels;
+            int order = ambisonic_order(channel_layout);
+            if (order < 0)
+                return AVERROR(ENOSYS);
+            mask = masked_description(channel_layout, (order + 1) * (order + 1));
+            if (mask < 0)
+                return AVERROR(ENOSYS);
+            lossy = has_channel_names(channel_layout);
+            if (!lossy || allow_lossy) {
+                av_channel_layout_uninit(channel_layout);
+                channel_layout->order       = AV_CHANNEL_ORDER_AMBISONIC;
+                channel_layout->nb_channels = nb_channels;
+                channel_layout->u.mask      = mask;
+                return lossy;
+            }
+        }
+        return AVERROR(ENOSYS);
+    default:
+        return AVERROR(EINVAL);
+    }
+}
diff --git a/libavutil/channel_layout.h b/libavutil/channel_layout.h
index dcc320cbfe..10260d3ec5 100644
--- a/libavutil/channel_layout.h
+++ b/libavutil/channel_layout.h
@@ -821,6 +821,46 @@ int av_channel_layout_check(const AVChannelLayout *channel_layout);
  */
 int av_channel_layout_compare(const AVChannelLayout *chl, const AVChannelLayout *chl1);
 
+/**
+ * The conversion must be lossless.
+ */
+#define AV_CHANNEL_LAYOUT_RETYPE_FLAG_LOSSLESS (1 << 0)
+
+/**
+ * Change the AVChannelOrder of a channel layout.
+ *
+ * Change of AVChannelOrder can be either lossless or lossy. In case of a
+ * lossless conversion all the channel designations and the associated channel
+ * names (if any) are kept. On a lossy conversion the channel names and channel
+ * designations might be lost depending on the capabilities of the desired
+ * AVChannelOrder. Note that some conversions are simply not possible in which
+ * case this function returns AVERROR(ENOSYS).
+ *
+ * The following conversions are supported:
+ *
+ * Any       -> Custom     : Always possible, always lossless.
+ * Any       -> Unspecified: Always possible, lossless if channel designations
+ *   are all unknown and channel names are not used, lossy otherwise.
+ * Custom    -> Ambisonic  : Possible if it contains ambisonic channels with
+ *   optional non-diegetic channels in the end. Lossy if the channels have
+ *   custom names, lossless otherwise.
+ * Custom    -> Native     : Possible if it contains native channels in native
+ *     order. Lossy if the channels have custom names, lossless otherwise.
+ *
+ * On error this function keeps the original channel layout untouched.
+ *
+ * @param channel_layout channel layout which will be changed
+ * @param order the desired channel layout order
+ * @param flags a combination of AV_CHANNEL_LAYOUT_RETYPE_FLAG_* constants
+ * @return 0 if the conversion was successful and lossless or if the channel
+ *           layout was already in the desired order
+ *         1 if the conversion was successful but lossy
+ *         AVERROR(ENOSYS) if the conversion was not possible (or would be
+ *           lossy and AV_CHANNEL_LAYOUT_RETYPE_FLAG_LOSSLESS was specified)
+ *         AVERROR(EINVAL), AVERROR(ENOMEM) on error
+ */
+int av_channel_layout_retype(AVChannelLayout *channel_layout, enum AVChannelOrder order, int flags);
+
 /**
  * @}
  */
diff --git a/libavutil/version.h b/libavutil/version.h
index 3b38f8f5da..cebf4a0acd 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@
  */
 
 #define LIBAVUTIL_VERSION_MAJOR  58
-#define LIBAVUTIL_VERSION_MINOR  37
+#define LIBAVUTIL_VERSION_MINOR  38
 #define LIBAVUTIL_VERSION_MICRO 100
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
-- 
2.35.3

_______________________________________________
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/5] avutil/channel_layout: add av_channel_layout_retype()

[Anton Khirnov]

Quoting Marton Balint (2024-02-04 20:28:11)
> +/**
> + * Change the AVChannelOrder of a channel layout.
> + *
> + * Change of AVChannelOrder can be either lossless or lossy. In case of a
> + * lossless conversion all the channel designations and the associated channel
> + * names (if any) are kept. On a lossy conversion the channel names and channel
> + * designations might be lost depending on the capabilities of the desired
> + * AVChannelOrder. Note that some conversions are simply not possible in which
> + * case this function returns AVERROR(ENOSYS).
> + *
> + * The following conversions are supported:
> + *
> + * Any       -> Custom     : Always possible, always lossless.
> + * Any       -> Unspecified: Always possible, lossless if channel designations
> + *   are all unknown and channel names are not used, lossy otherwise.
> + * Custom    -> Ambisonic  : Possible if it contains ambisonic channels with
> + *   optional non-diegetic channels in the end. Lossy if the channels have
> + *   custom names, lossless otherwise.
> + * Custom    -> Native     : Possible if it contains native channels in native
> + *     order. Lossy if the channels have custom names, lossless otherwise.
> + *
> + * On error this function keeps the original channel layout untouched.
> + *
> + * @param channel_layout channel layout which will be changed
> + * @param order the desired channel layout order
> + * @param flags a combination of AV_CHANNEL_LAYOUT_RETYPE_FLAG_* constants
> + * @return 0 if the conversion was successful and lossless or if the channel
> + *           layout was already in the desired order
> + *         1 if the conversion was successful but lossy

You could say 'positive number' instead of 1, which leaves us wiggle
room to use other numbers in the future in a backwards compatible way.

Looks ok otherwise.

-- 
Anton Khirnov
_______________________________________________
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/5] avutil/channel_layout: add av_channel_layout_retype()

[Marton Balint]

On Fri, 9 Feb 2024, Anton Khirnov wrote:

> Quoting Marton Balint (2024-02-04 20:28:11)
>> +/**
>> + * Change the AVChannelOrder of a channel layout.
>> + *
>> + * Change of AVChannelOrder can be either lossless or lossy. In case of a
>> + * lossless conversion all the channel designations and the associated channel
>> + * names (if any) are kept. On a lossy conversion the channel names and channel
>> + * designations might be lost depending on the capabilities of the desired
>> + * AVChannelOrder. Note that some conversions are simply not possible in which
>> + * case this function returns AVERROR(ENOSYS).
>> + *
>> + * The following conversions are supported:
>> + *
>> + * Any       -> Custom     : Always possible, always lossless.
>> + * Any       -> Unspecified: Always possible, lossless if channel designations
>> + *   are all unknown and channel names are not used, lossy otherwise.
>> + * Custom    -> Ambisonic  : Possible if it contains ambisonic channels with
>> + *   optional non-diegetic channels in the end. Lossy if the channels have
>> + *   custom names, lossless otherwise.
>> + * Custom    -> Native     : Possible if it contains native channels in native
>> + *     order. Lossy if the channels have custom names, lossless otherwise.
>> + *
>> + * On error this function keeps the original channel layout untouched.
>> + *
>> + * @param channel_layout channel layout which will be changed
>> + * @param order the desired channel layout order
>> + * @param flags a combination of AV_CHANNEL_LAYOUT_RETYPE_FLAG_* constants
>> + * @return 0 if the conversion was successful and lossless or if the channel
>> + *           layout was already in the desired order
>> + *         1 if the conversion was successful but lossy
>
> You could say 'positive number' instead of 1, which leaves us wiggle
> room to use other numbers in the future in a backwards compatible way.
>
> Looks ok otherwise.

Ok, thanks, will apply the series with that change.

Regards,
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 4/5] avutil/channel_layout: add av_channel_layout_retype()

[James Almer]

On 2/4/2024 4:28 PM, Marton Balint wrote:
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>   doc/APIchanges             |   3 ++
>   libavutil/channel_layout.c | 106 +++++++++++++++++++++++++++++++++++++
>   libavutil/channel_layout.h |  40 ++++++++++++++
>   libavutil/version.h        |   2 +-
>   4 files changed, 150 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index cdb9b6a458..221fea30c2 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2023-02-09
>   
>   API changes, most recent first:
>   
> +2024-02-xx - xxxxxxxxxx - lavu 58.38.100 - channel_layout.h
> +  Add av_channel_layout_retype().

Can you please add tests for it in tests/channel_layout.c?
_______________________________________________
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/5] avutil/channel_layout: add av_channel_layout_retype()

[Marton Balint]

On Sun, 11 Feb 2024, James Almer wrote:

> On 2/4/2024 4:28 PM, Marton Balint wrote:
>>  Signed-off-by: Marton Balint <cus@passwd.hu>
>>  ---
>>    doc/APIchanges             |   3 ++
>>    libavutil/channel_layout.c | 106 +++++++++++++++++++++++++++++++++++++
>>    libavutil/channel_layout.h |  40 ++++++++++++++
>>    libavutil/version.h        |   2 +-
>>    4 files changed, 150 insertions(+), 1 deletion(-)
>>
>>  diff --git a/doc/APIchanges b/doc/APIchanges
>>  index cdb9b6a458..221fea30c2 100644
>>  --- a/doc/APIchanges
>>  +++ b/doc/APIchanges
>>  @@ -2,6 +2,9 @@ The last version increases of all libraries were on
>>  2023-02-09
>>
>>    API changes, most recent first:
>>
>>  +2024-02-xx - xxxxxxxxxx - lavu 58.38.100 - channel_layout.h
>>  +  Add av_channel_layout_retype().
>
> Can you please add tests for it in tests/channel_layout.c?

Yes, will send a follow up series with tests once this series is merged.

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

[FFmpeg-devel] [PATCH v2 5/5] avformat/mov_chan: add support for reading custom channel layouts when layout_tag == 0

[Marton Balint]

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavformat/mov_chan.c | 99 +++++++++++++++++++++++-------------------
 1 file changed, 54 insertions(+), 45 deletions(-)

diff --git a/libavformat/mov_chan.c b/libavformat/mov_chan.c
index 6b206745b4..302639c332 100644
--- a/libavformat/mov_chan.c
+++ b/libavformat/mov_chan.c
@@ -377,23 +377,23 @@ static uint64_t mov_get_channel_layout(uint32_t tag, uint32_t bitmap)
     return layout_map[i].layout;
 }
 
-static uint64_t mov_get_channel_mask(uint32_t label)
+static enum AVChannel mov_get_channel_id(uint32_t label)
 {
     if (label == 0)
-        return 0;
+        return AV_CHAN_UNUSED;
     if (label <= 18)
-        return 1U << (label - 1);
+        return (label - 1);
     if (label == 35)
-        return AV_CH_WIDE_LEFT;
+        return AV_CHAN_WIDE_LEFT;
     if (label == 36)
-        return AV_CH_WIDE_RIGHT;
+        return AV_CHAN_WIDE_RIGHT;
     if (label == 37)
-        return AV_CH_LOW_FREQUENCY_2;
+        return AV_CHAN_LOW_FREQUENCY_2;
     if (label == 38)
-        return AV_CH_STEREO_LEFT;
+        return AV_CHAN_STEREO_LEFT;
     if (label == 39)
-        return AV_CH_STEREO_RIGHT;
-    return 0;
+        return AV_CHAN_STEREO_RIGHT;
+    return AV_CHAN_UNKNOWN;
 }
 
 static uint32_t mov_get_channel_label(enum AVChannel channel)
@@ -497,8 +497,8 @@ int ff_mov_read_chan(AVFormatContext *s, AVIOContext *pb, AVStream *st,
                      int64_t size)
 {
     uint32_t layout_tag, bitmap, num_descr;
-    uint64_t label_mask, mask = 0;
-    int i;
+    int ret;
+    AVChannelLayout *ch_layout = &st->codecpar->ch_layout;
 
     if (size < 12)
         return AVERROR_INVALIDDATA;
@@ -514,47 +514,56 @@ int ff_mov_read_chan(AVFormatContext *s, AVIOContext *pb, AVStream *st,
     if (size < 12ULL + num_descr * 20ULL)
         return 0;
 
-    label_mask = 0;
-    for (i = 0; i < num_descr; i++) {
-        uint32_t label;
-        if (pb->eof_reached) {
-            av_log(s, AV_LOG_ERROR,
-                   "reached EOF while reading channel layout\n");
-            return AVERROR_INVALIDDATA;
+    if (layout_tag == 0) {
+        int nb_channels = ch_layout->nb_channels ? ch_layout->nb_channels : num_descr;
+        if (num_descr > nb_channels) {
+            av_log(s, AV_LOG_WARNING, "got %d channel descriptions, capping to the number of channels %d\n",
+                   num_descr, nb_channels);
+            num_descr = nb_channels;
         }
-        label     = avio_rb32(pb);          // mChannelLabel
-        avio_rb32(pb);                      // mChannelFlags
-        avio_rl32(pb);                      // mCoordinates[0]
-        avio_rl32(pb);                      // mCoordinates[1]
-        avio_rl32(pb);                      // mCoordinates[2]
-        size -= 20;
-        if (layout_tag == 0) {
-            uint64_t mask_incr = mov_get_channel_mask(label);
-            if (mask_incr == 0 || mask_incr <= label_mask) {
-                label_mask = 0;
-                break;
+
+        av_channel_layout_uninit(ch_layout);
+        ret = av_channel_layout_custom_init(ch_layout, nb_channels);
+        if (ret < 0)
+            goto out;
+
+        for (int i = 0; i < num_descr; i++) {
+            uint32_t label;
+            if (pb->eof_reached) {
+                av_log(s, AV_LOG_ERROR,
+                       "reached EOF while reading channel layout\n");
+                return AVERROR_INVALIDDATA;
             }
-            label_mask |= mask_incr;
+            label     = avio_rb32(pb);          // mChannelLabel
+            avio_rb32(pb);                      // mChannelFlags
+            avio_rl32(pb);                      // mCoordinates[0]
+            avio_rl32(pb);                      // mCoordinates[1]
+            avio_rl32(pb);                      // mCoordinates[2]
+            size -= 20;
+            ch_layout->u.map[i].id = mov_get_channel_id(label);
         }
-    }
-    if (layout_tag == 0) {
-        if (label_mask)
-            mask = label_mask;
-    } else
-        mask = mov_get_channel_layout(layout_tag, bitmap);
-
-    if (mask) {
-        if (!st->codecpar->ch_layout.nb_channels || av_popcount64(mask) == st->codecpar->ch_layout.nb_channels) {
-            av_channel_layout_uninit(&st->codecpar->ch_layout);
-            av_channel_layout_from_mask(&st->codecpar->ch_layout, mask);
-        } else {
-            av_log(s, AV_LOG_WARNING, "ignoring channel layout with %d channels because the real number of channels is %d\n",
-                   av_popcount64(mask), st->codecpar->ch_layout.nb_channels);
+
+        ret = av_channel_layout_retype(ch_layout, AV_CHANNEL_ORDER_NATIVE, AV_CHANNEL_LAYOUT_RETYPE_FLAG_LOSSLESS);
+        if (ret < 0 && ret != AVERROR(ENOSYS))
+            goto out;
+    } else {
+        uint64_t mask = mov_get_channel_layout(layout_tag, bitmap);
+        if (mask) {
+            if (!ch_layout->nb_channels || av_popcount64(mask) == ch_layout->nb_channels) {
+                av_channel_layout_uninit(ch_layout);
+                av_channel_layout_from_mask(ch_layout, mask);
+            } else {
+                av_log(s, AV_LOG_WARNING, "ignoring channel layout with %d channels because number of channels is %d\n",
+                       av_popcount64(mask), ch_layout->nb_channels);
+            }
         }
     }
+    ret = 0;
+
+out:
     avio_skip(pb, size - 12);
 
-    return 0;
+    return ret;
 }
 
 /* ISO/IEC 23001-8, 8.2 */
-- 
2.35.3

_______________________________________________
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 v3 5/5] avformat/mov_chan: add support for reading custom channel layouts when layout_tag == 0

[Marton Balint]

v3: also try simplifying custom layout to unspecified layout

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavformat/mov_chan.c | 101 +++++++++++++++++++++++------------------
 1 file changed, 56 insertions(+), 45 deletions(-)

diff --git a/libavformat/mov_chan.c b/libavformat/mov_chan.c
index 6b206745b4..79768bc210 100644
--- a/libavformat/mov_chan.c
+++ b/libavformat/mov_chan.c
@@ -377,23 +377,23 @@ static uint64_t mov_get_channel_layout(uint32_t tag, uint32_t bitmap)
     return layout_map[i].layout;
 }
 
-static uint64_t mov_get_channel_mask(uint32_t label)
+static enum AVChannel mov_get_channel_id(uint32_t label)
 {
     if (label == 0)
-        return 0;
+        return AV_CHAN_UNUSED;
     if (label <= 18)
-        return 1U << (label - 1);
+        return (label - 1);
     if (label == 35)
-        return AV_CH_WIDE_LEFT;
+        return AV_CHAN_WIDE_LEFT;
     if (label == 36)
-        return AV_CH_WIDE_RIGHT;
+        return AV_CHAN_WIDE_RIGHT;
     if (label == 37)
-        return AV_CH_LOW_FREQUENCY_2;
+        return AV_CHAN_LOW_FREQUENCY_2;
     if (label == 38)
-        return AV_CH_STEREO_LEFT;
+        return AV_CHAN_STEREO_LEFT;
     if (label == 39)
-        return AV_CH_STEREO_RIGHT;
-    return 0;
+        return AV_CHAN_STEREO_RIGHT;
+    return AV_CHAN_UNKNOWN;
 }
 
 static uint32_t mov_get_channel_label(enum AVChannel channel)
@@ -497,8 +497,8 @@ int ff_mov_read_chan(AVFormatContext *s, AVIOContext *pb, AVStream *st,
                      int64_t size)
 {
     uint32_t layout_tag, bitmap, num_descr;
-    uint64_t label_mask, mask = 0;
-    int i;
+    int ret;
+    AVChannelLayout *ch_layout = &st->codecpar->ch_layout;
 
     if (size < 12)
         return AVERROR_INVALIDDATA;
@@ -514,47 +514,58 @@ int ff_mov_read_chan(AVFormatContext *s, AVIOContext *pb, AVStream *st,
     if (size < 12ULL + num_descr * 20ULL)
         return 0;
 
-    label_mask = 0;
-    for (i = 0; i < num_descr; i++) {
-        uint32_t label;
-        if (pb->eof_reached) {
-            av_log(s, AV_LOG_ERROR,
-                   "reached EOF while reading channel layout\n");
-            return AVERROR_INVALIDDATA;
+    if (layout_tag == 0) {
+        int nb_channels = ch_layout->nb_channels ? ch_layout->nb_channels : num_descr;
+        if (num_descr > nb_channels) {
+            av_log(s, AV_LOG_WARNING, "got %d channel descriptions, capping to the number of channels %d\n",
+                   num_descr, nb_channels);
+            num_descr = nb_channels;
         }
-        label     = avio_rb32(pb);          // mChannelLabel
-        avio_rb32(pb);                      // mChannelFlags
-        avio_rl32(pb);                      // mCoordinates[0]
-        avio_rl32(pb);                      // mCoordinates[1]
-        avio_rl32(pb);                      // mCoordinates[2]
-        size -= 20;
-        if (layout_tag == 0) {
-            uint64_t mask_incr = mov_get_channel_mask(label);
-            if (mask_incr == 0 || mask_incr <= label_mask) {
-                label_mask = 0;
-                break;
+
+        av_channel_layout_uninit(ch_layout);
+        ret = av_channel_layout_custom_init(ch_layout, nb_channels);
+        if (ret < 0)
+            goto out;
+
+        for (int i = 0; i < num_descr; i++) {
+            uint32_t label;
+            if (pb->eof_reached) {
+                av_log(s, AV_LOG_ERROR,
+                       "reached EOF while reading channel layout\n");
+                return AVERROR_INVALIDDATA;
             }
-            label_mask |= mask_incr;
+            label     = avio_rb32(pb);          // mChannelLabel
+            avio_rb32(pb);                      // mChannelFlags
+            avio_rl32(pb);                      // mCoordinates[0]
+            avio_rl32(pb);                      // mCoordinates[1]
+            avio_rl32(pb);                      // mCoordinates[2]
+            size -= 20;
+            ch_layout->u.map[i].id = mov_get_channel_id(label);
         }
-    }
-    if (layout_tag == 0) {
-        if (label_mask)
-            mask = label_mask;
-    } else
-        mask = mov_get_channel_layout(layout_tag, bitmap);
-
-    if (mask) {
-        if (!st->codecpar->ch_layout.nb_channels || av_popcount64(mask) == st->codecpar->ch_layout.nb_channels) {
-            av_channel_layout_uninit(&st->codecpar->ch_layout);
-            av_channel_layout_from_mask(&st->codecpar->ch_layout, mask);
-        } else {
-            av_log(s, AV_LOG_WARNING, "ignoring channel layout with %d channels because the real number of channels is %d\n",
-                   av_popcount64(mask), st->codecpar->ch_layout.nb_channels);
+
+        ret = av_channel_layout_retype(ch_layout, AV_CHANNEL_ORDER_NATIVE, AV_CHANNEL_LAYOUT_RETYPE_FLAG_LOSSLESS);
+        if (ret == AVERROR(ENOSYS))
+            ret = av_channel_layout_retype(ch_layout, AV_CHANNEL_ORDER_UNSPEC, AV_CHANNEL_LAYOUT_RETYPE_FLAG_LOSSLESS);
+        if (ret < 0 && ret != AVERROR(ENOSYS))
+            goto out;
+    } else {
+        uint64_t mask = mov_get_channel_layout(layout_tag, bitmap);
+        if (mask) {
+            if (!ch_layout->nb_channels || av_popcount64(mask) == ch_layout->nb_channels) {
+                av_channel_layout_uninit(ch_layout);
+                av_channel_layout_from_mask(ch_layout, mask);
+            } else {
+                av_log(s, AV_LOG_WARNING, "ignoring channel layout with %d channels because number of channels is %d\n",
+                       av_popcount64(mask), ch_layout->nb_channels);
+            }
         }
     }
+    ret = 0;
+
+out:
     avio_skip(pb, size - 12);
 
-    return 0;
+    return ret;
 }
 
 /* ISO/IEC 23001-8, 8.2 */
-- 
2.35.3

_______________________________________________
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 5/5] avformat/mov_chan: add support for reading custom channel layouts when layout_tag == 0

[Marton Balint]

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavformat/mov_chan.c | 99 +++++++++++++++++++++++-------------------
 1 file changed, 54 insertions(+), 45 deletions(-)

diff --git a/libavformat/mov_chan.c b/libavformat/mov_chan.c
index 6b206745b4..ce1e462dd1 100644
--- a/libavformat/mov_chan.c
+++ b/libavformat/mov_chan.c
@@ -377,23 +377,23 @@ static uint64_t mov_get_channel_layout(uint32_t tag, uint32_t bitmap)
     return layout_map[i].layout;
 }
 
-static uint64_t mov_get_channel_mask(uint32_t label)
+static enum AVChannel mov_get_channel_id(uint32_t label)
 {
     if (label == 0)
-        return 0;
+        return AV_CHAN_UNUSED;
     if (label <= 18)
-        return 1U << (label - 1);
+        return (label - 1);
     if (label == 35)
-        return AV_CH_WIDE_LEFT;
+        return AV_CHAN_WIDE_LEFT;
     if (label == 36)
-        return AV_CH_WIDE_RIGHT;
+        return AV_CHAN_WIDE_RIGHT;
     if (label == 37)
-        return AV_CH_LOW_FREQUENCY_2;
+        return AV_CHAN_LOW_FREQUENCY_2;
     if (label == 38)
-        return AV_CH_STEREO_LEFT;
+        return AV_CHAN_STEREO_LEFT;
     if (label == 39)
-        return AV_CH_STEREO_RIGHT;
-    return 0;
+        return AV_CHAN_STEREO_RIGHT;
+    return AV_CHAN_UNKNOWN;
 }
 
 static uint32_t mov_get_channel_label(enum AVChannel channel)
@@ -497,8 +497,8 @@ int ff_mov_read_chan(AVFormatContext *s, AVIOContext *pb, AVStream *st,
                      int64_t size)
 {
     uint32_t layout_tag, bitmap, num_descr;
-    uint64_t label_mask, mask = 0;
-    int i;
+    int ret;
+    AVChannelLayout *ch_layout = &st->codecpar->ch_layout;
 
     if (size < 12)
         return AVERROR_INVALIDDATA;
@@ -514,47 +514,56 @@ int ff_mov_read_chan(AVFormatContext *s, AVIOContext *pb, AVStream *st,
     if (size < 12ULL + num_descr * 20ULL)
         return 0;
 
-    label_mask = 0;
-    for (i = 0; i < num_descr; i++) {
-        uint32_t label;
-        if (pb->eof_reached) {
-            av_log(s, AV_LOG_ERROR,
-                   "reached EOF while reading channel layout\n");
-            return AVERROR_INVALIDDATA;
+    if (layout_tag == 0) {
+        int nb_channels = ch_layout->nb_channels ? ch_layout->nb_channels : num_descr;
+        if (num_descr > nb_channels) {
+            av_log(s, AV_LOG_WARNING, "got %d channel descriptions, capping to the number of channels %d\n",
+                   num_descr, nb_channels);
+            num_descr = nb_channels;
         }
-        label     = avio_rb32(pb);          // mChannelLabel
-        avio_rb32(pb);                      // mChannelFlags
-        avio_rl32(pb);                      // mCoordinates[0]
-        avio_rl32(pb);                      // mCoordinates[1]
-        avio_rl32(pb);                      // mCoordinates[2]
-        size -= 20;
-        if (layout_tag == 0) {
-            uint64_t mask_incr = mov_get_channel_mask(label);
-            if (mask_incr == 0 || mask_incr <= label_mask) {
-                label_mask = 0;
-                break;
+
+        av_channel_layout_uninit(ch_layout);
+        ret = av_channel_layout_from_custom(ch_layout, nb_channels);
+        if (ret < 0)
+            goto out;
+
+        for (int i = 0; i < num_descr; i++) {
+            uint32_t label;
+            if (pb->eof_reached) {
+                av_log(s, AV_LOG_ERROR,
+                       "reached EOF while reading channel layout\n");
+                return AVERROR_INVALIDDATA;
             }
-            label_mask |= mask_incr;
+            label     = avio_rb32(pb);          // mChannelLabel
+            avio_rb32(pb);                      // mChannelFlags
+            avio_rl32(pb);                      // mCoordinates[0]
+            avio_rl32(pb);                      // mCoordinates[1]
+            avio_rl32(pb);                      // mCoordinates[2]
+            size -= 20;
+            ch_layout->u.map[i].id = mov_get_channel_id(label);
         }
-    }
-    if (layout_tag == 0) {
-        if (label_mask)
-            mask = label_mask;
-    } else
-        mask = mov_get_channel_layout(layout_tag, bitmap);
-
-    if (mask) {
-        if (!st->codecpar->ch_layout.nb_channels || av_popcount64(mask) == st->codecpar->ch_layout.nb_channels) {
-            av_channel_layout_uninit(&st->codecpar->ch_layout);
-            av_channel_layout_from_mask(&st->codecpar->ch_layout, mask);
-        } else {
-            av_log(s, AV_LOG_WARNING, "ignoring channel layout with %d channels because the real number of channels is %d\n",
-                   av_popcount64(mask), st->codecpar->ch_layout.nb_channels);
+
+        ret = av_channel_layout_retype(ch_layout, AV_CHANNEL_ORDER_NATIVE);
+        if (ret < 0)
+            goto out;
+    } else {
+        uint64_t mask = mov_get_channel_layout(layout_tag, bitmap);
+        if (mask) {
+            if (!ch_layout->nb_channels || av_popcount64(mask) == ch_layout->nb_channels) {
+                av_channel_layout_uninit(ch_layout);
+                av_channel_layout_from_mask(ch_layout, mask);
+            } else {
+                av_log(s, AV_LOG_WARNING, "ignoring channel layout with %d channels because number of channels is %d\n",
+                       av_popcount64(mask), ch_layout->nb_channels);
+            }
         }
     }
+    ret = 0;
+
+out:
     avio_skip(pb, size - 12);
 
-    return 0;
+    return ret;
 }
 
 /* ISO/IEC 23001-8, 8.2 */
-- 
2.35.3

_______________________________________________
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/5] avformat/mov_chan: do not assume channels are in native order

[Marton Balint]

On Tue, 30 Jan 2024, Marton Balint wrote:

> Existing code could have caused wrong channel order signalling or reduced
> channel count if a channel designation appeared multiple times. This is
> actually an old bug, but the conversion to the new channel layout API made it
> visible, because now the code overrides the proper channel count with the one
> calculated from the mask.

Will apply and backport patches 1 and 2.

For the new API, I will wait a bit more.

Regards,
Marton

>
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
> libavformat/mov_chan.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavformat/mov_chan.c b/libavformat/mov_chan.c
> index f3d51899e1..5cb2de3820 100644
> --- a/libavformat/mov_chan.c
> +++ b/libavformat/mov_chan.c
> @@ -530,7 +530,7 @@ int ff_mov_read_chan(AVFormatContext *s, AVIOContext *pb, AVStream *st,
>         size -= 20;
>         if (layout_tag == 0) {
>             uint64_t mask_incr = mov_get_channel_mask(label);
> -            if (mask_incr == 0) {
> +            if (mask_incr == 0 || mask_incr <= label_mask) {
>                 label_mask = 0;
>                 break;
>             }
> -- 
> 2.35.3
>
> _______________________________________________
> 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 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".