Implement Opus stereo/mono codec configuration and SDP handling

RESOLVED FIXED in Firefox 46

Status

()

defect
P1
major
Rank:
10
RESOLVED FIXED
7 years ago
2 years ago

People

(Reporter: abr, Assigned: bwc)

Tracking

(Blocks 1 bug)

unspecified
mozilla47
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox44 wontfix, firefox45 wontfix, firefox46 fixed, firefox47 fixed)

Details

Attachments

(4 attachments, 1 obsolete attachment)

Reporter

Description

7 years ago
Currently, our webrtc signaling code contains a workaround for a bug in the WebRTC.org implementation wherein an attempt to configure two channels for the Opus codec fails (see http://code.google.com/p/webrtc/issues/detail?id=1013).

Once we update our WebRTC.org code to rev 3050 or later, the workaround should be pulled out.

At the moment, this workaround is in media/webrtc/signaling/src/media/VcmSIPCCBinding.cpp in the function map_VCM_Media_Payload_type. Following the anticipated landing of the patch Bug 817065, this workaround will move to media/webrtc/signaling/src/sipcc/core/gsm/gsm_sdp.c, in the function "gsmsdp_negotiate_codec". Search for the string "FIXME".
Reporter

Updated

7 years ago
Assignee: nobody → adam
Reporter

Updated

7 years ago
Depends on: webrtc_updates
Reporter

Comment 1

7 years ago
When the stereo handling is fixed in the webrtc.org code, we probably want to configure for stereo or mono based on the a=sprop-stereo and a=stereo values.
Reporter

Comment 2

7 years ago
"derf: So we want to send with min(us:sprop-stereo,them:stereo), and receive with min(us:stereo,them:sprop-stereo)."
Reporter

Comment 3

7 years ago
We also need to ensure that we send correct SDP attributes according to how we're configuring our engine.
Reporter

Updated

7 years ago
Summary: Update codec configuration handling for Opus → Update codec configuration handling for stereo Opus
Whiteboard: [WebRTC], [blocking-webrtc-]
Reporter

Comment 5

7 years ago
Note that the codec parameter changes need to be guided by the SDP handling cited in comment 1 and comment 2. I am currently intending this bug to handle both the SDP handling and the Opus configuration. The title has been updated accordingly.
Summary: Update codec configuration handling for stereo Opus → Implement Opus stereo/mono codec configuration and SDP handling
Attachment #702177 - Attachment is obsolete: true
Comment on attachment 712199 [details] [diff] [review]
adjust default opus frequency to 48000 in SDP

in-person r=derf
Attachment #712199 - Flags: review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/3cc609f4744c
Whiteboard: [WebRTC], [blocking-webrtc-] → [WebRTC], [blocking-webrtc-][leave-open]
Reporter

Updated

6 years ago
Priority: -- → P4

Updated

4 years ago
backlog: --- → webRTC+
Rank: 48
Whiteboard: [WebRTC], [blocking-webrtc-][leave-open] → [leave-open]
Reporter

Comment 10

4 years ago
It seems very unlikely that I'll have an opportunity to work on WebRTC platform development for the foreseeable future. I'm unassigning this bug so that someone else might pick it up.
Assignee: adam → nobody
Assignee

Comment 11

3 years ago
A lot of the implementation detail has changed here, but apparently this is still not working properly.
Assignee: nobody → docfaraday
Paul wrote on bug 1249098:
> What the team is telling me is that the existing clients cannot play stereo audio to the user.  Would > it be possible to force mono and then resolve the stereo/mono issue as another bug?

Can these existing clients play audio currently (with us encoding Opus as stereo)?  Except maybe in really unusual cases (if any), we're actually putting the same data into both channels.
Flags: needinfo?(paulej)
Making this a high priority (I'm assuming for now that we'll need this in Fx47), but I still want us to discuss with Cisco if this can be worked around in some other way.
Severity: trivial → major
Rank: 48 → 10
Priority: P4 → P1
QA Contact: jsmith
Comment on attachment 8724120 [details]
MozReview Request: Bug 818618: Honor (and emit) opus stereo fmtp param. r=jesup

https://reviewboard.mozilla.org/r/36877/#review33507

::: media/webrtc/signaling/src/sdp/SdpAttribute.h:1259
(Diff revision 1)
> -      os << "maxplaybackrate=" << maxplaybackrate;
> +      os << "maxplaybackrate=" << maxplaybackrate << "; "

I don't think it's normal to have spaces after commas in fmtp parameters, tough it may be syntactically valid.  (I saw one device a decade ago that insisted on a space, to the violation of the spec. We had to sniff the device manufacturer and change our params for that device to interop)

::: media/webrtc/signaling/test/sdp_unittests.cpp:1144
(Diff revision 1)
> -"a=fmtp:109 maxplaybackrate=32000" CRLF
> +"a=fmtp:109 maxplaybackrate=32000; stereo=1" CRLF

ditto for the tests

::: media/webrtc/signaling/test/sdp_unittests.cpp:1506
(Diff revision 1)
> -"a=fmtp:109 maxplaybackrate=32000" CRLF
> +"a=fmtp:109 maxplaybackrate=32000; stereo=1" CRLF

ditto
Attachment #8724120 - Flags: review?(rjesup) → review+

Comment 16

3 years ago
Randall, what I was told that some of our existing clients (the ones that are subject to bug 1249098 are also unable to play stereo.  If mono was the default, that would address the problem.  Newer endpoints can handle both.  The team is looking for a workaround, but confidence is low since we have little control over the legacy deployments.
Flags: needinfo?(paulej)
Assignee

Comment 17

3 years ago
Comment on attachment 8724120 [details]
MozReview Request: Bug 818618: Honor (and emit) opus stereo fmtp param. r=jesup

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/36877/diff/1-2/
Attachment #8724120 - Attachment description: MozReview Request: Bug 818618: Honor (and emit) opus stereo fmtp param. r?jesup → MozReview Request: Bug 818618: Honor (and emit) opus stereo fmtp param. r=jesup
Assignee

Comment 19

3 years ago
Try the binaries here and see if they work for you:

http://archive.mozilla.org/pub/firefox/try-builds/bcampen@mozilla.com-f99261d58e6a001a81d308f1c3bc14ff4acf9acd/
Flags: needinfo?(rdaware)
Flags: needinfo?(paulej)

Comment 20

3 years ago
(In reply to Byron Campen [:bwc] from comment #19)
> Try the binaries here and see if they work for you:
> 
> http://archive.mozilla.org/pub/firefox/try-builds/bcampen@mozilla.com-
> f99261d58e6a001a81d308f1c3bc14ff4acf9acd/

Hi Byron,

Thank you for the image. I verified this binary. It looks good now. Also, wanted to make sure if there is UT added for this bug. Thanks again.
Flags: needinfo?(rdaware)
Assignee

Comment 21

3 years ago
(In reply to rdaware from comment #20)
> (In reply to Byron Campen [:bwc] from comment #19)
> > Try the binaries here and see if they work for you:
> > 
> > http://archive.mozilla.org/pub/firefox/try-builds/bcampen@mozilla.com-
> > f99261d58e6a001a81d308f1c3bc14ff4acf9acd/
> 
> Hi Byron,
> 
> Thank you for the image. I verified this binary. It looks good now. Also,
> wanted to make sure if there is UT added for this bug. Thanks again.

   The signaling/negotiation part is unit-tested, but our integration tests do not have the means of covering something like this right now.

Comment 23

3 years ago
Thanks for getting this tested so quickly!
Flags: needinfo?(paulej)
Assignee

Updated

3 years ago
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Whiteboard: [leave-open]
Assignee

Updated

3 years ago
Target Milestone: --- → mozilla47
Assignee

Updated

3 years ago
Depends on: 1249098
Assignee

Updated

3 years ago
No longer depends on: webrtc_updates
Assignee

Comment 25

3 years ago
Comment on attachment 8724120 [details]
MozReview Request: Bug 818618: Honor (and emit) opus stereo fmtp param. r=jesup

Approval Request Comment
[Feature/regressing bug #]:

   Bug 953265 (sort of)

[User impact if declined]:

   Wasted bandwidth when interoping with endpoints that cannot make use of stereo opus, or even failure to interop with endpoints that cannot handle stereo opus.

[Describe test coverage new/current, TreeHerder]:

   There are some new unit-tests that cover the signaling aspects.

[Risks and why]: 

   Very low, not many changes were required here.

[String/UUID change made/needed]:

   None.
Attachment #8724120 - Flags: approval-mozilla-aurora?
Comment on attachment 8724120 [details]
MozReview Request: Bug 818618: Honor (and emit) opus stereo fmtp param. r=jesup

Approved, thanks for the extra tests, let's fix the wasted bandwidth problem
Attachment #8724120 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Hitting conflicts on this, though that might be because I can't uplift bug 1249098 due to conflicts in that patch.
Flags: needinfo?(docfaraday)
Assignee

Comment 28

3 years ago
This applies cleanly on the aurora backport of bug 1249098
Flags: needinfo?(docfaraday)

Comment 30

3 years ago
Is this actually available in FF46? I am trying to set audio to stereo, but still get a mono result. I also disabled echo cancellation to get rid of AEC, but this did not help either. I'm using these instructions as a reference how to enable stereo in WebRTC:

https://www.webrtcexample.com/blog/?go=all/how-to-support-stereo-in-a-webrtc-application/
Assignee

Comment 31

3 years ago
(In reply to Tom Brückner from comment #30)
> Is this actually available in FF46? I am trying to set audio to stereo, but
> still get a mono result. I also disabled echo cancellation to get rid of
> AEC, but this did not help either. I'm using these instructions as a
> reference how to enable stereo in WebRTC:
> 
> https://www.webrtcexample.com/blog/?go=all/how-to-support-stereo-in-a-webrtc-
> application/

   Can you attach the about:webrtc for your case?

Comment 33

3 years ago
Sure, thanks for the super-fast reply. I added the aboutWebrtc.html for your reference. I'm using Chrome as a peer for Firefox, and I am transmitting two streams in two separate peer connections:

1. The video stream in the main PC
2. The audio stream in a secondary stream

Reason for this setup is that I am building an app for musicians where they can transmit additional audio streams along with the primary (chat) stream.

If I start the secondary stream in Chrome, FF receives it as stereo correctly and I can hear it in stereo (this worked even without patching the SDP manually). But if I try it the opposite around (i.e. start the stream in FF), Chrome receives it in mono (at least I can hear it only in mono). It's a stream that I feed from Ableton Live into a virtual audio device on my Mac, to be able to route it.

Comment 34

3 years ago
> in a secondary stream

should read: secondary PC
Assignee

Comment 35

3 years ago
There's a stereo=0 in the remote SDP; since the stereo param describes what you're willing to receive, we send mono.

Comment 36

3 years ago
Ok, thanks! So I am trying to detect patch the answer SDP on Chrome side as well. I expected that this would be done automatically, since Chrome supports stereo.

Do you happen to know what might trigger Chrome to neglect the stereo request from FF and patch it down to mono? Is this "normal" behaviour?
Assignee

Comment 37

3 years ago
Are you asking why Chrome uses stereo=0 when Firefox used stereo=1?

When firefox puts stereo=1 in its SDP, all that means is that firefox would like to receive stereo. It says nothing about what should happen in the other direction, and so it does not have anything to do with whether chrome also uses stereo=1.
(In reply to Byron Campen [:bwc] (PTO Apr 15) from comment #37)
> Are you asking why Chrome uses stereo=0 when Firefox used stereo=1?
> 
> When firefox puts stereo=1 in its SDP, all that means is that firefox would
> like to receive stereo. It says nothing about what should happen in the
> other direction, and so it does not have anything to do with whether chrome
> also uses stereo=1.

to rephrase byron: SDP (with a few exceptions) describes what you want/need to receive.  In this case, Chrome wants to receive mono, so we aren't going to send stereo if Chrome says stereo=0 in an offer (or an answer).

Comment 39

3 years ago
Thanks guys for these additional insights. I was indeed assuming the opposite, i.e. that the offer signals which resources are available and in which formats. So I guess it's the best way to use the signaling channel for this information, since I need the sender to determine what is actually transferred (the peer is just listening to the audio in my use-case).

Comment 40

3 years ago
I tried this out now. Both SDP are set to stereo now, but still the sound I receive in Chrome (sent from FF) is mono. And even worse, it sounds very very bad, like a poor quality phone line. It sounds as if something (echo cancellation?) is degrading the quality. If I try the same the other way around, i.e. by sending the audio from Chrome to FF, the quality I'm receiving in Firefox is very good.
Assignee

Comment 41

3 years ago
This is just a guess, but the maxplaybackrate=0 in the remote SDP might be causing problems.

Comment 42

3 years ago
Yes, I noticed that as well and wondered whether this was causing problems. As I am using Muaz Khan's BandwidthHandler library to modify the SDP, I relied on his experience that this might be the right way to do it.

Comment 43

3 years ago
I set the maxplaybackrate to 48000 now, but this did not change anything. Still in mono and with bad quality.
Assignee

Comment 44

3 years ago
I wonder what happens if you remove maxplaybackrate entirely. Maybe there's some problem with the patch on bug 1249098.

Comment 45

3 years ago
If I remove it completely, Firefox sets it to maxplaybackrate = 0 automatically. That was the reason why it was set this way initially.


In Chrome I send this as the answer SDP:

a=fmtp:109 minptime=10; useinbandfec=1; stereo=1; sprop-stereo=1

and Firefox interprets it like this (in about:webrtc):

a=fmtp:109 maxplaybackrate=0;stereo=1

Interestingly, the sprop-stereo=1 is missing as well.

I double-checked the SDP fingerprint, it's the very same.
Assignee

Comment 46

3 years ago
How does it work when it is just firefox/firefox? Chrome/chrome? From what I can tell the audio quality is good with firefox/firefox (which will use stereo by default). It may be that Chrome just won't render stereo, regardless of what you put in the SDP.

Comment 47

3 years ago
I've tested it with two machines now, one Windows, one Mac. Communication direction was always from Mac (sending audio stream) to Windows (receiving audio stream), since it was easier to set-up a virtual audio device on the Mac to feed Ableton Live as an input device into WebRTC.

1. Chrome => Chrome

works perfectly, with top sound quality and rock-stable connection.

Mac offers SDP:
a=fmtp:111 minptime=10; useinbandfec=1; stereo=1; sprop-stereo=1

Windows answers with SDP:
a=fmtp:111 minptime=10; useinbandfec=1; stereo=1; sprop-stereo=1

2. Chrome => Firefox

Top sound quality and rock-stable connection. Yet, the left and right channels are swapped.

Mac offers SDP:
a=fmtp:111 minptime=10; useinbandfec=1; stereo=1; sprop-stereo=1
In about:webrtc on the Windows machine, this is shown as the external SDP: 
a=fmtp:111 maxplaybackrate=0;stereo=1

Windows answers with SDP:
a=fmtp:111 maxplaybackrate=48000; stereo=1; stereo=1; sprop-stereo=1
In about:webrtc, this is shown as the local SDP: 
a=fmtp:111 maxplaybackrate=48000;stereo=1

(The doubled stereo=1 is due to a bug in the BandwidthHandler library.)

3. Firefox => Chrome

Audio quality is not too bad (= no distortions, opposed to my tests Mac => Mac where the quality was inacceptable, as if some noise cancellation algorithm was working, and where drop-outs occurred from time to time, although both browsers were running on the same machine). However, the sound is mono only and seems to lack bass (which could be due to the mono quality).

Mac offers SDP:
a=fmtp:109 maxplaybackrate=48000;stereo=1; stereo=1; sprop-stereo=1

Windows answers with SDP:
a=fmtp:109 minptime=10; useinbandfec=1; stereo=1; sprop-stereo=1

4. Firefox => Firefox

Same as 3., mono.

Mac offers SDP:
a=fmtp:109 maxplaybackrate=48000;stereo=1; stereo=1; sprop-stereo=1
In about:webrtc on the Windows machine, this is shown as the external SDP: 
a=fmtp:109 maxplaybackrate=48000;stereo=1

Windows answers with SDP:
a=fmtp:109 maxplaybackrate=48000;stereo=1; stereo=1; sprop-stereo=1
In about:webrtc, this is shown as the local SDP: 
a=fmtp:109 maxplaybackrate=48000;stereo=1

So the mono conversion happens always if Firefox is the sending browser. If Chrome is sending, stereo works, but if Firefox is receiving the L/R channels are swapped.

If I can test anything else, just let me know.

Comment 48

3 years ago
Yes, the negotiation between Firefox and Chrome is not yet fully working...

If you wish, I can provide test accounts of sipcast.net to you, if you want to study the issues.
You need to log in before you can comment on or make changes to this bug.