Closed Bug 1015519 Opened 6 years ago Closed 5 years ago
Creating a new Audio
Context causes a brief clicking sound
Sorry I don't have a good test case for this, as I'm otherwise occupied at the moment, but I've noticed that when creating a new AudioContext and hooking up an Audio object to it, I get a brief clicking sound. This happens on both B2G and Firefox desktop (at least on nightly). I noticed this in the past couple of weeks on my branch here: <https://github.com/jimporter/gaia/tree/ringtones-merge>. Here's the relevant block of code: <https://github.com/jimporter/gaia/blob/ringtones-merge/apps/ringtones/js/tone_player.js#L13>.
(In reply to Jim Porter (:squib) from comment #0) > I've noticed that when creating a new AudioContext and > hooking up an Audio object to it, I get a brief clicking sound. Does the sound contain a single click or multiple clicks? Does this happen even if there is only silence from the newly connected Audio object? Or is this when the Audio begins playing, like bug 974232? > I noticed this in the past couple of weeks Do you know whether this is a regression? i.e. do you know whether the click was absent some time before these weeks?
(In reply to Karl Tomlinson (needinfo?:karlt) from comment #1) > (In reply to Jim Porter (:squib) from comment #0) > > I've noticed that when creating a new AudioContext and > > hooking up an Audio object to it, I get a brief clicking sound. > > Does the sound contain a single click or multiple clicks? A single click. > Does this happen even if there is only silence from the newly connected > Audio object? Yes. You get a click immediately when the Audio object is connected to the AudioContext. > Or is this when the Audio begins playing, like bug 974232? This can happen if I set the audio to begin playing immediately. > > I noticed this in the past couple of weeks > > Do you know whether this is a regression? > i.e. do you know whether the click was absent some time before these weeks? I am pretty sure this regressed sometime 2-3 weeks ago. I'll try to check.
fwiw, I've seen (heard) this as well, but at thought at the time it was my home audio system acting up. I think this is a regression, because it surprised me at the time.
I noticed this yesterday too!
Jim - Is there a way to reproduce this in an E2E fashion with the ringtones app? STR could help us get a window here.
STR 1. Go to settings -> Sound 2. Ensure volume is turned up - it seems like the click is louder at higher settings 3. Tap on either "Manage Ringtones" or the Ringer settings
Using the STR in comment 6, check if this reproduces on 1.4.
The dupe answered the question in comment 7 - this is only present in 2.0 because 1.4 does not have this feature. The video is dupe though proves that the click is obvious though, so we need this fixed.
blocking-b2g: backlog → 2.0?
Priority: -- → P1
A window is going to be hard to do w/o a reduced test case. Jim - Can you give QA a test case to use to do a window against?
Here's a test case. Note: You don't need a device for this, or even B2G! It reproduces just fine on Firefox Desktop (Linux x86_64 nightly, at least).
Oh, note that if you can't reproduce it, you might need to close the tab and reopen it, or even restart the browser, but it happens at least 90% of the time for me.
Using the test tool on comment 11, I was able to find a window EARLIER than when 'Manage Ringtones' feature was introduced. Mozilla inbound regression window: Last Working Environmental Variables: Device: Buri Build ID: 20140411035728 Gaia: 1368d716072adf308e1b435ac828f97545a045f1 Gecko: 783c5013dbec Version: 31.0a1 (Master) Firmware Version: v1.2device.cfg First Broken Environmental Variables: Device: Buri Build ID: 20140411072825 Gaia: 1368d716072adf308e1b435ac828f97545a045f1 Gecko: 16803b53b8a5 Version: 31.0a1 (Master) Firmware Version: v1.2device.cfg Gaia is the same on both builds, leading this to a Gecko issue. Gecko pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=783c5013dbec&tochange=16803b53b8a5
QA Whiteboard: [QAnalyst-Triage?]
(In reply to Pi Wei Cheng from comment #13) > Using the test tool on comment 11, I was able to find a window EARLIER than > when 'Manage Ringtones' feature was introduced. This is unsurprising, since we first noticed this while I was still working on bug 960329. Thanks for the analysis!
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Maybe bug 982490 broke this?
(In reply to Jason Smith [:jsmith] from comment #15) > Maybe bug 982490 broke this? Paul - What do you think?
I think that you are right, I came to the same conclusion, and posted in this bug, but I just saw it mid-aired. This should not be hard to track down, we should just make sure all the data that go into the "AudioStream::Write" calls are good (e.g., pure silence when we just get an AudioContext and do nothing). The bug is probably in AudioSegment::WriteTo.
Paul -- since this is a 2.0 blocker and likely a regression from earlier your bug, I'm going to assign you as the bug owner. If you need additional help tracking this down, please let me know. Thanks.
Assignee: nobody → paul
Depends on: 982490
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][lead-review+]
This was caused by the fact that we would write |GetDuration()| frames without checking how many frame we actually wrote in the buffer, writing uninitialized data to the AudioStream.
Attachment #8442755 - Flags: review?(roc)
Attachment #8442755 - Flags: review?(roc) → review+
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
https://hg.mozilla.org/releases/mozilla-aurora/rev/34d3a6ced61d Can we land a test for this regression?
Paul, could you fill the uplift request?
Comment on attachment 8442755 [details] [diff] [review] Don't write uninitialized buffers to the AudioStream in AudioSegment::WriteTo. r= [Approval Request Comment] Bug caused by (feature/regressing bug #): 982490 User impact if declined: Short click when creating an AudioContext. Testing completed (on m-c, etc.): baked on m-c for a while, verified manually on two FxOS devices + Firefox desktop on Linux (I could repro on all those devices/platforms without this patch). Risk to taking this patch (and alternatives if risky): low risk, the cause is well understood and the patch is not complicated String or IDL/UUID changes made by this patch: none
Attachment #8442755 - Flags: approval-mozilla-beta?
Attachment #8442755 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.