It's a little slow to hear a sound when you enter a phone number. STR: 1. Launch dialer app 2. try to dial a number Expected Result: *. Hear a sound feedback when I tap a number Actual Result: *. There's around 0.5 seconds delay between the touch event and the sound Environment: Gaia: bcb4f94524ef02e9ef16b604d16c39e5a9c41281 Gecko: http://hg.mozilla.org/mozilla-central/rev/bc8c1eb0f2ba BuildID 20131111040200 Version 28.0a1

Al, is this a regression?

It looks like a regression but it's hard to figure out which commit breaks it. Tested it with the v1.1 buri devices from partner and it worked better than it does.

OK. I'm going to go ahead and add the regression keyword, though it sounds a little vague to the point where I'm not sure trying to window it would be productive.

(In reply to Geo Mealer [:geo] from comment #3) > OK. I'm going to go ahead and add the regression keyword, though it sounds a > little vague to the point where I'm not sure trying to window it would be > productive. We should probably check for how easy it for someone to notice while testing this. I'll add the window flag for now to see if a window is possible.

The issue is reproducible on 1.3 central, the tone sound is more loader and choppy The last working build: Device: Buiri 1.3 central BuildID: 20131106151707 Gaia: 94c9d69d38c6f1ca48c9739e328eb5e13646183b Gecko: 845e58b0cf7f Version: 28.0a1 The issue reproduces from: Device: Buri 1.3 Central build BuildID: 20131107040200 Gaia: 42bbe26a72e030faf07a6fc297f61a3a8ccda25b Gecko: 70de5e24d79b Version: 28.0a1 The issue doesn't reproduce on 1.2 and 1.1 builds

See the video in the attachment with two builds On a working build (orange device) the sound is clear, smooth without delays On a broken build (white) the sound is vague, louder and with a slight delay

triage: regression. 1.3+

We're now using WebAudio, so the fix probably won't come from the gaia side. That said, we should definitely QA this again now that bug 919215 landed.

Etienne -- So, the dialer on B2G in v1.3 uses Web Audio? Was that a change between v1.2 and v1.3? (FWIW, I think Bug 919215 is unlikely to make a difference to this bug.) Paul, Karl -- This bug is a high P1 if the dialer is relying on Web Audio for v1.3. Are either of you able to look at this and repro on a B2G phone?

(In reply to Maire Reavy [:mreavy] from comment #9) > Etienne -- So, the dialer on B2G in v1.3 uses Web Audio? Was that a change > between v1.2 and v1.3? Yes (bug 927852).

(In reply to Etienne Segonzac (:etienne) from comment #10) > Yes (bug 927852). Thanks for the pointer, Etienne! I remember seeing that bug go by a few weeks ago and hadn't thought about its effect on the dialer. Does the dialer use Web Audio just to generate key tones, or does it use it for other sounds as well? Karl, Paul -- This bug is definitely a high P1 which we'll need to have a solution for in the next 2 weeks (by Dec 6), if possible. I'm going to assign this bug to Karl for now so it doesn't fall through the cracks. NOTE: We may get other bugs like this showing up since I'm not sure what other B2G apps now use Web Audio in v1.3 (which used to use the Moz Audio API in v1.2 and earlier).

Note that in case we don't get to resolve it back in time, we can resort to re-enabling the Moz Audio Data for 1.3. Which would be *very very sad*...

(In reply to :Ehsan Akhgari (needinfo? me!) from comment #12) > Note that in case we don't get to resolve it back in time, we can resort to > re-enabling the Moz Audio Data for 1.3. Which would be *very very sad*... Yeah, I thought the same thing. Reverting to Moz Audio is our very last resort. That's why this is a high P1 bug to fix ASAP. I'd rather fix than revert. (I know you feel the same way.) Do you know of any other default apps (other than the Dialer) that now depend on Web Audio? (I'm concerned about any important bugs we haven't yet found.)

Would it be possible to know which phone is affected? I'm in the process of landing a patch that _could_ mitigate this (by helping us to choose a proper latency value instead of the default, which is good for playback, not so good for real-time stuff). Also, would it be possible for someone to retry with a newer Gecko build? I've landed a patch a few days ago that should somewhat improve the situation, and the hash of the revision in comment 0 tells me that the reported did not have this specific patch. This patch is https://hg.mozilla.org/mozilla-central/rev/37153a22e87a.

(In reply to Maire Reavy [:mreavy] from comment #11) > Does the dialer use Web Audio just to generate key tones, or does it use it for other > sounds as well? Just the key tones.

(In reply to Maire Reavy [:mreavy] on PTO Nov 25-30 from comment #13) > (In reply to :Ehsan Akhgari (needinfo? me!) from comment #12) > > Note that in case we don't get to resolve it back in time, we can resort to > > re-enabling the Moz Audio Data for 1.3. Which would be *very very sad*... > > Yeah, I thought the same thing. Reverting to Moz Audio is our very last > resort. That's why this is a high P1 bug to fix ASAP. I'd rather fix than > revert. (I know you feel the same way.) Do you know of any other default > apps (other than the Dialer) that now depend on Web Audio? (I'm concerned > about any important bugs we haven't yet found.) Etienne will probably know better.

(In reply to :Ehsan Akhgari (needinfo? me!) from comment #16) > (In reply to Maire Reavy [:mreavy] on PTO Nov 25-30 from comment #13) > > (In reply to :Ehsan Akhgari (needinfo? me!) from comment #12) > > > Note that in case we don't get to resolve it back in time, we can resort to > > > re-enabling the Moz Audio Data for 1.3. Which would be *very very sad*... > > > > Yeah, I thought the same thing. Reverting to Moz Audio is our very last > > resort. And we still had a fair amount of latency using Moz Audio, let's not overestimate the gain here. > > That's why this is a high P1 bug to fix ASAP. I'd rather fix than > > revert. (I know you feel the same way.) Do you know of any other default > > apps (other than the Dialer) that now depend on Web Audio? (I'm concerned > > about any important bugs we haven't yet found.) > > Etienne will probably know better. Just the dialer keypad and the emergency dialer keypad (same use-case).

The delay symptom reported here is exactly what is expected from bug 919215. With the adjustment made for that bug, things could be much better. The latency may still not be quite as good as the moz audio API due to MSG/cubeb interaction (or it may be better). We'll have to do something like bug 848954 to be as good as we can here, but that is not likely to happen by Dec 6. I've filed bug 943138 and bug 942751 for other issues with the Web Audio tone player. Let's verify please that bug 919215 provided the necessary improvements in latency and reopen if not. If the remaining issues are still blockers, then we have the option of backing out the media.audio_data.enabled pref change on the branches used by b2g and using the old mozWriteAudio code until we have a satisfactory solution.

(In reply to Karl Tomlinson (:karlt) from comment #18) > The delay symptom reported here is exactly what is expected from bug 919215. > With the adjustment made for that bug, things could be much better. > The latency may still not be quite as good as the moz audio API due to > MSG/cubeb interaction (or it may be better). We'll have to do something > like bug 848954 to be as good as we can here, but that is not likely to > happen by Dec 6. > > I've filed bug 943138 and bug 942751 for other issues with the Web Audio > tone player. > > Let's verify please that bug 919215 provided the necessary improvements in > latency and reopen if not. > > If the remaining issues are still blockers, then we have the option of > backing out the media.audio_data.enabled pref change on the branches used by > b2g and using the old mozWriteAudio code until we have a satisfactory > solution. > > *** This bug has been marked as a duplicate of bug 919215 *** Are you sure this is a dupe of that bug? jesup's comments seem to imply the issues in that bug are Mac-specific. Note - we've already got qawanted here, so we don't need verifyme with it as well.

I'm sure bug 919215 is a bug on all platforms, and Paul observed it on PulseAudio as well as Mac. We won't know for certain how much of the problem bug 919215 was until someone tries a newer build.

(In reply to Jason Smith [:jsmith] from comment #19) > Are you sure this is a dupe of that bug? jesup's comments seem to imply the > issues in that bug are Mac-specific. > > Note - we've already got qawanted here, so we don't need verifyme with it as > well. Yes, sorry, this bug makes the situation better on all platform, the reporter just happen to observe this on a mac with a specific setup that made the situation much worse.

As per comment 8, I don't see any changes after the fix is landed for Bug 919215 on the 1.3 Central build. The keypad sound still with latency when dialing fast and the sound is vague Device: Buri 1.3 Central BuildID: 20131126052050 Gaia: 4ad796b196d468bdb231beba4392acbc90a74e96 Gecko: 99479edbee2a Version: 28.0a1

Reopening because the testing above seems to indicate there's a different bug here.

(With correct url this time.)

Al, can you tell me please whether you hear any difference in delay between the buttons in attachment 8339037 [details]? (They sound similar on the Otoro device I tested.)

it sounds the same to me.

Needs media.audio_data.enabled;true or a build with Gecko older than 9a82f8dbb719.

Viewing attachment attachment 8339708 [details] with Firefox Mobile on a GT-I8160 running Android 2.3.6: With 2013-11-06-03-02-00-mozilla-central-android 9ba3faa35c96, Audio mozWriteAudio() sounds the same as Audio play(), both with a delay of 0.6-0.7 seconds or more. Web Audio takes about 0.1-0.2 seconds longer. With 20131127030201 6ecf0c4dfcbe mozWriteAudio() is not longer available. Web Audio sounds the same as Audio play(), both with a delay of 0.6-0.7 seconds.

My Otoro did get into a state once where a few tings queued up with Web Audio before they played in turn, while Audio.play() seemed to work fine. I haven't worked out STR.

A couple patches have landed to help this, can someone retest?

As per comment 33, even when a couple patches have landed, the bug still reproduces on the latest Buri 1.3 central, the keypad sound is vague and with latency Device: Buri 1.3 Central BuildID: 20131205040201 Gaia: 1dd0e5c644b4c677a4e8fa02e50d52136db489d9 Gecko: 725c36b5de1a Version: 28.0a1 Firmware version: v1.2_20131115

This is not really actionable, can someone provide a video?

(In reply to Paul Adenot (:padenot) from comment #35) > This is not really actionable, can someone provide a video? How is this not actionable? It's the dialer. The STR in comment 0 is dirt simple to reproduce on a device.

I don't know what "vague" means, as it is certainly not a technical audio term (hence my request for a video). I have landed patches that improved the situation a lot, on my device, and other people agree. I don't know if the person is hearing something else, has higher expectations, has a bizarre device. With a video, I can estimate the latency myself, and answer all those questions.

Okay. Can you someone get an updated video of this bug?

An updated video about the issue, the white device shows the problem with the dialer, the sound is corrupted -choppy and delayed

It looks like latency is now better but still not as good as what was provided by mozWriteAudio. I don't know whether or not the remaining latency is reason enough to return to mozWriteAudio. My main concern with Web Audio latency is: [Fri, 18 Oct 2013] [14:12:57] <karl> roc: the MSG looks like it produces a number of frames calculated from the system clock. What corrects for differences between this and the number of frames that cubeb wants? [Fri, 18 Oct 2013] [14:13:19] <roc> nothing Bug 848954 is the plan for dealing with that. mozWriteAudio is not perfect either - see bug 943138 comment 6. I'll leave the decision to Etienne. I could address the other Web Audio dialer issues (bug 943138 and bug 942751) next week if the latency is not the major concern, but improving the latency further is going to take time.

Comment on attachment 8343646 [details] revert gaia to mozWriteAudio I think we should keep the WebAudio version. The gaia code is much cleaner, and the difference of performance is small enough. Again, it's not like we were super happy with the latency on the mozAudio version. I could be overruled by product but I'll demand a blind test first :)

I think we still have time to try to get this working with Web Audio. However, if push comes to shove, then I think we'll have to revert to mozAudio. I think we can make that decision six weeks into stabilization. I do think the sound quality issue based on hearing the video is quite obvious. I wouldn't be surprised if a partner tried to block certification due to sound quality regression here, as TEF has blocked certification before on dialer tone quality.

(In reply to comment #42) > I think we still have time to try to get this working with Web Audio. However, > if push comes to shove, then I think we'll have to revert to mozAudio. I think > we can make that decision six weeks into stabilization. FWIW doing this requires just a pref change on the platform side, so it should not be that difficult.

Do the sound quality issues affect the DTMF tones sent over the line at all? If so, I'd be consider about impact on bank access systems, conference call, voice mail, stuff like that.

er, concerned about impact.

I need to investigate a koi+ bug first, but I plan to spend some time helping out here next week. If there are any profiles or other investigation you would like me to do, please let me know. Adding a ni? to myself to keep this flagged in my dashboard.

(In reply to Geo Mealer [:geo] from comment #44) > Do the sound quality issues affect the DTMF tones sent over the line at all? > If so, I'd be consider about impact on bank access systems, conference call, > voice mail, stuff like that. Nope, the DTMF tones are generated by the modem.

(In reply to Karl Tomlinson (:karlt) from comment #40) > > I could address the other Web Audio dialer issues (bug 943138 and bug 942751) > next week if the latency is not the major concern, but improving the latency > further is going to take time. Karl -- I'd love for you to fix bug 943138 and bug 942751 as soon as you can (this week?). And I'll get help fixing the remaining dialer issues. Thanks!

I've looked at both videos showing this bug multiple times, and I need more info to understand and isolate exactly what the problem is. In particular, I need a video showing (1) "normal" phone dialing (not just super fast key presses) as well as showing (2) single, isolated key presses for both a working build and a non-working build. Thank you!

(In reply to Maire Reavy [:mreavy] from comment #49) > I've looked at both videos showing this bug multiple times, and I need more > info to understand and isolate exactly what the problem is. In particular, > I need a video showing (1) "normal" phone dialing (not just super fast key > presses) as well as showing (2) single, isolated key presses for both a > working build and a non-working build. Thank you! The recent video in comment 39 to my understanding already covers this.

(In reply to Jason Smith [:jsmith] from comment #50) > (In reply to Maire Reavy [:mreavy] from comment #49) > > I've looked at both videos showing this bug multiple times, and I need more > > info to understand and isolate exactly what the problem is. In particular, > > I need a video showing (1) "normal" phone dialing (not just super fast key > > presses) as well as showing (2) single, isolated key presses for both a > > working build and a non-working build. Thank you! > > The recent video in comment 39 to my understanding already covers this. I don't think it does. While the video is great (and much appreciated), the key presses on that video are very fast. (I've never dialed a phone number that fast in my life.) I want to observe what the behavior is with slower key presses -- as well as single, isolated key presses -- to characterize the start/stop behaviors of the tones, which is tough to do when the tones are fast and right on top of one another.

Alright. QA Wanted to a get video showing this bug with slow key presses on the dialer keypad.

I added some simple instrumentation. It looks like about 34ms between event spew starting and TonePlayer.start calling AudioContext.currentTime. Then it's about 200ms more until the first call to AudioStream::DataCallback produces nonzero data. This is on my Unagi, which does sound like it has high audible latency. We should be able to squeeze quite a bit out of that 200ms. However, to get really good latency we need improvements at cubeb level or below ... bug 879774 maybe.

In another run, it's 27ms from event spew to AudioContext.currentTime, then 103ms until the new streams are added to the MediaStreamGraph on the graph thread. We start writing to the AudioStream immediately but it's 63ms before AudioStream::DataCallback gets called to pick nonzero data. Total latency from JS to DataCallback is only 166ms in this case.

In fact, 140-180ms from JS to non-zero AudioStream::DataCallback is typical on my phone.

Logging the time intervals which DataCallback is being called, it's being called approximately once every 80ms. So that's another big chunk of latency right there.

OK, this is interesting. The delay from TonePlayer.start calling AudioContext.currentTime until we reach MediaStreamGraphImpl::RunInStableState (i.e. the end of that script run, where we send the batched MediaStreamGraph updates to the MSG thread) is consistently around 100ms.

The Audio Data API wouldn't have that latency since it doesn't do that batching. So we need to figure out what else the script is doing before it yields. Possibly we need to make that stuff run off a setTimeout so we can yield immediately after triggering the tone.

Yeah. We're spending maybe 7-10ms doing the actual Web Audio setup in TonePlayer.start and then a lot more time running JS and doing other things before the event handler returns and the Web Audio messages can actually be sent to the MSG thread. _touchStart in keypad.js calls telephony.startTone to play DTMF codes (no idea how long that takes). More importantly, via the onValueChanged handler it does contacts lookup for suggestions in suggestion_bar.js's _updateByContacts, which I expect is not particularly fast :-). So I think we should have the _touchStart handler make the DOM changes to highlight the pressed button, and call TonePlayer.start, and then yield, hanging the rest of its work off a setTimeout(0). We may even want to hang that work off a setTimeout(0) triggered by requestAnimationFrame, to ensure that the slow work happens after we've updated the rendering of the keypad.

(In reply to Maire Reavy [:mreavy] from comment #48) > Karl -- I'd love for you to fix bug 943138 and bug 942751 as soon as you can > (this week?). Yes, I expect to finish those off tomorrow.

The video with slow dialing

Comment on attachment 8345874 [details] [diff] [review] 0001-Bug-937505-Dialer-Keypad-Touch-Handling-Doing-the-ex.patch Here's a quick patch following up on Comment 59. Looks like it helps, but might be placebo. What do you think?

Comment on attachment 8345874 [details] [diff] [review] 0001-Bug-937505-Dialer-Keypad-Touch-Handling-Doing-the-ex.patch >- this._playDtmfTone(key); >+ setTimeout(function nextTick(self) { >+ self._playDtmfTone(key); >+ }, 0, this); > } I worry a bit about _playDtmfTone and _stopDtmfTone getting out of order. When not on a call, all that _playDtmfTone does is get navigator.mozTelephony. If this hasn't been got before then there may be some setup time there, but can that be moved inside the (this._onCall) block, to make _playDtmfTone a no-op in the usual case? >- this._updatePhoneNumberView('begin', false); >+ setTimeout(function(self) { >+ self._updatePhoneNumberView('begin', false); >+ }, 0, this); > }, This is safe, but perhaps may come with the expense of slowing the visual update. I think roc was suggesting still running most of _updatePhoneNumberView before yielding, but running this.onValueChanged off setTimeout().

(In reply to Karl Tomlinson (:karlt back 3 Jan) from comment #64) > Comment on attachment 8345874 [details] [diff] [review] > 0001-Bug-937505-Dialer-Keypad-Touch-Handling-Doing-the-ex.patch > > >- this._playDtmfTone(key); > >+ setTimeout(function nextTick(self) { > >+ self._playDtmfTone(key); > >+ }, 0, this); > > } > > I worry a bit about _playDtmfTone and _stopDtmfTone getting out of order. > When not on a call, all that _playDtmfTone does is get > navigator.mozTelephony. > If this hasn't been got before then there may be some setup time there, but > can that be moved inside the (this._onCall) block, to make _playDtmfTone a > no-op in the usual case? Good point, done. > > >- this._updatePhoneNumberView('begin', false); > >+ setTimeout(function(self) { > >+ self._updatePhoneNumberView('begin', false); > >+ }, 0, this); > > }, > > This is safe, but perhaps may come with the expense of slowing the visual > update. > I think roc was suggesting still running most of _updatePhoneNumberView > before yielding, but running this.onValueChanged off setTimeout(). I decided to keep it this way since the _updatePhoneNumberView is pretty heavy and delaying it doesn't delay the key highlight. Thanks for the feedback! Moving this for review.

quick 1.3+ review

Comment on attachment 8347279 [details] Gaia PR Code is ok but I'm not hearing any latency improvement. It would be nice to get some numbers after this lands to see if it really improves things.

The last gaia patch landed: https://github.com/mozilla-b2g/gaia/commit/3bd9d999d30f9b5b0f2369b36d03b3d328f646fb Can we consider this resolved?

QA Wanted - does this still reproduce on master given the landing in comment 68?

This issue still reproduces on the Buri 1.3 Build ID: 20140102004001 Gaia 01e9da49be2cc4bc134eeefc434740d572ec2246 SourceStamp 61f553e5db49 BuildID 20140102004001 Version 28.0a2

(In reply to Sarah Parsons from comment #70) > This issue still reproduces on the Buri 1.3 Build ID: 20140102004001 > > Gaia 01e9da49be2cc4bc134eeefc434740d572ec2246 > SourceStamp 61f553e5db49 > BuildID 20140102004001 > Version 28.0a2 Master, not 1.3. We need retest this on the 1.4.

This issue is not reproducing the Buri 1.4 Build ID: 20140106040201 Gaia 9a222ac02db176e47299bb37112ae40aeadbeca7 SourceStamp 14ac61461f2a BuildID 20140106040201 Version 29.0a1

Okay - that confirms this is fixed by the most recent patch then.

I was not able to uplift this bug to v1.3. If this bug has dependencies which are not marked in this bug, please comment on this bug. If this bug depends on patches that aren't approved for v1.3, we need to re-evaluate the approval. Otherwise, if this is just a merge conflict, you might be able to resolve it with: git checkout v1.3 git cherry-pick -x -m1 3bd9d999d30f9b5b0f2369b36d03b3d328f646fb <RESOLVE MERGE CONFLICTS> git commit

I don't know of other dependencies.

My bad, it was because of bug 922006. Since it's a refactoring we shouldn't uplift it. I pushed the resolved patch on v1.3: https://github.com/mozilla-b2g/gaia/commit/e1a62130ffb381c9ed4444051c0c70605a254faf