Closed Bug 901527 Opened 11 years ago Closed 11 years ago

Audio static/"burble"/breakup in mozilla to mozilla webrtc calls

Categories

(Core :: WebRTC: Audio/Video, defect)

22 Branch
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla26
Tracking Status
firefox22 --- unaffected
firefox23 + verified
firefox24 + fixed
firefox25 + verified
firefox26 --- fixed

People

(Reporter: jesup, Assigned: jesup)

References

Details

Attachments

(5 files, 1 obsolete file)

When doing some testing, mreavy and I noticed that with both Aurora and Nightly we heard "static and 'burbles'" in the audio, but only when *both* ends were Mozilla.  If one end was Chrome, we heard perfect audio in both directions, which was somewhat surprising. If was somewhat subtle and slightly intermittent (so a second or two you might miss it), but clearly there.  A constant "ooooo" into the mic makes it easier to hear.

To eliminate the speex resampler, I set the microphone on my windows machine to 16K (invoking the no-resampling passthrough in the resampler code); it made no difference.  Also, the resampler passes the SNR tests in the webrtc.org unit tests - but since this is a recent change in the audio path, it makes sense to test that out first.  Also, if it was the resampler, I would expect it to manifest itself in mozilla->chrome calls (in one direction).
I hear significant audio distortion and latency when Firefox 23.0b10 is receiving an audio/video stream in my application.  This behavior reproduces 100% for FF/23 on Mac 10.6.8, 10.7.5 and Windows 7.  

FF/22 plays out the audio and video without distortion or delay.  The behavior of the FF/23 receiver is the same whether Chrome or Firefox is sending.

Sending a private repro case to Randell Jesup via email for further details.
I just now tested with apprtc.appspot.com between chrome 28 and the FF 23 stable that released today -

On the FF side, I hear audio distortion and >= 1 second latency.
On the chrome side, no distortion, no latency.

This test was conducted on Mac OS 10.8.4 with both browsers running on the same desktop.
Attached file loopback test to hear audio glitching (obsolete) —
this local loopback test demonstrates audio glitching on FF23 Mac OS 10.8.4.
Attachment #786430 - Attachment mime type: text/plain → text/html
@randell - the test page that I sent you via PM exhibits significantly worse issues than the loopback test attached.  I think may have something to do with how FF processes sender reports.  I noticed that FF 23 sends SRs for the audio SSRC but not the video SSRC.
Confirm the problem, had to disable WebRTC support for FF23 (problem exists in beta/nightly as well)
Works with aurora/nightly
Attachment #786430 - Attachment is obsolete: true
(In reply to bryandonnovan from comment #4)
> @randell - the test page that I sent you via PM exhibits significantly worse
> issues than the loopback test attached.  I think may have something to do
> with how FF processes sender reports.  I noticed that FF 23 sends SRs for
> the audio SSRC but not the video SSRC.

Yes (SRs for audio only): that's bug 864654 (audio/video conduit refactor)

I'll be very sad if that's the cause....
No, it is not the cause.  I modified my MCU to not forward video SRs and it made no difference. I've tried tests like not sending any SRs and that also made no appreciable difference.  I'm stumped and could not find a workaround.  This is a bummer.  I have to drop Firefox support since 23 is rolling out now.
On the output side, it appears the webrtc.org code is changing frequency when it gets real codec data, and that causes the speex resampler to return a non-10ms number of samples.  Solution is to go back to resetting the resampler on any frequency change and live with it glitching (which is what the upstream code does)
Comment on attachment 786625 [details] [diff] [review]
reset the resampler on rate change

Review of attachment 786625 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM
Attachment #786625 - Flags: review+
Try builds:
Release + patch (resubmitted due to typoing mochitest-3 as mochitest3)
https://tbpl.mozilla.org/?tree=Try&rev=d4c0d8598652
https://tbpl.mozilla.org/?tree=Try&rev=9b1e77501ec5

Inbound + patch:
https://tbpl.mozilla.org/?tree=Try&rev=73a84b8ced05

So far, I've verified locally on Linux (Beta/24 and Inbound/26) and Mac (inbound/26), and Windows (downloaded from inbound Try).  

Note that on Windows Inbound & Aurora there's a separate bug introduced on July 26th that causes increasing delay (but no quality issues like this one); that bug is only in 25 and 26 so far and I'm tracking it down with bisect.
Summary: Audio static/"burble" in mozilla to mozilla webrtc calls, but not mozilla to chrome. → Audio static/"burble"/breakup in mozilla to mozilla webrtc calls
Assignee: nobody → rjesup
Is one of the try builds specific to Stable/23 that I should try?
Either "Release + patch" Try build (those are 23 plus the patch).
fix valgrind error - null pointer when destroying a speex resampler
Comment on attachment 786726 [details] [diff] [review]
null pointer when resetting a resampler

Either...
Attachment #786726 - Flags: review?(roc)
Attachment #786726 - Flags: review?(jmvalin)
Found via valgrind; also seen on one build on Try
https://hg.mozilla.org/integration/mozilla-inbound/rev/2f380c0d1670

Also tested locally with a mozilla ASAN linux build, and valgrind on the webrtc.org unit tests.
I verified the fix in https://tbpl.mozilla.org/?tree=Try&rev=cfe192c1875b on OS 10.8.4 and Win7.
Confirmed reproducible with the latest Nightly, Aurora, Beta, and Release builds.
Verified fixed with the try build: https://tbpl.mozilla.org/?tree=Try&rev=cfe192c1875b

Please land ASAP.
Comment on attachment 786625 [details] [diff] [review]
reset the resampler on rate change

[Approval Request Comment]
(For this and the separate null-pointer patch following)

Regression caused by (bug #): 886886

User impact if declined: Unusable audio in WebRTC

Testing completed (on m-c, etc.): On inbound; Tested via Try and local builds against Release and Inbound on multiple platforms, tested via valgrind in webrtc.org unit tests, tested on Linux under ASAN.  Verified by external contributor using the Try.  Verified by QA on Release Try.

Risk to taking this patch (and alternatives if risky): Low; removes an optimization for frequency changes.

String or IDL/UUID changes made by this patch: none
Attachment #786625 - Flags: approval-mozilla-release?
Attachment #786625 - Flags: approval-mozilla-beta?
Attachment #786625 - Flags: approval-mozilla-aurora?
Attachment #786625 - Flags: approval-mozilla-beta?
Attachment #786625 - Flags: approval-mozilla-beta+
Attachment #786625 - Flags: approval-mozilla-aurora?
Attachment #786625 - Flags: approval-mozilla-aurora+
Attachment #786726 - Flags: review?(jmvalin)
Attachment #786726 - Flags: approval-mozilla-release?
Attachment #786726 - Flags: approval-mozilla-beta?
Attachment #786726 - Flags: approval-mozilla-aurora?
Attachment #786726 - Flags: approval-mozilla-beta?
Attachment #786726 - Flags: approval-mozilla-beta+
Attachment #786726 - Flags: approval-mozilla-aurora?
Attachment #786726 - Flags: approval-mozilla-aurora+
Flags: needinfo?(rjesup)
Why not just back out bug 886886 on release? Seems like the safest option.
Blocks: 886886
Backing it out is an option and should be safe.  I'll run try builds on that  (Probably 3 patches to back out.)  I've already run an inbound Try with this removed and it was verified on mac and windows by the external contributor.

Note that backing out will leave any windows user with a 44100Hz mic with unusable audio beyond a minute or so due to audio drift (increasing delay).  Some examples: Lenovo W520/530's default to 44100, a common headset (Plantronics 626 DSP) that mozilla IT gives out maxes out and defaults to 44100, etc.  This behavior would be the same as FF22, we can relnote it - but most users will never figure out what the reason is.  But it would not be a regression.

On the other side, while tested (!) and simple, this fix has no bake time.

Personally, I'd go with the fix.  We have had some real QA ears on it.  But I'm ok either way.  I'll start tries of a release backout.
Flags: needinfo?(rjesup)
(In reply to Alex Keybl [:akeybl] from comment #23)
> Why not just back out bug 886886 on release? Seems like the safest option.

I think the risks are approximately the same either way, which suggests it's better to go with the option that improves things for users and has at least had some testing.
(In reply to Timothy B. Terriberry (:derf) from comment #26)
> (In reply to Alex Keybl [:akeybl] from comment #23)
> > Why not just back out bug 886886 on release? Seems like the safest option.
> 
> I think the risks are approximately the same either way, which suggests it's
> better to go with the option that improves things for users and has at least
> had some testing.

:derf,

Can you explain a bit more why this is the right path forward given we lived with it with so many weeks on beta ? Also the forward fix will be going in only in on Tuesday for next Beta, so not sure we will have enough feedback/testing either or other fall-outs this may cause.
(In reply to bhavana bajaj [:bajaj] from comment #28)
> Can you explain a bit more why this is the right path forward given we lived
> with it with so many weeks on beta ? Also the forward fix will be going in
> only in on Tuesday for next Beta, so not sure we will have enough
> feedback/testing either or other fall-outs this may cause.

Because I think the original patches landing in 886886 had enough moving parts that risk of screwing up the backout is approximately as large as the risk of the fix being incorrect.
The backout-on-release Try is largely complete and green
Tracking for a potential 23.0.1 - we'll discuss whether to forward fix or backout to return to known-good state offline.
Firefox 26.0a1 2013-08-06: stuttering sound
Firefox 26.0a1 2013-08-07: no sound
Firefox 26.0a1 2013-08-08: no sound

Firefox 25.0a2 2013-08-06: stuttering sound
Firefox 25.0a2 2013-08-07: stuttering sound
Firefox 25.0a2 2013-08-08: smooth sound

It's strange that I'm getting mixed results. It seems as though this is now fixed in Aurora but completely broken in Nightly.
Today's nightly (a0dd80f800e2) does not yet have the fix in it (for some reason).  Please re-verify with tomorrow's nightly (it landed in m-c about 150 changesets afterwards).  Because it took so long to merge, it got on aurora/beta first.
Randall - is one of these patches the backout of bug 886886?  We'll want to approve and land the backout to mozilla-release today/early tomorrow so we can go to build on a 23.0.1
Flags: needinfo?(rjesup)
Sorry, I had run the Try but not uploaded them.  I still prefer the fix, but the backout is ready.   (clean backout)
Flags: needinfo?(rjesup)
Comment on attachment 789280 [details] [diff] [review]
backout speex resampler fixes for 44100Hz mics (bug 886886)

[Approval Request Comment]
Regression caused by (bug #): 886886

User impact if declined: We need to take this set or the fixes or webrtc is unusable.  Taking this patch will leave 44100 mics on Windows broken and should be relnoted.

Testing completed (on m-c, etc.): Tested locally and a clean Try which was also tested by the contributor who reported this.

Risk to taking this patch (and alternatives if risky):  Very low (restores previous function).  Alternative is the fix already on m-c/aurora/beta.

String or IDL/UUID changes made by this patch: none
Attachment #789280 - Flags: approval-mozilla-release?
Attachment #789281 - Flags: approval-mozilla-release?
(In reply to Randell Jesup [:jesup] from comment #39)
> Comment on attachment 789280 [details] [diff] [review]
> backout speex resampler fixes for 44100Hz mics (bug 886886)
> 
> [Approval Request Comment]
> Regression caused by (bug #): 886886
> 
> User impact if declined: We need to take this set or the fixes or webrtc is
> unusable.  Taking this patch will leave 44100 mics on Windows broken and
> should be relnoted.
> 
> Testing completed (on m-c, etc.): Tested locally and a clean Try which was
> also tested by the contributor who reported this.
> 
> Risk to taking this patch (and alternatives if risky):  Very low (restores
> previous function).  Alternative is the fix already on m-c/aurora/beta.
> 
> String or IDL/UUID changes made by this patch: none

In beta 24 on Mac the issue still exists.
The fix won't be in a released beta until b2 is built; you can download a build off of https://tbpl.mozilla.org/?tree=Mozilla-Beta if you need one.
Attachment #789280 - Flags: approval-mozilla-release? → approval-mozilla-release+
Attachment #789281 - Flags: approval-mozilla-release? → approval-mozilla-release+
We're going to take this backout into 23.0.1 and will relnote that this is a known issue, resolved in FF24 coming in 5 weeks.
Blocks: 904754
No longer blocks: 904754
See Also: → 904754
Attachment #786625 - Flags: approval-mozilla-release?
Attachment #786726 - Flags: approval-mozilla-release?
See Also: 904754
Verified as fixed on Firefox 23.0.1, on multiple environments. Details of the testing done are available here:
https://wiki.mozilla.org/Releases/Firefox_23/Test_Plan#WebRTC
(In reply to Ioana Budnar, QA [:ioana] from comment #44)
> Verified as fixed on Firefox 23.0.1, on multiple environments. Details of
> the testing done are available here:
> https://wiki.mozilla.org/Releases/Firefox_23/Test_Plan#WebRTC

I'm not sure we can call this verified based on bug 906057. Please advise.
(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #45)
> (In reply to Ioana Budnar, QA [:ioana] from comment #44)
> > Verified as fixed on Firefox 23.0.1, on multiple environments. Details of
> > the testing done are available here:
> > https://wiki.mozilla.org/Releases/Firefox_23/Test_Plan#WebRTC
> 
> I'm not sure we can call this verified based on bug 906057. Please advise.

Bug 906057 only reports on delay.  This bug is only deals with audio break up or "burble". 

I would expect to hear delay on Windows that use 44.1KHz mics and run Fx 23.0.1 because we backed out the fix for Bug 886886 on Fx 23.0.1 to resolve the break up or "burble" reporting in this bug.
There are a couple instances where "jerky" sound was reported:

> Firefox 23.0.1 on Mac OSX 10.8 to Firefox 22.0 on Ubuntu 13.04 
> Firefox 23.0.1 on Mac OSX 10.8 to Firefox 22.0 on Windows 7 

Should this have been addressed by the backout?
The version 23.0.1 user agent string incorrectly reports the version as 23.0

Here is the value of navigator.userAgent on my mac:
"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:23.0) Gecko/20100101 Firefox/23.0"
(In reply to Maire Reavy [:mreavy] from comment #46)
> I would expect to hear delay on Windows that use 44.1KHz mics and run Fx
> 23.0.1 because we backed out the fix for Bug 886886 on Fx 23.0.1 to resolve
> the break up or "burble" reporting in this bug.

That needs to be reported in bug 886886, then. Right now it's still labeled as fixed, even thought it is now reported as a known issue.
(In reply to bryandonnovan from comment #48)
> The version 23.0.1 user agent string incorrectly reports the version as 23.0
> 
> Here is the value of navigator.userAgent on my mac:
> "Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:23.0) Gecko/20100101
> Firefox/23.0"

This is normal, we do not indicate the minor version number in the user agent string anymore. You should see it in the About dialog though. That said, this bug is not the right place to report this. You should report a new bug and refrain from hijacking unrelated RESOLVED FIXED bugs.
We still had one-side Audio static/burble while testing 1 on 1 (call was made from the first OS to the second one) as it follows:
Windows 7x64 (Firefox 23.0.1) to XP (Firefox 24.0b4) - the Caller
Windows 7x64 (Chrome Stable) to Mac OS X 10.8 (Firefox 24.0b4) - Callee
Ubuntu 12.04 (Firefox 24.0b4) to Windows 7x64 (Firefox 23.0.1) - Callee
Ubuntu 12.04 (Firefox 23.0.1) to Mac OS X 10.8 (Firefox 24.0b4) - Caller
Note that this bug (901527) would cause a major regression in all incoming audio in calls.  It would not be dependent on who called, and it would be almost totally repeatable.  It does vary in impact from one OS to another (it seems least easily heard on Windows), but it's always there.

If there are still some intermittent quality issues, please file a new bug describing them as best you can.  If recordings are possible, please add those.

Audacity is preferred for recording and is available on all OS's, but it is a full-featured audio editor with many options, and so could be confusing (and may be a problem for a call on Mac due to the preferred solutions causing output to appear as input).  It should work well on Linux and Windows:

http://manual.audacityteam.org/man/Tutorial_-_Recording_audio_playing_on_the_computer

or (very simple and old, Windows only)

http://www.milosoftware.com/en/index.php?body=soundleech.php
I've tried to capture the background noise but in the recording everything was smooth, without any audio burble.

Also, found these cases:
1. If we muted the mics from the system settings, the noise was still there.
2. Muting from the browser streamzone (right click - mute), that noise was gone.

Also, this affects Chrome and it can be stopped using step 2.
Cornel, how did you proceed to record the audio?
Using http://www.milosoftware.com/en/index.php?body=soundleech.php 

I think that the background noise was caused by the microphones we used for testing, as we didn't hear any audio burble when we disabled them and used only video.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: