| Message ID | 20200616210253.7169-2-jeebjp@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | 22.2 channel layout support for AAC decoding | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
On 6/16/20, Jan Ekström <jeebjp@gmail.com> wrote: > Requires some extraneous top side and bottom front channels to be > defined. > > According to STD-B59v2, the defined channel layout is: > - FL > - FR > - FC > - LFE1 > - BL > - BR > - FLc > - FRc > - BC > - LFE2 > - SiL > - SiR > - TpFL > - TpFR > - TpFC > - TpC > - TpBL > - TpBR > - TpSiL > - TpSiR > - TpBC > - BtFC > - BtFL > - BtFR > --- > doc/APIchanges | 5 +++++ > libavutil/channel_layout.c | 6 ++++++ > libavutil/channel_layout.h | 6 ++++++ > libavutil/version.h | 2 +- > 4 files changed, 18 insertions(+), 1 deletion(-) > probably OK BTW What happened with new channel layout API?
Am Di., 16. Juni 2020 um 23:11 Uhr schrieb Jan Ekström <jeebjp@gmail.com>: > > Requires some extraneous top side and bottom front channels to be > defined. > > According to STD-B59v2, the defined channel layout is: > - FL > - FR > - FC > - LFE1 > - BL > - BR > - FLc > - FRc > - BC > - LFE2 > - SiL > - SiR No way to change the order to make it compatible with 7.1? Carl Eugen
On Sat, Aug 1, 2020 at 2:51 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > Am Di., 16. Juni 2020 um 23:11 Uhr schrieb Jan Ekström <jeebjp@gmail.com>: > > > > Requires some extraneous top side and bottom front channels to be > > defined. > > > > According to STD-B59v2, the defined channel layout is: > > - FL > > - FR > > - FC > > - LFE1 > > - BL > > - BR > > > - FLc > > - FRc > > - BC > > - LFE2 > > - SiL > > - SiR > > No way to change the order to make it compatible with 7.1? > I don't think that's realistic since then if we would like our actual 22.2 output to be compatible with the standard-defined channel order we would have to have a forced conversion at an end user facing end point, while having another one for the programmatic end points. 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. The specification is available in English @ http://www.arib.or.jp/english/html/overview/doc/6-STD-B59v2_0-E1.pdf . Best regards, Jan
Am Sa., 1. Aug. 2020 um 14:06 Uhr schrieb Jan Ekström <jeebjp@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? Is there an option to request the 5.1 mapping? Carl Eugen
On Sat, Aug 1, 2020 at 3:08 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > Am Sa., 1. Aug. 2020 um 14:06 Uhr schrieb Jan Ekström <jeebjp@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. At this point I would not like to attempt to figure out how to skip channel coding units in the AAC decoder, as swresample seems to handle this nicely. So far the only usage for requested channel layout in aacdec has been to skip reordering with AV_CH_LAYOUT_NATIVE , to output the coded units as-is. Best regards, Jan
Am Sa., 1. Aug. 2020 um 14:47 Uhr schrieb Jan Ekström <jeebjp@gmail.com>: > > On Sat, Aug 1, 2020 at 3:08 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > > Am Sa., 1. Aug. 2020 um 14:06 Uhr schrieb Jan Ekström <jeebjp@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... > 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? Thank you for your effort, Carl Eugen
On 8/1/2020 9:17 AM, Jan Ekström wrote: > On Sat, Aug 1, 2020 at 3:08 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> >> Am Sa., 1. Aug. 2020 um 14:06 Uhr schrieb Jan Ekström <jeebjp@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. I assume that does a "conversion" using swr from 22.2 to 5.1? (even if it just outputs the first six channels without extra resampling). What Carl wants is the decoder outputting only the first six channels, without the need for external resampling. This should be achievable with the -request_channel_layout input option (Try it with the DTS decoder on DTS-MA 7.1 samples to get 5.1, or in some DTS core samples to get stereo). No external conversion takes place in this scenario, since the decoder takes the hint to only output the requested channels, assuming it's doable (In the DTS case, the stream may not have a defined way within the bitstream to achieve this, and so the decoder just outputs the original channel layout silently). This would of course be separate work, but IMO very welcome and worth a look, and not a blocker. In any case, the decoder should by default output what the stream reports it contains, and not some arbitrary layout we think a CLI or player user will prefer. The user can and is expected to use a resampling library for that. > > At this point I would not like to attempt to figure out how to skip > channel coding units in the AAC decoder, as swresample seems to handle > this nicely. So far the only usage for requested channel layout in > aacdec has been to skip reordering with AV_CH_LAYOUT_NATIVE , to > output the coded units as-is. > > Best regards, > Jan > _______________________________________________ > 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". >
On Sat, Aug 1, 2020 at 4:29 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > Am Sa., 1. Aug. 2020 um 14:47 Uhr schrieb Jan Ekström <jeebjp@gmail.com>: > > > > On Sat, Aug 1, 2020 at 3:08 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > > > > Am Sa., 1. Aug. 2020 um 14:06 Uhr schrieb Jan Ekström <jeebjp@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
On Sat, Aug 1, 2020 at 4:42 PM James Almer <jamrial@gmail.com> wrote: > > On 8/1/2020 9:17 AM, Jan Ekström wrote: > > On Sat, Aug 1, 2020 at 3:08 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> > >> Am Sa., 1. Aug. 2020 um 14:06 Uhr schrieb Jan Ekström <jeebjp@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. > > I assume that does a "conversion" using swr from 22.2 to 5.1? (even if > it just outputs the first six channels without extra resampling). What > Carl wants is the decoder outputting only the first six channels, > without the need for external resampling. This should be achievable with > the -request_channel_layout input option (Try it with the DTS decoder on > DTS-MA 7.1 samples to get 5.1, or in some DTS core samples to get > stereo). No external conversion takes place in this scenario, since the > decoder takes the hint to only output the requested channels, assuming > it's doable (In the DTS case, the stream may not have a defined way > within the bitstream to achieve this, and so the decoder just outputs > the original channel layout silently). > > This would of course be separate work, but IMO very welcome and worth a > look, and not a blocker. In any case, the decoder should by default > output what the stream reports it contains, and not some arbitrary > layout we think a CLI or player user will prefer. The user can and is > expected to use a resampling library for that. > Aye. I know of that feature, and I actually did utilize `-request_channel_layout 9223372036854775808` for testing my decoder work (It is the only usage of that feature in the AAC decoder, it skips the coding unit reordering. That feature still works even with my changes). I do agree that it could be implemented, but I'm not sure if it should be part of this patch set which makes: 1. The streams demux and decode'able (unblocks remux) 2. Seems to output the correct 22.2 channel layout. (so that you can dump the 22.2 into WAV or otherwise utilize it through the API by default) 3. Can be utilized as output for 5.1 or stereo by utilizing swresample (if input is 22.2 and output is not 22.2 then it gets handled as 5.1 - at least until someone decides to make a proper mixing matrix configuration for it). I would actually prefer comments on how the swresample changes were done. They led to correct results for me, but this is the first time I'm poking swresample, so I might have missed something obvious. Best regards, Jan
On Sat, Aug 1, 2020 at 2:46 PM Paul B Mahol <onemda@gmail.com> wrote: > > On 6/16/20, Jan Ekström <jeebjp@gmail.com> wrote: > > Requires some extraneous top side and bottom front channels to be > > defined. > > > > According to STD-B59v2, the defined channel layout is: > > - FL > > - FR > > - FC > > - LFE1 > > - BL > > - BR > > - FLc > > - FRc > > - BC > > - LFE2 > > - SiL > > - SiR > > - TpFL > > - TpFR > > - TpFC > > - TpC > > - TpBL > > - TpBR > > - TpSiL > > - TpSiR > > - TpBC > > - BtFC > > - BtFL > > - BtFR > > --- > > doc/APIchanges | 5 +++++ > > libavutil/channel_layout.c | 6 ++++++ > > libavutil/channel_layout.h | 6 ++++++ > > libavutil/version.h | 2 +- > > 4 files changed, 18 insertions(+), 1 deletion(-) > > > > probably OK > > BTW What happened with new channel layout API? Thanks for taking a look. I think I've seen it mentioned recently when I've been discussing this patch set, so I have a feeling we might see a new version of that at some point. We already had multiple channels which were outside of WAVEFORMATEXTENSIBLE (such as LFE2), so a more defining API is clearly going to be more needed in the future (since the channels aren't always in the rising order of definition due to how things were defined age-wise). Jan
On Sat, Aug 1, 2020 at 4:44 PM Jan Ekström <jeebjp@gmail.com> wrote: > > > > 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. > I took a quick look at output_configure in aacdec_template, and it isn't too clear what's the best way of getting this done. The more proper way would seem to be to see if sniff_channel_order returned a layout, and then proceed to applying a switch/case thing for request_channel_layout. But more looking into the decoder would be required to figure out if one can get it done with: A. Just zero out mappings past the needed coding units. B. A and resetting the tags count within output_configure (since that is a parameter received from an outer function) C. Something else? Alternatively, something similar to B could get received by having the switch/case just before the get_new_frame check, and just overriding the layout and channels variables. The problem is that I'm not sure how well the best working alternative of these will work: A. Not at all, since it will attempt to pull in data for 24 channels but the AVFrame only contains 6. Segfault. B. Kind of. 24 channels will get allocated, but layout is 5.1. C. ??? I hope this clears up why I'm not too interested in trying to figure this out right now, since I have just been able to figure out the base decoding (which is currently everything the decoder supports!) and downmix through swresample. Jan
Jan Ekström (12020-08-02): > I think I've seen it mentioned recently when I've been discussing this > patch set, so I have a feeling we might see a new version of that at > some point. We already had multiple channels which were outside of > WAVEFORMATEXTENSIBLE (such as LFE2), so a more defining API is clearly > going to be more needed in the future (since the channels aren't > always in the rising order of definition due to how things were > defined age-wise). I did not see anything recently. As far as I remember, the proposal had serious limitations that must be addressed before it can be accepted: - It has nothing to handle cases where the same channel is present twice in the layout. For example, no syntax for the user to specify "FL, but the second one". - It has nothing to set extra labels and information about each channel. As it is, it is more complex than our current very simple bit mask but does not bring real benefit over it. As such, it is not worth it. Regards,
On 8/2/2020 11:22 AM, Jan Ekström wrote: > On Sat, Aug 1, 2020 at 4:44 PM Jan Ekström <jeebjp@gmail.com> wrote: >> >>>> 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. >> > > I took a quick look at output_configure in aacdec_template, and it > isn't too clear what's the best way of getting this done. > > The more proper way would seem to be to see if sniff_channel_order > returned a layout, and then proceed to applying a switch/case thing > for request_channel_layout. But more looking into the decoder would be > required to figure out if one can get it done with: > A. Just zero out mappings past the needed coding units. > B. A and resetting the tags count within output_configure (since that > is a parameter received from an outer function) > C. Something else? > > Alternatively, something similar to B could get received by having the > switch/case just before the get_new_frame check, and just overriding > the layout and channels variables. > > The problem is that I'm not sure how well the best working alternative > of these will work: > A. Not at all, since it will attempt to pull in data for 24 channels > but the AVFrame only contains 6. Segfault. > B. Kind of. 24 channels will get allocated, but layout is 5.1. > C. ??? > > I hope this clears up why I'm not too interested in trying to figure > this out right now, since I have just been able to figure out the base > decoding (which is currently everything the decoder supports!) and > downmix through swresample. As i said, that's fine for now. request_channel_layout implementations are optional and not always feasible (Very few audio codecs truly define a way to implement it, like it's the case of DTS). The decoder's job is to output what the stream contains, and a resampler can take care of everything afterwards should the user want something that's not defined within the bitstream.
On Sat, Aug 1, 2020 at 4:29 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > Am Sa., 1. Aug. 2020 um 14:47 Uhr schrieb Jan Ekström <jeebjp@gmail.com>: > > > > On Sat, Aug 1, 2020 at 3:08 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > > > > Am Sa., 1. Aug. 2020 um 14:06 Uhr schrieb Jan Ekström <jeebjp@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... > As this can be implied to be a comment regarding me not caring about the API users, and generally being wording that can be interpreted as disagreeing on the whole approach of the patch set, please respond whether this is a blocker or not. I'd rather get some closure on this one way or another, than stressing over whether this review was a blocker or not. Thank you, Jan
> Am 02.08.2020 um 23:16 schrieb Jan Ekström <jeebjp@gmail.com>: > >> On Sat, Aug 1, 2020 at 4:29 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> >>> Am Sa., 1. Aug. 2020 um 14:47 Uhr schrieb Jan Ekström <jeebjp@gmail.com>: >>> >>>> On Sat, Aug 1, 2020 at 3:08 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >>>> >>>>> Am Sa., 1. Aug. 2020 um 14:06 Uhr schrieb Jan Ekström <jeebjp@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... >> > > As this can be implied to be a comment regarding me not caring about > the API users, It was a comment about me not understanding how the average of hundreds of library users will be able to use this - important - new feature. > and generally being wording that can be interpreted as > disagreeing on the whole approach of the patch set, please respond > whether this is a blocker or not. Definitely not and I am sorry if my wording was so bad. Carl Eugen
On Mon, Aug 3, 2020 at 9:34 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > > > Am 02.08.2020 um 23:16 schrieb Jan Ekström <jeebjp@gmail.com>: > > > >> On Sat, Aug 1, 2020 at 4:29 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> > >>> Am Sa., 1. Aug. 2020 um 14:47 Uhr schrieb Jan Ekström <jeebjp@gmail.com>: > >>> > >>>> On Sat, Aug 1, 2020 at 3:08 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >>>> > >>>>> Am Sa., 1. Aug. 2020 um 14:06 Uhr schrieb Jan Ekström <jeebjp@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... > >> > > > > As this can be implied to be a comment regarding me not caring about > > the API users, > > It was a comment about me not understanding how the average of hundreds of library users will be able to use this - important - new feature. > They can use it like they use any other uncommon channel layout, by using lavfi or swresample directly to remix the audio - the same process that ffmpeg.c uses internally. - Hendrik
On Mon, Aug 3, 2020 at 10:34 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > > > Am 02.08.2020 um 23:16 schrieb Jan Ekström <jeebjp@gmail.com>: > > > >> On Sat, Aug 1, 2020 at 4:29 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> > >>> Am Sa., 1. Aug. 2020 um 14:47 Uhr schrieb Jan Ekström <jeebjp@gmail.com>: > >>> > >>>> On Sat, Aug 1, 2020 at 3:08 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >>>> > >>>>> Am Sa., 1. Aug. 2020 um 14:06 Uhr schrieb Jan Ekström <jeebjp@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... > >> > > > > As this can be implied to be a comment regarding me not caring about > > the API users, > > It was a comment about me not understanding how the average of hundreds of library users will be able to use this - important - new feature. > > > and generally being wording that can be interpreted as > > disagreeing on the whole approach of the patch set, please respond > > whether this is a blocker or not. > > Definitely not and I am sorry if my wording was so bad. > > Carl Eugen Thank you for confirming that this is not a blocker. Jan
diff --git a/doc/APIchanges b/doc/APIchanges index 1d6cc36b8c..84fe736454 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -15,6 +15,11 @@ libavutil: 2017-10-21 API changes, most recent first: +2020-06-16 - xxxxxxxxxx - lavu 56.56.100 - channel_layout.h + Add AV_CH_LAYOUT_22POINT2 together with its newly required pieces: + AV_CH_TOP_SIDE_LEFT, AV_CH_TOP_SIDE_RIGHT, AV_CH_BOTTOM_FRONT_LEFT, + AV_CH_BOTTOM_FRONT_CENTER, AV_CH_BOTTOM_FRONT_RIGHT. + 2020-06-12 - b09fb030c1 - lavu 56.55.100 - pixdesc.h Add AV_PIX_FMT_X2RGB10. diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c index 3bd5ee29b7..d6bfc74927 100644 --- a/libavutil/channel_layout.c +++ b/libavutil/channel_layout.c @@ -62,6 +62,11 @@ static const struct channel_name channel_names[] = { [33] = { "SDL", "surround direct left" }, [34] = { "SDR", "surround direct right" }, [35] = { "LFE2", "low frequency 2" }, + [36] = { "TSL", "top side left" }, + [37] = { "TSR", "top side right" }, + [38] = { "BFL", "bottom front left" }, + [39] = { "BFC", "bottom front center" }, + [40] = { "BFR", "bottom front right" }, }; static const char *get_channel_name(int channel_id) @@ -104,6 +109,7 @@ static const struct { { "octagonal", 8, AV_CH_LAYOUT_OCTAGONAL }, { "hexadecagonal", 16, AV_CH_LAYOUT_HEXADECAGONAL }, { "downmix", 2, AV_CH_LAYOUT_STEREO_DOWNMIX, }, + { "22.2", 24, AV_CH_LAYOUT_22POINT2, }, }; static uint64_t get_channel_layout_single(const char *name, int name_len) diff --git a/libavutil/channel_layout.h b/libavutil/channel_layout.h index 50bb8f03c5..43297505cb 100644 --- a/libavutil/channel_layout.h +++ b/libavutil/channel_layout.h @@ -71,6 +71,11 @@ #define AV_CH_SURROUND_DIRECT_LEFT 0x0000000200000000ULL #define AV_CH_SURROUND_DIRECT_RIGHT 0x0000000400000000ULL #define AV_CH_LOW_FREQUENCY_2 0x0000000800000000ULL +#define AV_CH_TOP_SIDE_LEFT 0x0000001000000000ULL +#define AV_CH_TOP_SIDE_RIGHT 0x0000002000000000ULL +#define AV_CH_BOTTOM_FRONT_LEFT 0x0000004000000000ULL +#define AV_CH_BOTTOM_FRONT_CENTER 0x0000008000000000ULL +#define AV_CH_BOTTOM_FRONT_RIGHT 0x0000010000000000ULL /** Channel mask value used for AVCodecContext.request_channel_layout to indicate that the user requests the channel order of the decoder output @@ -110,6 +115,7 @@ #define AV_CH_LAYOUT_OCTAGONAL (AV_CH_LAYOUT_5POINT0|AV_CH_BACK_LEFT|AV_CH_BACK_CENTER|AV_CH_BACK_RIGHT) #define AV_CH_LAYOUT_HEXADECAGONAL (AV_CH_LAYOUT_OCTAGONAL|AV_CH_WIDE_LEFT|AV_CH_WIDE_RIGHT|AV_CH_TOP_BACK_LEFT|AV_CH_TOP_BACK_RIGHT|AV_CH_TOP_BACK_CENTER|AV_CH_TOP_FRONT_CENTER|AV_CH_TOP_FRONT_LEFT|AV_CH_TOP_FRONT_RIGHT) #define AV_CH_LAYOUT_STEREO_DOWNMIX (AV_CH_STEREO_LEFT|AV_CH_STEREO_RIGHT) +#define AV_CH_LAYOUT_22POINT2 (AV_CH_LAYOUT_5POINT1_BACK|AV_CH_FRONT_LEFT_OF_CENTER|AV_CH_FRONT_RIGHT_OF_CENTER|AV_CH_BACK_CENTER|AV_CH_LOW_FREQUENCY_2|AV_CH_SIDE_LEFT|AV_CH_SIDE_RIGHT|AV_CH_TOP_FRONT_LEFT|AV_CH_TOP_FRONT_RIGHT|AV_CH_TOP_FRONT_CENTER|AV_CH_TOP_CENTER|AV_CH_TOP_BACK_LEFT|AV_CH_TOP_BACK_RIGHT|AV_CH_TOP_SIDE_LEFT|AV_CH_TOP_SIDE_RIGHT|AV_CH_TOP_BACK_CENTER|AV_CH_BOTTOM_FRONT_CENTER|AV_CH_BOTTOM_FRONT_LEFT|AV_CH_BOTTOM_FRONT_RIGHT) enum AVMatrixEncoding { AV_MATRIX_ENCODING_NONE, diff --git a/libavutil/version.h b/libavutil/version.h index 3ce9b1831e..a63f79feb1 100644 --- a/libavutil/version.h +++ b/libavutil/version.h @@ -79,7 +79,7 @@ */ #define LIBAVUTIL_VERSION_MAJOR 56 -#define LIBAVUTIL_VERSION_MINOR 55 +#define LIBAVUTIL_VERSION_MINOR 56 #define LIBAVUTIL_VERSION_MICRO 100 #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \