[FFmpeg-devel] [PATCH 1/5] avutil/channel_layout: add 22.2 layout

[Jan Ekström]

On Sat, Aug 1, 2020 at 4:29 PM Carl Eugen Hoyos <ceffmpeg at gmail.com> wrote:
>
> Am Sa., 1. Aug. 2020 um 14:47 Uhr schrieb Jan Ekström <jeebjp at gmail.com>:
> >
> > On Sat, Aug 1, 2020 at 3:08 PM Carl Eugen Hoyos <ceffmpeg at gmail.com> wrote:
> > >
> > > Am Sa., 1. Aug. 2020 um 14:06 Uhr schrieb Jan Ekström <jeebjp at gmail.com>:
> > >
> > > > Additionally, the spec only mentions stereo/5.1 for direct mapping, so
> > > > I've kept to 5.1 since we have the capability for that one.
> > >
> > > And you still believe it would be a disadvantage if the decoder outputs
> > > this 5.1 mapping by default if nothing else was requested?
> >
> > Yes. API users so far have received the audio frames according to
> > encoded layout by default, so doing something else breaks that rule of
> > least surprises.
> >
> > > Is there an option to request the 5.1 mapping?
> >
> > As noted in the 0/0 cover letter, you can do it just fine with
> > `-channel_layout "5.1"` with ffmpeg.c. I have verified that it returns
> > the 5.1 channels as-is.
>
> But this does not make the decoder output 5.1 or does it?
>
> While I should probably only care about ffmpeg and ignore
> the library users I still wonder who can live with your approach...
>

I do actually think of the API users, or at least how I see API users
around my point of vision (aka "what I can see from where I am
currently"). You are of course free to note of other users, and how
they see things problematically.

I think from the decoding API, the API users will expect whatever is
coded in the file, just like with video 12bit YCbCr it is being
decoded to 12bit YCbCr and not auto-converted to 8bit YCbCr just so
that it is simpler to handle by most applications (even if 10bit YCbCr
is becoming popular, 12 bit is still rather rare - just like 22.2).
And such decoded AVFrames then require swscale or some libavfilter
filter to dither it down to some specific bit depth and/or to 8bit
RGB. Although to be honest by the amount of actual broadcast samples
I've received of 22.2 AAC at this point 22.2 might be more common than
non-community made 12bit YCbCr encodes. Feel free to replace 12bit
YCbCr with 10bit YCbCr if you wish.

Applications which have their own audio frame thing and have a manual
#define MAX_CHANNEL_COUNT 8 on their own side will fail their own
sanity checks, yes. I know I will quite likely be poking at one API
user which currently has such a limit after this work goes in. That
said, this same issue would have happened each time larger channel
layout support was added.

I know there used to be a remix/resampling-included API before in the
decoding side of things. That way if an application knew it always
wanted X, it could request that with a single API, and not care what
was actually there in the bit stream. I can see the usefulness of
that. Granted, now that decoding is decoding, and remix is in its own
thing, most API users already have moved to either our remixing
library, or remixing through lavfi, or to a 3rd party remixer. I think
having a discussion on a higher level API like ffms2 provides is worth
it, but then again I'm not sure if it should be straddled on this
patch set.

> > At this point I would not like to attempt to figure out how to skip
> > channel coding units in the AAC decoder
>
> Just throw them away?
>

Very easy to say in words, but I am not sure how simple this is in the
code. That said, if you are interested in implementing this in the
decoder, I am not opposed to the idea since in this specific case the
spec seems to note that 5.1 can be mapped into those channels.

> Thank you for your effort, Carl Eugen

Thank you.

Jan