After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 757152 - opus: Add proper multichannel support
opus: Add proper multichannel support
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal enhancement
: 1.7.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 756282
 
 
Reported: 2015-10-26 18:43 UTC by Sebastian Dröge (slomo)
Modified: 2015-11-03 18:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
opus: Add proper support for multichannel audio (17.41 KB, patch)
2015-11-03 12:52 UTC, Sebastian Dröge (slomo)
none Details | Review
matroskademux: There is no multistream field for Opus anymore (1.30 KB, patch)
2015-11-03 12:52 UTC, Sebastian Dröge (slomo)
committed Details | Review
opus: Add proper support for multichannel audio (17.41 KB, patch)
2015-11-03 13:03 UTC, Sebastian Dröge (slomo)
none Details | Review
opus: Add proper support for multichannel audio (16.98 KB, patch)
2015-11-03 13:05 UTC, Sebastian Dröge (slomo)
none Details | Review
opus: Add proper support for multichannel audio (16.98 KB, patch)
2015-11-03 14:46 UTC, Sebastian Dröge (slomo)
none Details | Review
opusparse: Also try to parse multichannel related fields from the upstream caps (3.94 KB, patch)
2015-11-03 14:46 UTC, Sebastian Dröge (slomo)
none Details | Review
oggdemux: Create full Opus caps with all fields (2.67 KB, patch)
2015-11-03 15:13 UTC, Sebastian Dröge (slomo)
none Details | Review
tsdemux: Don't create an incomplete OpusHead but set all the other caps fields instead (10.98 KB, patch)
2015-11-03 15:14 UTC, Sebastian Dröge (slomo)
none Details | Review
codec-utils: Add utilities for Opus caps and the OpusHead header (19.78 KB, patch)
2015-11-03 16:39 UTC, Sebastian Dröge (slomo)
none Details | Review
oggdemux: Create full Opus caps with all fields (1.22 KB, patch)
2015-11-03 16:40 UTC, Sebastian Dröge (slomo)
none Details | Review
codec-utils: Add utilities for Opus caps and the OpusHead header (19.88 KB, patch)
2015-11-03 17:06 UTC, Sebastian Dröge (slomo)
none Details | Review
opus: Add proper support for multichannel audio (31.86 KB, patch)
2015-11-03 17:44 UTC, Sebastian Dröge (slomo)
committed Details | Review
codec-utils: Add utilities for Opus caps and the OpusHead header (19.78 KB, patch)
2015-11-03 17:44 UTC, Sebastian Dröge (slomo)
committed Details | Review
oggdemux: Create full Opus caps with all fields (1.22 KB, patch)
2015-11-03 17:45 UTC, Sebastian Dröge (slomo)
committed Details | Review
tsmux: Simplify Opus caps parsing by using codecutils helpers (5.86 KB, patch)
2015-11-03 17:51 UTC, Sebastian Dröge (slomo)
committed Details | Review
tsdemux: Don't create an incomplete OpusHead but set all the other caps fields instead (2.19 KB, patch)
2015-11-03 17:54 UTC, Sebastian Dröge (slomo)
committed Details | Review
matroskademux: Use codecutils helpers for creating Opus caps (2.17 KB, patch)
2015-11-03 18:34 UTC, Sebastian Dröge (slomo)
committed Details | Review

Description Sebastian Dröge (slomo) 2015-10-26 18:43:03 UTC
Before bug #756282 (moving Opus to good/base) we should think about how to properly expose multichannel support, as this might need breaking caps changes or not.

So with Opus we need a way to pass around a few values
1) number of channels
2) number of streams
3) number of coupled streams
4) channel mapping array
5) actual channel layout

With Ogg this is stored in the OpusHead header, where 5) is solved by having a channel mapping family. 0 meaning "RTP" (1 channel => mono, 2 channels => stereo left/right, also no channel mapping array then and fixed numbers for 2) and 3)) and 1 meaning "Vorbis" (channel layout like in Vorbis).

With MP4 it's basically the same, just stored in a different structure. With Matroska/WebM it's exactly the OpusHead.

With MPEG-TS it's simpler (+ an extension mechanism) but everything maps (so far) to the OpusHead in a trivial way.


My proposal would be that we always require a streamheader field in Opus caps, which contain an array of the OpusHead and the comment buffer. The only exception to this rule would be for channels=1 and channels=2, which would then trivially map to the RTP channel mapping.
This would mean that the MP4 demuxer/muxer need to parse/generate the OpusHead (bug #742643) and the MPEG-TS demuxer/muxer too (bug #757049 , done already). It's not much code.

Comments?
Comment 1 Olivier Crête 2015-10-27 04:01:20 UTC
I'm not a big fan of passing a binary structure that doesn't allow any kind of caps negotiation.
Comment 2 Sebastian Dröge (slomo) 2015-10-27 07:06:29 UTC
The only alternative would then be to duplicate the contents of the OpusHead in multiple caps fields. Which would mean that we should put some helper in pbutils for example to convert these caps fields to an OpusHead and back, as otherwise we'll have to duplicate this code everywhere.
Comment 3 Tim-Philipp Müller 2015-10-27 08:59:41 UTC
What parts of this could make sense to negotiate?

The encoder will encode what it's given, the decoder will output what it received. The channel mappings in audio/x-raw will be given.

There's nothing to negotiate between muxer and encoder really, is there?
Comment 4 Sebastian Dröge (slomo) 2015-10-27 11:53:23 UTC
You could negotiate the channel mapping family (RTP elements can only do 0) at least. The number of channels is already in the caps, and the other fields are something every decoder must handle anyway if they can handle the channel mapping family and number of channels.
Comment 5 Sebastian Dröge (slomo) 2015-10-28 09:43:54 UTC
Olivier, Tim? What about the following then:
- always have channels and channel mapping family in the caps
- always have OpusHead in the caps unless channels==2 and RTP mapping


We can then negotiate the family and number of channels, and only duplicate those fields. While still requiring an OpusHead for passing all the other multichannel settings that are needed for decoders.
Comment 6 Tim-Philipp Müller 2015-10-28 10:26:41 UTC
I don't really understand the practical scenarios where it would make sense to negotiate anything here. Could you elaborate? One can always add these things later IMHO once someone has an actual use for it.
Comment 7 Sebastian Dröge (slomo) 2015-10-28 10:53:04 UTC
You would like to restrict channels=[1,2], channel-mapping-family=0 for the RTP payloader for example. It should just fail negotiating if given anything else.
Comment 8 Tim-Philipp Müller 2015-10-28 11:18:06 UTC
Right, that makes sense.

I can see the channel restriction of course, but it seemed a bit of a forced example because I didn't really see any reason to not output family=0 for mono/stereo, or does oggmux require the vorbis family type in this case?

For pretty much everything else but mono/stereo, there is exactly one suitable output option currently.
Comment 9 Sebastian Dröge (slomo) 2015-10-28 11:48:16 UTC
MPEG-TS can have 2 channel stereo with Vorbis family, there's something in a table of the spec :)

And once other families are added we have the problem of backwards compatibility with the caps. If we don't make the family mandatory now, negotiation will later become annoying.
Comment 10 Olivier Crête 2015-10-28 23:07:49 UTC
What are channel-mapping-family mean to convey?
Comment 11 Sebastian Dröge (slomo) 2015-10-29 08:11:50 UTC
See https://tools.ietf.org/html/draft-terriberry-oggopus-01#section-5.1.1

The same terminology is used in the other container mappings, and also for the opus_multistream_surround_encoder_*() API.


It basically tells you: If you have N channels, this is the position and order of them. Currently defined is RTP (family 0) which only allows mono and stereo, and Vorbis (family 1) which allows 1-8 channels.


Note that this is independent of the number of streams and number of coupled streams in the OpusHead (and as passed to the decoder/encoder). The number of streams / coupled streams is just information for the codec about how the N channels are represented internally.
Comment 12 Sebastian Dröge (slomo) 2015-10-29 21:31:27 UTC
It might actually also make sense to negotiate the other fields. In the case of MPEGTS there are two specific combinations of the other fields per channel that require only a simple descriptor. For all others you need a more complicated one, which nobody currently implements.
Comment 13 Sebastian Dröge (slomo) 2015-10-31 15:20:17 UTC
Ok as there were no further comments, I'm going to implement this by adding all 5 things to the caps: channel mapping family, channel count, stream count, coupled stream count and the channel mapping array. The last 3 things are optional of channel mapping family 0 (RTP) but mandatory otherwise.

See above why we actually need the array.


OpusHead would then be optional in this setup.
Comment 14 Sebastian Dröge (slomo) 2015-11-03 12:52:24 UTC
Created attachment 314727 [details] [review]
opus: Add proper support for multichannel audio
Comment 15 Sebastian Dröge (slomo) 2015-11-03 12:52:31 UTC
Created attachment 314728 [details] [review]
matroskademux: There is no multistream field for Opus anymore
Comment 16 Sebastian Dröge (slomo) 2015-11-03 13:03:26 UTC
Created attachment 314731 [details] [review]
opus: Add proper support for multichannel audio
Comment 17 Sebastian Dröge (slomo) 2015-11-03 13:05:44 UTC
Created attachment 314732 [details] [review]
opus: Add proper support for multichannel audio
Comment 18 Sebastian Dröge (slomo) 2015-11-03 14:46:25 UTC
Created attachment 314735 [details] [review]
opus: Add proper support for multichannel audio
Comment 19 Sebastian Dröge (slomo) 2015-11-03 14:46:31 UTC
Created attachment 314736 [details] [review]
opusparse: Also try to parse multichannel related fields from the upstream caps
Comment 20 Sebastian Dröge (slomo) 2015-11-03 15:13:47 UTC
Created attachment 314737 [details] [review]
oggdemux: Create full Opus caps with all fields
Comment 21 Sebastian Dröge (slomo) 2015-11-03 15:14:14 UTC
Created attachment 314738 [details] [review]
tsdemux: Don't create an incomplete OpusHead but set all the other caps fields instead

OpusHead is optional, the other fields are not.
Comment 22 Sebastian Dröge (slomo) 2015-11-03 16:39:47 UTC
Created attachment 314748 [details] [review]
codec-utils: Add utilities for Opus caps and the OpusHead header
Comment 23 Sebastian Dröge (slomo) 2015-11-03 16:40:01 UTC
Created attachment 314749 [details] [review]
oggdemux: Create full Opus caps with all fields
Comment 24 Sebastian Dröge (slomo) 2015-11-03 17:06:32 UTC
Created attachment 314751 [details] [review]
codec-utils: Add utilities for Opus caps and the OpusHead header
Comment 25 Sebastian Dröge (slomo) 2015-11-03 17:44:38 UTC
Created attachment 314752 [details] [review]
opus: Add proper support for multichannel audio
Comment 26 Sebastian Dröge (slomo) 2015-11-03 17:44:48 UTC
Created attachment 314753 [details] [review]
codec-utils: Add utilities for Opus caps and the OpusHead header
Comment 27 Sebastian Dröge (slomo) 2015-11-03 17:45:08 UTC
Created attachment 314754 [details] [review]
oggdemux: Create full Opus caps with all fields
Comment 28 Sebastian Dröge (slomo) 2015-11-03 17:51:48 UTC
Created attachment 314759 [details] [review]
tsmux: Simplify Opus caps parsing by using codecutils helpers
Comment 29 Sebastian Dröge (slomo) 2015-11-03 17:54:01 UTC
Created attachment 314760 [details] [review]
tsdemux: Don't create an incomplete OpusHead but set all the other caps fields instead

OpusHead is optional, the other fields are not.
Comment 30 Sebastian Dröge (slomo) 2015-11-03 18:34:03 UTC
Created attachment 314762 [details] [review]
matroskademux: Use codecutils helpers for creating Opus caps

Also fix up codec data with values from the container.
Comment 31 Sebastian Dröge (slomo) 2015-11-03 18:41:14 UTC
All merged