Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: "Raphaël Zumer" <raphael.zumer@vimeo.com>
To: ffmpeg-devel@ffmpeg.org
Subject: [FFmpeg-devel] [RFC] [PATCH] avcodec/codec_par: Keep format channel layout if specified
Date: Sat, 15 Jul 2023 10:51:23 -0400
Message-ID: <3b5364c5-6a72-a387-2b72-60d542379551@vimeo.com> (raw)

Hello,

Tagging this as RFC in case there is disagreement on the correct/desired behavior.

For context:
I am working with spatial audio and noticed that FFmpeg does not honor the SA3D box metadata, which is used to signal ambisonic audio channel layouts.
FFmpeg does parse the SA3D box and applies the specified channel layout properties to the codec parameters. However, they are later reset based on codec-level parameters in avcodec_parameters_from_context(), which is called from multiple locations between SA3D being parsed and the channel layout being resolved in FFprobe.
The result is that only codecs which support ambisonic signaling (I don't know of any besides Opus) can be recognized as ambisonic, despite container-level metadata being present. Since a lot of ambisonic content is distributed in AAC or PCM/Wave format, I think that the SA3D metadata should take precedence over codec-level metadata in this case.

Obviously this means ignoring the codec-level metadata which may not match. If there are reasons to prioritize the channel layout defined at the codec level at the expense of SA3D, please let me know the rationale. Also, I am aware that at least libswresample is also ignoring the container-level metadata, so other changes may be needed to propagate the SA3D properties when decoding/filtering ambisonic tracks, but this allows for detection as a first step.

The implementation could be greatly simplified by not resetting the channel layout, but that would require removing av_channel_layout_uninit() from codec_parameters_reset() or adding an argument to skip it. I would favor the former, but this affects a few other functions in this file so let me know if you have a preference.

I assume this would be a micro version bump (or minor?) but will add that once other details are sorted out.

Thanks,
Raphaël Zumer

Signed-off-by: Raphaël Zumer <rzumer@tebako.net>
---
  libavcodec/codec_par.c | 32 +++++++++++++++++++++++++++++---
  1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/libavcodec/codec_par.c b/libavcodec/codec_par.c
index 775c187073..0cd707d431 100644
--- a/libavcodec/codec_par.c
+++ b/libavcodec/codec_par.c
@@ -100,10 +100,34 @@ int avcodec_parameters_copy(AVCodecParameters *dst, const AVCodecParameters *src
  int avcodec_parameters_from_context(AVCodecParameters *par,
                                      const AVCodecContext *codec)
  {
+    int keep_ch_layout = par->ch_layout.order != AV_CHANNEL_ORDER_UNSPEC;
+    AVChannelLayout ch_layout;
      int ret;
  
+    /*
+     * The stream codec parameters may have a channel layout set
+     * already that is not represented in the codec context.
+     * For example, spatial audio channel layouts in codecs with no
+     * signaling for them may be decoded from container-level metadata.
+     *
+     * Assume that if the channel order is specified, we should
+     * preserve the existing layout rather than let
+     * avcodec_parameters_from_context() override it.
+     */
+    if (keep_ch_layout) {
+        ret = av_channel_layout_copy(&ch_layout, &par->ch_layout);
+        if (ret < 0)
+            return ret;
+    }
+
      codec_parameters_reset(par);
  
+    if (keep_ch_layout) {
+        ret = av_channel_layout_copy(&par->ch_layout, &ch_layout);
+        if (ret < 0)
+            return ret;
+    }
+
      par->codec_type = codec->codec_type;
      par->codec_id   = codec->codec_id;
      par->codec_tag  = codec->codec_tag;
@@ -146,9 +170,11 @@ FF_DISABLE_DEPRECATION_WARNINGS
  FF_ENABLE_DEPRECATION_WARNINGS
          } else {
  #endif
-        ret = av_channel_layout_copy(&par->ch_layout, &codec->ch_layout);
-        if (ret < 0)
-            return ret;
+        if (!keep_ch_layout) {
+            ret = av_channel_layout_copy(&par->ch_layout, &codec->ch_layout);
+            if (ret < 0)
+                return ret;
+        }
  #if FF_API_OLD_CHANNEL_LAYOUT
  FF_DISABLE_DEPRECATION_WARNINGS
          }
-- 
2.41.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".

             reply	other threads:[~2023-07-15 14:51 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-15 14:51 Raphaël Zumer [this message]
2023-07-25 17:27 ` Vittorio Giovara
2023-07-25 18:53   ` Anton Khirnov
2023-07-25 19:37     ` Raphaël Zumer

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=3b5364c5-6a72-a387-2b72-60d542379551@vimeo.com \
    --to=raphael.zumer@vimeo.com \
    --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