Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Marton Balint <cus@passwd.hu>
To: ffmpeg-devel@ffmpeg.org
Cc: Marton Balint <cus@passwd.hu>
Subject: [FFmpeg-devel] [PATCH 5/5] avformat/mov: rework ff_mov_read_chnl
Date: Mon, 12 Feb 2024 22:15:37 +0100
Message-ID: <20240212211537.18468-5-cus@passwd.hu> (raw)
In-Reply-To: <20240212211537.18468-1-cus@passwd.hu>

A lot of changes and fixes to channel layout parsing, notably
- get rid of dynamic allocation of channel positions
- signal unimplemented speaker positions as unknown instead of failure, but
  warn the user about it
- native order, and that a single channel only appears once was always assumed
  for less than 64 channels, obviously this was incorrect

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavformat/mov_chan.c | 106 ++++++++++-------------------------------
 libavformat/mov_chan.h |   6 ---
 2 files changed, 26 insertions(+), 86 deletions(-)

diff --git a/libavformat/mov_chan.c b/libavformat/mov_chan.c
index cce9d7a697..3e186b0837 100644
--- a/libavformat/mov_chan.c
+++ b/libavformat/mov_chan.c
@@ -804,66 +804,6 @@ int ff_mov_get_channel_positions_from_layout(const AVChannelLayout *layout,
     return 0;
 }
 
-int ff_mov_get_layout_from_channel_positions(const uint8_t *position, int position_num,
-                                             AVChannelLayout *layout)
-{
-    int ret;
-    enum AVChannel channel;
-
-    av_channel_layout_uninit(layout);
-
-    if (position_num <= 63) {
-        layout->order = AV_CHANNEL_ORDER_NATIVE;
-        layout->nb_channels = position_num;
-        for (int i = 0; i < position_num; i++) {
-            if (position[i] >= FF_ARRAY_ELEMS(iso_channel_position)) {
-                ret = AVERROR_PATCHWELCOME;
-                goto error;
-            }
-
-            channel = iso_channel_position[position[i]];
-            // unsupported layout
-            if (channel == AV_CHAN_NONE) {
-                ret = AVERROR_PATCHWELCOME;
-                goto error;
-            }
-
-            layout->u.mask |= 1ULL << channel;
-        }
-    } else {
-        layout->order = AV_CHANNEL_ORDER_CUSTOM;
-        layout->nb_channels = position_num;
-        layout->u.map = av_calloc(position_num, sizeof(*layout->u.map));
-        if (!layout->u.map) {
-            ret = AVERROR(ENOMEM);
-            goto error;
-        }
-
-        for (int i = 0; i < position_num; i++) {
-            if (position[i] >= FF_ARRAY_ELEMS(iso_channel_position)) {
-                ret = AVERROR_PATCHWELCOME;
-                goto error;
-            }
-
-            channel = iso_channel_position[position[i]];
-            // unsupported layout
-            if (channel == AV_CHAN_NONE) {
-                ret = AVERROR_PATCHWELCOME;
-                goto error;
-            }
-
-            layout->u.map[i].id = channel;
-        }
-    }
-
-
-    return 0;
-
-error:
-    av_channel_layout_uninit(layout);
-    return ret;
-}
-
 int ff_mov_read_chnl(AVFormatContext *s, AVIOContext *pb, AVStream *st)
 {
     int stream_structure = avio_r8(pb);
@@ -875,33 +815,39 @@ int ff_mov_read_chnl(AVFormatContext *s, AVIOContext *pb, AVStream *st)
 
         av_log(s, AV_LOG_TRACE, "'chnl' layout %d\n", layout);
         if (!layout) {
-            uint8_t *positions = av_malloc(st->codecpar->ch_layout.nb_channels);
+            AVChannelLayout *ch_layout = &st->codecpar->ch_layout;
+            int nb_channels = ch_layout->nb_channels;
+
+            av_channel_layout_uninit(ch_layout);
+            ret = av_channel_layout_custom_init(ch_layout, nb_channels);
+            if (ret < 0)
+                return ret;
 
-            if (!positions)
-                return AVERROR(ENOMEM);
-            for (int i = 0; i < st->codecpar->ch_layout.nb_channels; i++) {
+            for (int i = 0; i < nb_channels; i++) {
                 int speaker_pos = avio_r8(pb);
+                enum AVChannel channel;
+
+                if (speaker_pos == 126) // explicit position
+                    avio_skip(pb, 3);   // azimuth, elevation
 
-                av_log(s, AV_LOG_TRACE, "speaker_position %d\n", speaker_pos);
-                if (speaker_pos == 126) { // explicit position
-                    avpriv_request_sample(s, "explicit position");
-                    av_freep(&positions);
-                    return AVERROR_PATCHWELCOME;
-                } else {
-                    positions[i] = speaker_pos;
+                if (speaker_pos >= FF_ARRAY_ELEMS(iso_channel_position))
+                    channel = AV_CHAN_NONE;
+                else
+                    channel = iso_channel_position[speaker_pos];
+
+                if (channel == AV_CHAN_NONE) {
+                    av_log(s, AV_LOG_WARNING, "speaker position %d is not implemented\n", speaker_pos);
+                    channel = AV_CHAN_UNKNOWN;
                 }
+
+                ch_layout->u.map[i].id = channel;
             }
 
-            ret = ff_mov_get_layout_from_channel_positions(positions,
-                    st->codecpar->ch_layout.nb_channels,
-                    &st->codecpar->ch_layout);
-            av_freep(&positions);
-            if (ret) {
-                av_log(s, AV_LOG_ERROR,
-                        "get channel layout from speaker positions failed, %s\n",
-                        av_err2str(ret));
+            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))
                 return ret;
-            }
         } else {
             uint64_t omitted_channel_map = avio_rb64(pb);
 
diff --git a/libavformat/mov_chan.h b/libavformat/mov_chan.h
index b7d435b99f..e480809c44 100644
--- a/libavformat/mov_chan.h
+++ b/libavformat/mov_chan.h
@@ -183,12 +183,6 @@ int ff_mov_get_channel_layout_from_config(int config, AVChannelLayout *layout);
 int ff_mov_get_channel_positions_from_layout(const AVChannelLayout *layout,
                                              uint8_t *position, int position_num);
 
-/**
- * Get AVChannelLayout from ISO/IEC 23001-8 OutputChannelPosition.
- */
-int ff_mov_get_layout_from_channel_positions(const uint8_t *position, int position_num,
-                                             AVChannelLayout *layout);
-
 /**
  * Read 'chnl' tag from the input stream.
  *
-- 
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".

  parent reply	other threads:[~2024-02-12 21:16 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-12 21:15 [FFmpeg-devel] [PATCH 1/5] avutil/channel_layout: change AV_CHAN_SILENCE to AV_CHAN_UNUSED in the docs Marton Balint
2024-02-12 21:15 ` [FFmpeg-devel] [PATCH 2/5] avutil/channel_layout: add AV_CHANNEL_ORDER_NB Marton Balint
2024-02-13 17:25   ` James Almer
2024-02-13 20:27     ` Marton Balint
2024-02-15 14:51       ` Anton Khirnov
2024-02-16 22:42         ` Marton Balint
2024-02-16 22:44           ` James Almer
2024-02-17 23:15             ` Marton Balint
2024-02-12 21:15 ` [FFmpeg-devel] [PATCH 3/5] avutil/tests/channel_layout: add tests for av_channel_order_retype Marton Balint
2024-02-13 17:39   ` [FFmpeg-devel] [PATCH] avutil/channel_layout: print known layout names in custom layout James Almer
2024-02-18 12:58     ` James Almer
2024-02-12 21:15 ` [FFmpeg-devel] [PATCH 4/5] avformat/mov: factorize reading the main part of the chnl atom to mov_chan Marton Balint
2024-02-12 21:15 ` Marton Balint [this message]
2024-02-15 14:52   ` [FFmpeg-devel] [PATCH 5/5] avformat/mov: rework ff_mov_read_chnl Anton Khirnov
2024-02-15 21:12     ` Marton Balint
2024-02-12 22:09 ` [FFmpeg-devel] [PATCH 1/5] avutil/channel_layout: change AV_CHAN_SILENCE to AV_CHAN_UNUSED in the docs James Almer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240212211537.18468-5-cus@passwd.hu \
    --to=cus@passwd.hu \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git