Last Comment Bug 901527 - Audio static/"burble"/breakup in mozilla to mozilla webrtc calls
: Audio static/"burble"/breakup in mozilla to mozilla webrtc calls
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: WebRTC: Audio/Video (show other bugs)
: 22 Branch
: All All
: -- major with 1 vote (vote)
: mozilla26
Assigned To: [:jesup] on pto until 2016/8/1 Randell Jesup
:
Mentors:
: 901539 (view as bug list)
Depends on:
Blocks: 886886
  Show dependency treegraph
 
Reported: 2013-08-05 07:24 PDT by [:jesup] on pto until 2016/8/1 Randell Jesup
Modified: 2013-09-05 15:42 PDT (History)
17 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
+
verified
+
fixed
+
verified
fixed


Attachments
loopback test to hear audio glitching (2.49 KB, text/html)
2013-08-06 12:10 PDT, bryandonnovan
no flags Details
loopback test to hear audio glitching (2.58 KB, text/html)
2013-08-06 13:14 PDT, [:jesup] on pto until 2016/8/1 Randell Jesup
no flags Details
reset the resampler on rate change (1.51 KB, patch)
2013-08-06 17:15 PDT, [:jesup] on pto until 2016/8/1 Randell Jesup
jmvalin: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
Details | Diff | Splinter Review
null pointer when resetting a resampler (1.07 KB, patch)
2013-08-06 21:36 PDT, [:jesup] on pto until 2016/8/1 Randell Jesup
roc: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
Details | Diff | Splinter Review
backout speex resampler fixes for 44100Hz mics (bug 886886) (64.07 KB, patch)
2013-08-12 17:26 PDT, [:jesup] on pto until 2016/8/1 Randell Jesup
lukasblakk+bugs: approval‑mozilla‑release+
Details | Diff | Splinter Review
backout 44000/44100Hz kludge removals (bug 886886) (54.38 KB, patch)
2013-08-12 17:26 PDT, [:jesup] on pto until 2016/8/1 Randell Jesup
lukasblakk+bugs: approval‑mozilla‑release+
Details | Diff | Splinter Review

Description [:jesup] on pto until 2016/8/1 Randell Jesup 2013-08-05 07:24:46 PDT
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).
Comment 1 bryandonnovan 2013-08-05 19:34:54 PDT
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.
Comment 2 bryandonnovan 2013-08-06 09:46:30 PDT
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.
Comment 3 bryandonnovan 2013-08-06 12:10:21 PDT
Created attachment 786430 [details]
loopback test to hear audio glitching

this local loopback test demonstrates audio glitching on FF23 Mac OS 10.8.4.
Comment 4 bryandonnovan 2013-08-06 12:54:03 PDT
@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.
Comment 5 Andrey Kovalenko 2013-08-06 12:56:20 PDT
Confirm the problem, had to disable WebRTC support for FF23 (problem exists in beta/nightly as well)
Comment 6 [:jesup] on pto until 2016/8/1 Randell Jesup 2013-08-06 13:14:21 PDT
Created attachment 786466 [details]
loopback test to hear audio glitching

Works with aurora/nightly
Comment 7 [:jesup] on pto until 2016/8/1 Randell Jesup 2013-08-06 13:31:38 PDT
(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....
Comment 8 bryandonnovan 2013-08-06 13:45:12 PDT
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.
Comment 9 [:jesup] on pto until 2016/8/1 Randell Jesup 2013-08-06 17:15:02 PDT
Created attachment 786625 [details] [diff] [review]
reset the resampler on rate change

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 10 Jean-Marc Valin (:jmspeex) 2013-08-06 19:15:41 PDT
Comment on attachment 786625 [details] [diff] [review]
reset the resampler on rate change

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

LGTM
Comment 11 [:jesup] on pto until 2016/8/1 Randell Jesup 2013-08-06 20:03:16 PDT
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.
Comment 12 [:jesup] on pto until 2016/8/1 Randell Jesup 2013-08-06 20:14:57 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/fba5c857adb6
Comment 13 bryandonnovan 2013-08-06 20:20:55 PDT
Is one of the try builds specific to Stable/23 that I should try?
Comment 14 [:jesup] on pto until 2016/8/1 Randell Jesup 2013-08-06 20:44:53 PDT
Either "Release + patch" Try build (those are 23 plus the patch).
Comment 15 [:jesup] on pto until 2016/8/1 Randell Jesup 2013-08-06 21:36:07 PDT
Created attachment 786726 [details] [diff] [review]
null pointer when resetting a resampler

fix valgrind error - null pointer when destroying a speex resampler
Comment 16 [:jesup] on pto until 2016/8/1 Randell Jesup 2013-08-06 21:37:52 PDT
Comment on attachment 786726 [details] [diff] [review]
null pointer when resetting a resampler

Either...
Comment 17 [:jesup] on pto until 2016/8/1 Randell Jesup 2013-08-06 21:38:23 PDT
Found via valgrind; also seen on one build on Try
Comment 18 [:jesup] on pto until 2016/8/1 Randell Jesup 2013-08-06 22:38:04 PDT
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.
Comment 19 [:jesup] on pto until 2016/8/1 Randell Jesup 2013-08-07 07:37:48 PDT
Try at https://tbpl.mozilla.org/?tree=Try&rev=cfe192c1875b (Release+patch)
and https://tbpl.mozilla.org/?tree=Try&rev=a170a10bb2e2 (Inbound+patch) are both green.
Comment 20 bryandonnovan 2013-08-07 08:48:06 PDT
I verified the fix in https://tbpl.mozilla.org/?tree=Try&rev=cfe192c1875b on OS 10.8.4 and Win7.
Comment 21 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-08-07 10:22:15 PDT
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 22 [:jesup] on pto until 2016/8/1 Randell Jesup 2013-08-07 10:39:17 PDT
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
Comment 23 Alex Keybl [:akeybl] 2013-08-07 10:45:38 PDT
Why not just back out bug 886886 on release? Seems like the safest option.
Comment 24 [:jesup] on pto until 2016/8/1 Randell Jesup 2013-08-07 11:01:34 PDT
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.
Comment 25 [:jesup] on pto until 2016/8/1 Randell Jesup 2013-08-07 11:13:11 PDT
https://tbpl.mozilla.org/?tree=Try&rev=73d2ea96c2fd
Comment 26 Timothy B. Terriberry (:derf) 2013-08-07 11:48:23 PDT
(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.
Comment 28 bhavana bajaj [:bajaj] 2013-08-07 12:24:01 PDT
(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.
Comment 29 Timothy B. Terriberry (:derf) 2013-08-07 13:34:55 PDT
(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.
Comment 30 [:jesup] on pto until 2016/8/1 Randell Jesup 2013-08-07 13:46:01 PDT
The backout-on-release Try is largely complete and green
Comment 32 Lukas Blakk [:lsblakk] use ?needinfo 2013-08-07 21:40:05 PDT
Tracking for a potential 23.0.1 - we'll discuss whether to forward fix or backout to return to known-good state offline.
Comment 33 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-08-08 16:35:48 PDT
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.
Comment 34 [:jesup] on pto until 2016/8/1 Randell Jesup 2013-08-08 16:48:41 PDT
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.
Comment 35 Lukas Blakk [:lsblakk] use ?needinfo 2013-08-12 15:42:25 PDT
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
Comment 36 [:jesup] on pto until 2016/8/1 Randell Jesup 2013-08-12 17:26:16 PDT
Created attachment 789280 [details] [diff] [review]
backout speex resampler fixes for 44100Hz mics (bug 886886)
Comment 37 [:jesup] on pto until 2016/8/1 Randell Jesup 2013-08-12 17:26:30 PDT
Created attachment 789281 [details] [diff] [review]
backout 44000/44100Hz kludge removals (bug 886886)
Comment 38 [:jesup] on pto until 2016/8/1 Randell Jesup 2013-08-12 17:28:51 PDT
Sorry, I had run the Try but not uploaded them.  I still prefer the fix, but the backout is ready.   (clean backout)
Comment 39 [:jesup] on pto until 2016/8/1 Randell Jesup 2013-08-12 17:33:38 PDT
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
Comment 40 Andrey Kovalenko 2013-08-12 23:30:22 PDT
(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.
Comment 41 [:jesup] on pto until 2016/8/1 Randell Jesup 2013-08-13 03:31:46 PDT
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.
Comment 42 Lukas Blakk [:lsblakk] use ?needinfo 2013-08-13 16:13:46 PDT
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.
Comment 44 Ioana (away) 2013-08-16 07:41:54 PDT
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
Comment 45 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-08-16 09:27:19 PDT
(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.
Comment 46 Maire Reavy (On PTO July 23rd to 31st) 2013-08-16 09:53:16 PDT
(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.
Comment 47 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-08-16 10:17:37 PDT
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?
Comment 48 bryandonnovan 2013-08-16 17:38:38 PDT
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"
Comment 49 Terrell Kelley 2013-08-16 22:25:37 PDT
(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.
Comment 50 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-08-19 11:20:04 PDT
(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.
Comment 51 Cornel Ionce [QA] (:cornel_ionce) 2013-08-20 06:15:39 PDT
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
Comment 52 [:jesup] on pto until 2016/8/1 Randell Jesup 2013-08-20 07:09:45 PDT
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
Comment 53 Cornel Ionce [QA] (:cornel_ionce) 2013-08-21 06:42:40 PDT
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.
Comment 54 Paul Adenot (:padenot) 2013-08-21 06:47:26 PDT
Cornel, how did you proceed to record the audio?
Comment 55 Cornel Ionce [QA] (:cornel_ionce) 2013-08-21 06:50:32 PDT
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.
Comment 56 [:jesup] on pto until 2016/8/1 Randell Jesup 2013-08-23 12:32:56 PDT
*** Bug 901539 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.