1444479 - Remove decoder channel limit restrictions.

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, enhancement, P3)

Description

[Jean-Yves Avenard [:jya]]

Following bug 1432779 and bug 1431221, we no longer need to be limited to 8 decoded audio channels.

We will remove all limits and let each decoder and cubeb deal with it...

Comment 1

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1444479 - P1. Remove 8 channels limitation in AudioConfig.

Instead we place it at 32.
Future changes will change the meaning of this limit to when we can deal with channel layout. If outside that limit the audio will be played on a best attempt basis.

Review commit: https://reviewboard.mozilla.org/r/228384/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/228384/

Comment 2

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1444479 - P2. Make AudioConverter works with unknown layout.

If a channel layout is unsupported, the AudioConverter will instead just use the channel count information to leave the data as-is, only trimming extra channels, or inserting silence if needed.

Review commit: https://reviewboard.mozilla.org/r/228386/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/228386/

Comment 3

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1444479 - P3. Add Channels(ChannelMap) method.

Review commit: https://reviewboard.mozilla.org/r/228388/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/228388/

Comment 4

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1444479 - P4. Add new AudioConfig constructor.

This will allow to create an AudioConfig with an unknown or unsupported channel layout, defaulting instead to the number of channels.

Review commit: https://reviewboard.mozilla.org/r/228390/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/228390/

Comment 5

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1444479 - P5. Let AudioSink deal with unknown layout.

Review commit: https://reviewboard.mozilla.org/r/228392/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/228392/

Comment 6

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1444479 - P6. Make Opus and Vorbis decoder deal with more channels than 8.

Under 8 channels, the audio will be reordered so it can be playable on any platforms.
Over 8 channels, the channels will be as output by the decoder. Playing of such stream will be platform dependent as neither Opus nor Vorbis define a channel layout with more than 8 channels.

With WebAudio however, the result will be platform independent (as long as you don't attempt to play it)

Review commit: https://reviewboard.mozilla.org/r/228394/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/228394/

Comment 7

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1444479 - P7. Allow Apple AudioToolbox decoder handle more than 8 channels.

Apple layout standard has a 1:1 equivalence with the WAVE standard. As such, any streams under 18 channels, properly defining a channel layout, should play on all platform.
Otherwise, as Opus and Vorbis, the result will be platform dependent.

Review commit: https://reviewboard.mozilla.org/r/228396/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/228396/

Comment 8

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1444479 - P8. Remove FFmpeg audio decoder channel limitation.

Review commit: https://reviewboard.mozilla.org/r/228398/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/228398/

Comment 9

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1444479 - P9. Remove WMF audio decoder channel limit.

Review commit: https://reviewboard.mozilla.org/r/228400/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/228400/

Comment 10

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1444479 - P10. Don't reject files with more than 8 audio channels.

Review commit: https://reviewboard.mozilla.org/r/228402/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/228402/

Comment 11

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1444479 - P11. Remove ununsed constant.

Review commit: https://reviewboard.mozilla.org/r/228404/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/228404/

Comment 12

[Paul Adenot (:padenot)]

Comment on attachment 8959583 [details]
Bug 1444479 - P1. Remove 8 channels limitation in AudioConfig.

https://reviewboard.mozilla.org/r/228384/#review234288

Comment 13

[Paul Adenot (:padenot)]

Comment on attachment 8959584 [details]
Bug 1444479 - P2. Make AudioConverter works with unknown layout.

https://reviewboard.mozilla.org/r/228386/#review234280

::: dom/media/AudioConverter.cpp:148
(Diff revision 1)
>    return aX < -32768 ? -32768 : aX <= 32767 ? aX : 32767;
>  }
>  
> +template<typename TYPE>
> +static void
> +dumbCopy(TYPE* aOut,

Maybe this could have a name that is a bit more explicit.

Comment 14

[Paul Adenot (:padenot)]

Comment on attachment 8959585 [details]
Bug 1444479 - P3. Add Channels(ChannelMap) method.

https://reviewboard.mozilla.org/r/228388/#review234282

::: dom/media/AudioConfig.h:121
(Diff revision 1)
> +      aMap -= (aMap >> 1) & 0x55555555;
> +      aMap = (aMap & 0x33333333) + ((aMap >> 2) & 0x33333333);
> +      aMap = (aMap + (aMap >> 4)) & 0x0F0F0F0F;
> +      aMap += aMap >> 8;
> +      return (aMap + (aMap >> 16)) & 0x3F;
> +#endif

Use https://searchfox.org/mozilla-central/source/mfbt/MathAlgorithms.h#338

Comment 15

[Paul Adenot (:padenot)]

Comment on attachment 8959586 [details]
Bug 1444479 - P4. Add new AudioConfig constructor.

https://reviewboard.mozilla.org/r/228390/#review234290

Comment 16

[Paul Adenot (:padenot)]

Comment on attachment 8959587 [details]
Bug 1444479 - P5. Let AudioSink deal with unknown layout.

https://reviewboard.mozilla.org/r/228392/#review234292

Comment 17

[Paul Adenot (:padenot)]

Comment on attachment 8959588 [details]
Bug 1444479 - P6. Make Opus and Vorbis decoder deal with more channels than 8.

https://reviewboard.mozilla.org/r/228394/#review234284

::: commit-message-b1c55:4
(Diff revision 1)
> +Bug 1444479 - P6. Make Opus and Vorbis decoder deal with more channels than 8. r?padenot
> +
> +Under 8 channels, the audio will be reordered so it can be playable on any platforms.
> +Over 8 channels, the channels will be as output by the decoder. Playing of such stream will be platform dependent as neither Opus nor Vorbis define a channel layout with more than 8 channels.

Fix the message so that it's clear what happens at exactly 8 channels (strictly over vs. over).

::: dom/media/ogg/OpusParser.cpp:84
(Diff revision 1)
>        mCoupledStreams = mChannels - 1;
>        mMappingTable[0] = 0;
>        mMappingTable[1] = 1;
>      } else if (mChannelMapping == 1 || mChannelMapping == 255) {
>        // Currently only up to 8 channels are defined for mapping family 1 and we
>        // only supports only up to 8 channels for mapping family 255.

I think the comment needs an update: we now support more channels when the mapping family _is_ 255.

Comment 18

[Paul Adenot (:padenot)]

Comment on attachment 8959589 [details]
Bug 1444479 - P7. Allow Apple AudioToolbox decoder handle more than 8 channels.

https://reviewboard.mozilla.org/r/228396/#review234296

Comment 19

[Paul Adenot (:padenot)]

Comment on attachment 8959590 [details]
Bug 1444479 - P8. Remove FFmpeg audio decoder channel limitation.

https://reviewboard.mozilla.org/r/228398/#review234298

Comment 20

[Paul Adenot (:padenot)]

Comment on attachment 8959591 [details]
Bug 1444479 - P9. Remove WMF audio decoder channel limit.

https://reviewboard.mozilla.org/r/228400/#review234302

Comment 21

[Paul Adenot (:padenot)]

Comment on attachment 8959592 [details]
Bug 1444479 - P10. Don't reject files with more than 8 audio channels.

https://reviewboard.mozilla.org/r/228402/#review234304

Comment 22

[Paul Adenot (:padenot)]

Comment on attachment 8959592 [details]
Bug 1444479 - P10. Don't reject files with more than 8 audio channels.

https://reviewboard.mozilla.org/r/228402/#review234306

Comment 23

[Paul Adenot (:padenot)]

Comment on attachment 8959593 [details]
Bug 1444479 - P11. Remove ununsed constant.

https://reviewboard.mozilla.org/r/228404/#review234308

Comment 24

[Jean-Yves Avenard [:jya]]

Comment on attachment 8959585 [details]
Bug 1444479 - P3. Add Channels(ChannelMap) method.

https://reviewboard.mozilla.org/r/228388/#review234626

::: dom/media/AudioConfig.h:121
(Diff revision 1)
> +      aMap -= (aMap >> 1) & 0x55555555;
> +      aMap = (aMap & 0x33333333) + ((aMap >> 2) & 0x33333333);
> +      aMap = (aMap + (aMap >> 4)) & 0x0F0F0F0F;
> +      aMap += aMap >> 8;
> +      return (aMap + (aMap >> 16)) & 0x3F;
> +#endif

thanks.. learnt something new today...

Comment 25

[Jean-Yves Avenard [:jya]]

Comment on attachment 8959588 [details]
Bug 1444479 - P6. Make Opus and Vorbis decoder deal with more channels than 8.

https://reviewboard.mozilla.org/r/228394/#review234284

> Fix the message so that it's clear what happens at exactly 8 channels (strictly over vs. over).

Suggestions welcome, I don't see how I can make this comment any clearer.

Comment 26

[Jean-Yves Avenard [:jya]]

Comment on attachment 8959584 [details]
Bug 1444479 - P2. Make AudioConverter works with unknown layout.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/228386/diff/1-2/

Comment 27

[Jean-Yves Avenard [:jya]]

Comment on attachment 8959585 [details]
Bug 1444479 - P3. Add Channels(ChannelMap) method.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/228388/diff/1-2/

Comment 28

[Jean-Yves Avenard [:jya]]

Comment on attachment 8959586 [details]
Bug 1444479 - P4. Add new AudioConfig constructor.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/228390/diff/1-2/

Comment 29

[Jean-Yves Avenard [:jya]]

Comment on attachment 8959587 [details]
Bug 1444479 - P5. Let AudioSink deal with unknown layout.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/228392/diff/1-2/

Comment 30

[Jean-Yves Avenard [:jya]]

Comment on attachment 8959588 [details]
Bug 1444479 - P6. Make Opus and Vorbis decoder deal with more channels than 8.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/228394/diff/1-2/

Comment 31

[Jean-Yves Avenard [:jya]]

Comment on attachment 8959589 [details]
Bug 1444479 - P7. Allow Apple AudioToolbox decoder handle more than 8 channels.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/228396/diff/1-2/

Comment 32

[Jean-Yves Avenard [:jya]]

Comment on attachment 8959590 [details]
Bug 1444479 - P8. Remove FFmpeg audio decoder channel limitation.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/228398/diff/1-2/

Comment 33

[Jean-Yves Avenard [:jya]]

Comment on attachment 8959591 [details]
Bug 1444479 - P9. Remove WMF audio decoder channel limit.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/228400/diff/1-2/

Comment 34

[Jean-Yves Avenard [:jya]]

Comment on attachment 8959592 [details]
Bug 1444479 - P10. Don't reject files with more than 8 audio channels.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/228402/diff/1-2/

Comment 35

[Jean-Yves Avenard [:jya]]

Comment on attachment 8959593 [details]
Bug 1444479 - P11. Remove ununsed constant.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/228404/diff/1-2/

Comment 36

[Jean-Yves Avenard [:jya]]

Comment on attachment 8959583 [details]
Bug 1444479 - P1. Remove 8 channels limitation in AudioConfig.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/228384/diff/1-2/

Comment 37

[Jean-Yves Avenard [:jya]]

Comment on attachment 8959584 [details]
Bug 1444479 - P2. Make AudioConverter works with unknown layout.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/228386/diff/2-3/

Comment 38

[Jean-Yves Avenard [:jya]]

Comment on attachment 8959585 [details]
Bug 1444479 - P3. Add Channels(ChannelMap) method.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/228388/diff/2-3/

Comment 39

[Jean-Yves Avenard [:jya]]

Comment on attachment 8959586 [details]
Bug 1444479 - P4. Add new AudioConfig constructor.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/228390/diff/2-3/

Comment 40

[Jean-Yves Avenard [:jya]]

Comment on attachment 8959587 [details]
Bug 1444479 - P5. Let AudioSink deal with unknown layout.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/228392/diff/2-3/

Comment 41

[Jean-Yves Avenard [:jya]]

Comment on attachment 8959588 [details]
Bug 1444479 - P6. Make Opus and Vorbis decoder deal with more channels than 8.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/228394/diff/2-3/

Comment 42

[Jean-Yves Avenard [:jya]]

Comment on attachment 8959589 [details]
Bug 1444479 - P7. Allow Apple AudioToolbox decoder handle more than 8 channels.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/228396/diff/2-3/

Comment 43

[Jean-Yves Avenard [:jya]]

Comment on attachment 8959590 [details]
Bug 1444479 - P8. Remove FFmpeg audio decoder channel limitation.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/228398/diff/2-3/

Comment 44

[Jean-Yves Avenard [:jya]]

Comment on attachment 8959591 [details]
Bug 1444479 - P9. Remove WMF audio decoder channel limit.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/228400/diff/2-3/

Comment 45

[Jean-Yves Avenard [:jya]]

Comment on attachment 8959592 [details]
Bug 1444479 - P10. Don't reject files with more than 8 audio channels.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/228402/diff/2-3/

Comment 46

[Jean-Yves Avenard [:jya]]

Comment on attachment 8959593 [details]
Bug 1444479 - P11. Remove ununsed constant.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/228404/diff/2-3/

Comment 47

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/63b5bd67f700
P1. Remove 8 channels limitation in AudioConfig. r=padenot
https://hg.mozilla.org/integration/mozilla-inbound/rev/2ea728f4e688
P2. Make AudioConverter works with unknown layout. r=padenot
https://hg.mozilla.org/integration/mozilla-inbound/rev/b821de55844f
P3. Add Channels(ChannelMap) method. r=padenot
https://hg.mozilla.org/integration/mozilla-inbound/rev/5e3da7510ef1
P4. Add new AudioConfig constructor. r=padenot
https://hg.mozilla.org/integration/mozilla-inbound/rev/3207ab7618d5
P5. Let AudioSink deal with unknown layout. r=padenot
https://hg.mozilla.org/integration/mozilla-inbound/rev/938c300a46ca
P6. Make Opus and Vorbis decoder deal with more channels than 8. r=padenot
https://hg.mozilla.org/integration/mozilla-inbound/rev/6531087bc206
P7. Allow Apple AudioToolbox decoder handle more than 8 channels. r=padenot
https://hg.mozilla.org/integration/mozilla-inbound/rev/c3ff23419244
P8. Remove FFmpeg audio decoder channel limitation. r=padenot
https://hg.mozilla.org/integration/mozilla-inbound/rev/ef58e2386804
P9. Remove WMF audio decoder channel limit. r=padenot
https://hg.mozilla.org/integration/mozilla-inbound/rev/34d5728973b4
P10. Don't reject files with more than 8 audio channels. r=padenot
https://hg.mozilla.org/integration/mozilla-inbound/rev/8d4c3cf648d6
P11. Remove unused constant. r=padenot

Comment 48

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/63b5bd67f700
https://hg.mozilla.org/mozilla-central/rev/2ea728f4e688
https://hg.mozilla.org/mozilla-central/rev/b821de55844f
https://hg.mozilla.org/mozilla-central/rev/5e3da7510ef1
https://hg.mozilla.org/mozilla-central/rev/3207ab7618d5
https://hg.mozilla.org/mozilla-central/rev/938c300a46ca
https://hg.mozilla.org/mozilla-central/rev/6531087bc206
https://hg.mozilla.org/mozilla-central/rev/c3ff23419244
https://hg.mozilla.org/mozilla-central/rev/ef58e2386804
https://hg.mozilla.org/mozilla-central/rev/34d5728973b4
https://hg.mozilla.org/mozilla-central/rev/8d4c3cf648d6