Closed
Bug 818618
Opened 12 years ago
Closed 9 years ago
Implement Opus stereo/mono codec configuration and SDP handling
Categories
(Core :: WebRTC: Signaling, defect, P1)
Core
WebRTC: Signaling
Tracking
()
RESOLVED
FIXED
mozilla47
backlog | webrtc/webaudio+ |
People
(Reporter: abr, Assigned: bwc)
References
(Blocks 1 open bug)
Details
Attachments
(4 files, 1 obsolete file)
(deleted),
patch
|
jesup
:
review+
|
Details | Diff | Splinter Review |
(deleted),
text/x-review-board-request
|
jesup
:
review+
lizzard
:
approval-mozilla-aurora+
|
Details |
(deleted),
text/html
|
Details | |
(deleted),
text/html
|
Details |
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•12 years ago
|
Assignee: nobody → adam
Reporter | ||
Updated•12 years ago
|
Depends on: webrtc_updates
Reporter | ||
Comment 1•12 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•12 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•12 years ago
|
||
We also need to ensure that we send correct SDP attributes according to how we're configuring our engine.
Reporter | ||
Updated•12 years ago
|
Summary: Update codec configuration handling for Opus → Update codec configuration handling for stereo Opus
Updated•12 years ago
|
Whiteboard: [WebRTC], [blocking-webrtc-]
Comment 4•12 years ago
|
||
Reporter | ||
Comment 5•12 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
Comment 6•12 years ago
|
||
Updated•12 years ago
|
Attachment #702177 -
Attachment is obsolete: true
Comment 7•12 years ago
|
||
Comment on attachment 712199 [details] [diff] [review]
adjust default opus frequency to 48000 in SDP
in-person r=derf
Attachment #712199 -
Flags: review+
Comment 8•12 years ago
|
||
Whiteboard: [WebRTC], [blocking-webrtc-] → [WebRTC], [blocking-webrtc-][leave-open]
Comment 9•12 years ago
|
||
Reporter | ||
Updated•12 years ago
|
Priority: -- → P4
Updated•10 years ago
|
backlog: --- → webRTC+
Rank: 48
Whiteboard: [WebRTC], [blocking-webrtc-][leave-open] → [leave-open]
Reporter | ||
Comment 10•9 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•9 years ago
|
||
A lot of the implementation detail has changed here, but apparently this is still not working properly.
Assignee: nobody → docfaraday
Comment 12•9 years ago
|
||
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)
Comment 13•9 years ago
|
||
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
Assignee | ||
Comment 14•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/36877/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/36877/
Attachment #8724120 -
Flags: review?(rjesup)
Comment 15•9 years ago
|
||
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•9 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•9 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
Comment 18•9 years ago
|
||
Assignee | ||
Comment 19•9 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•9 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•9 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 22•9 years ago
|
||
Comment 24•9 years ago
|
||
bugherder |
Assignee | ||
Updated•9 years ago
|
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [leave-open]
Assignee | ||
Updated•9 years ago
|
Target Milestone: --- → mozilla47
Assignee | ||
Updated•9 years ago
|
No longer depends on: webrtc_updates
Assignee | ||
Comment 25•9 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?
Updated•9 years ago
|
status-firefox44:
--- → wontfix
status-firefox45:
--- → wontfix
status-firefox46:
--- → affected
status-firefox47:
--- → fixed
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•9 years ago
|
||
This applies cleanly on the aurora backport of bug 1249098
Flags: needinfo?(docfaraday)
Comment 29•9 years ago
|
||
bugherder uplift |
Comment 30•9 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•9 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 32•9 years ago
|
||
Comment 33•9 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•9 years ago
|
||
> in a secondary stream
should read: secondary PC
Assignee | ||
Comment 35•9 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•9 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•9 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.
Comment 38•9 years ago
|
||
(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•9 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•9 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•9 years ago
|
||
This is just a guess, but the maxplaybackrate=0 in the remote SDP might be causing problems.
Comment 42•9 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•9 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•9 years ago
|
||
I wonder what happens if you remove maxplaybackrate entirely. Maybe there's some problem with the patch on bug 1249098.
Comment 45•9 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•9 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•9 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•8 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.
Description
•