On Sat, Jul 15, 2023 at 4:51 PM Raphaël Zumer <[email protected]> wrote:
> 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 <[email protected]> > --- > 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 > Any comments on this patch? If no objections I'd like to push it at the end of the week -- Vittorio _______________________________________________ ffmpeg-devel mailing list [email protected] https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email [email protected] with subject "unsubscribe".
