Enpei Reporter
	Description • 11 years ago

When press dialer keypad no matter during the call or not, sometimes the tone plays with broken sound and it's choppy.

* Build Number                
Gaia:     c6b4cc05b2de6884a652c1c5ab8401216ffa46c1
Gecko:    78b3dbc50a8cddea792b6c2870c0bfbe3726335c
BuildID   20130917062051
Version   26.0a1

* Reproduce Steps
Press dialer keypad like in attached video.

* Expected Result
Tone can be played smoothly.

* Actual Result
Sometimes it's choppy.

* Occurrence rate
High.

	Enpei Reporter
	Comment 2 • 11 years ago

add Koi? since this can easily be found during voice call as well.

	Jason Smith [:jsmith]
	Comment 3 • 11 years ago

I'm seeing this in the log all over the place:

09-17 15:43:16.303 E/GeckoConsole( 1200): [JavaScript Warning: "HTTP "Content-Type" of "text/html" is not supported. Load of media resource app://communications.gaiamobile.org/dialer/index.html failed." {file: "app://communications.gaiamobile.org/dialer/index.html#keyboard-view" line: 0}]

	gbennett
	Comment 4 • 11 years ago

- Does not Repro -
Environmental Variables
Device: Buri 1.2 mozRIL
Build ID: 20130916040205
Gecko: http://hg.mozilla.org/mozilla-central/rev/c4bcef90cef9
Gaia: a0079597d510ce8ea0b9cbb02c506030510b9eeb
Platform Version: 26.0a1

This does not repro on yesterdays build.

	Preeti Raghunath(:Preeti)
	Comment 5 • 11 years ago

David,

Please reassign since it is a dialer issue.

	Enpei Reporter
	Comment 6 • 11 years ago

Hi,

I tried v1.2 on both buri and Wasabi, only wasabi has this problem. This may device specific problem.

Wasabi:
Gaia:     7b6147372cbf560744a02be50e0a862a825caef6
Gecko:    ec0e3d60bebe2fe582d99d94d3bf4d61e52b083c
BuildID   20130923140309
Version   26.0a2

	Enpei Reporter
	Comment 7 • 11 years ago

remove regression keyword since v1.2 buri has no such issue.

	Francis Lee [:frlee]
	Comment 8 • 11 years ago

according to 9/25 triage result: this issue needs to be reproduced on real device, since it happens only on wasabi.

	David Scravaglieri [:scravag]
	Comment 9 • 11 years ago

per comment 8

	Ben Kelly [:bkelly, not reviewing]
	Comment 10 • 11 years ago

I see this pretty consistently on my buri.  I will try to take a video of it later today.

Paul indicated that pops and clicks in dialer tones are probably due to bug 848954.

Jason, if this is showing on buri for 1.3, do you think it should be a blocker?

	Jason Smith [:jsmith]
	Comment 11 • 11 years ago

(In reply to Ben Kelly [:bkelly] from comment #10)
> I see this pretty consistently on my buri.  I will try to take a video of it
> later today.
> 
> Paul indicated that pops and clicks in dialer tones are probably due to bug
> 848954.
> 
> Jason, if this is showing on buri for 1.3, do you think it should be a
> blocker?

Are we sure this is the same issue? Enpei originally said she couldn't reproduce this on Buri. I'll mark qawanted to check this on Buri as well.

	Jason Smith [:jsmith]
	Comment 12 • 11 years ago

Okay I've confirmed this on Buri as well. Yeah we would need to block on this.

	Ben Kelly [:bkelly, not reviewing]
	Comment 13 • 11 years ago

Note, if bug 848954 is too far out we could consider using the media files provided by ux:

  https://mozilla.app.box.com/s/lrani6d3ejspdxnso6wu/1/864533338

The "liquid" are short tones and the "clear" are long tones.  I'm trying to get the original wav's now to convert to opus.

I might try hacking up a prototype using web audio's buffered data source or the html5 cloneNode approach keyboard uses.

Etienne, thoughts?

	Preeti Raghunath(:Preeti)
	Comment 14 • 11 years ago

1.3+

	David Scravaglieri [:scravag]
	Comment 15 • 11 years ago

Anthony, please take a look at this and reassign if needed.

	Ben Kelly [:bkelly, not reviewing]
	Comment 16 • 11 years ago

Here is a video showing the problem:

  https://www.dropbox.com/sc/qnmck7c2qinqpnt/sWOSddy4gB

Its a bit hard to hear the pops in the video unfortunately.  I think maybe the video compression is masking it a bit.  You can still hear it if you listen closely, though.  It typically occurs on the first button press after launching or re-opening dialer.

Paul, what do you think?

	Etienne Segonzac (:etienne)
	Comment 17 • 11 years ago

(In reply to Ben Kelly [:bkelly] from comment #13)
> Note, if bug 848954 is too far out we could consider using the media files
> provided by ux:
> 
>   https://mozilla.app.box.com/s/lrani6d3ejspdxnso6wu/1/864533338
> 
> The "liquid" are short tones and the "clear" are long tones.  I'm trying to
> get the original wav's now to convert to opus.
> 
> I might try hacking up a prototype using web audio's buffered data source or
> the html5 cloneNode approach keyboard uses.
> 
> Etienne, thoughts?

We moved away from files a long time ago, it's probably worth the shot to revisit this.
But we need to keep in mind that we still need the possibility to play the tone continuously  (as long as the key in pressed, while on a call) IIRC looping a file wasn't doing the trick.

Frankly I wouldn't do it for 1.3 (and I wouldn't block here).

	David Scravaglieri [:scravag]
	Comment 18 • 11 years ago

Per Etienne's comment not a blocker.

	Anthony Ricaud (:rik)
	Comment 19 • 11 years ago

What I'm hearing and seeing in the two videos is that the sounds are choppy when the device is busy. In Ben's video, it's just after launching the dialer so we are probably lazy loading a lot of files. There's not much we can do here imho.

In Enpei's video, it's worse when the phone number font size changes. That's when our font size matcher does more reflows than usual to compute the right size. I've seen a prototype from Vivien yesterday that uses canvas to do that computation, hence avoiding reflows. So that should help.

	Jason Smith [:jsmith]
	Comment 20 • 11 years ago

(In reply to David Scravaglieri [:scravag] from comment #18)
> Per Etienne's comment not a blocker.

Sorry, but we can't ship choppy audio here when typing in dial keypad. This will easily block in a certification process. If that means we need to back out the Web Audio integration in the Dialer for 1.3, then so be it.

	Jason Smith [:jsmith]
	Comment 21 • 11 years ago

Also adding mwcdemo2014 as this impacts pretty much every on floor demo at MWC.

	Etienne Segonzac (:etienne)
	Comment 22 • 11 years ago

(In reply to Jason Smith [:jsmith] from comment #20)
> (In reply to David Scravaglieri [:scravag] from comment #18)
> > Per Etienne's comment not a blocker.
> 
> Sorry, but we can't ship choppy audio here when typing in dial keypad. This
> will easily block in a certification process. If that means we need to back
> out the Web Audio integration in the Dialer for 1.3, then so be it.

Jason, can you please get a 1.1 buri a play with the keypad a bit.
It's *not* pretty, but certified, and field tested at MWC.

We all want the feature to get better, let's do this properly instead of winging big code changes on a deadline.

	Jason Smith [:jsmith]
	Comment 23 • 11 years ago

(In reply to Etienne Segonzac (:etienne) from comment #22)
> (In reply to Jason Smith [:jsmith] from comment #20)
> > (In reply to David Scravaglieri [:scravag] from comment #18)
> > > Per Etienne's comment not a blocker.
> > 
> > Sorry, but we can't ship choppy audio here when typing in dial keypad. This
> > will easily block in a certification process. If that means we need to back
> > out the Web Audio integration in the Dialer for 1.3, then so be it.
> 
> Jason, can you please get a 1.1 buri a play with the keypad a bit.
> It's *not* pretty, but certified, and field tested at MWC.

I'll check this.

> 
> We all want the feature to get better, let's do this properly instead of
> winging big code changes on a deadline.

I never mentioned about big code changes - I'm arguing for a backout. We already have too many problems as is with the dial keypad with the Web Audio integration as is (CS blockers, severe regressions, etc), so the better option is to backout at this point if there's large changes required here to make the actual fix.

	Etienne Segonzac (:etienne)
	Comment 24 • 11 years ago

(In reply to Jason Smith [:jsmith] from comment #23)
> I never mentioned about big code changes - I'm arguing for a backout.

Of an old patch, with multiple patches built on top of it, using an old API which hasn't been QA'd on the gecko+hardware we're targeting and already had [1] issues [2] in the past.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=779914
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=832842

	Jason Smith [:jsmith]
	Comment 25 • 11 years ago

I've confirmed this does not reproduce on a 1.2 build without the web audio patches. This confirms this is caused by the introduction of Web Audio into 1.3.

	Jason Smith [:jsmith]
	Comment 26 • 11 years ago

(In reply to Etienne Segonzac (:etienne) from comment #24)
> (In reply to Jason Smith [:jsmith] from comment #23)
> > I never mentioned about big code changes - I'm arguing for a backout.
> 
> Of an old patch, with multiple patches built on top of it, using an old API
> which hasn't been QA'd on the gecko+hardware we're targeting and already had
> [1] issues [2] in the past.
> 
> [1] https://bugzilla.mozilla.org/show_bug.cgi?id=779914
> [2] https://bugzilla.mozilla.org/show_bug.cgi?id=832842

That does not matter - you should always plan for a backup plan for backouts with any feature you land. There's no excuse for shipping an obvious regression to a user in a basic phone use case (in fact - this is as basic as it gets). Buri is the reference device for 1.3, so this is being QAed on target hardware supported in 1.3 timeframe.

I'm +ing this again because we don't time to wait until Tuesday triage this again because MWC folks want fixes on these type of issues asap.

	Etienne Segonzac (:etienne)
	Comment 27 • 11 years ago

Putting a + on this bug or **** each other off (no hard feelings, 1.3 isn't easy on anybody) is unlikely to solve anything here, and surely isn't making the change less risky.

The current 1.3 is considered unacceptable by QA, and the suggested change is considered too risky by devs.

Product should definitely get the last shipped version (this is 1.1) to compare and see if what was acceptable then is on par with what we have right now.

(But in the meantime, what time is it in the US? Sleep is probably in order :))

	David Scravaglieri [:scravag]
	Comment 28 • 11 years ago

Preeti, we need a *release driver* decision on this bug.

	Beatriz Rodríguez [:brg]
	Comment 29 • 11 years ago

IMHO, this is the kind of issues that we can face in a close future because they will be reported during the certification process. Any improvement in current behaviour will reduce the risk of being found.

	Etienne Segonzac (:etienne)
	Comment 30 • 11 years ago

You can hear a "poc" after literally each key.

(But the latency is good :))

	Etienne Segonzac (:etienne)
	Comment 31 • 11 years ago

for comparison

	Jason Smith [:jsmith]
	Comment 32 • 11 years ago

(In reply to David Scravaglieri [:scravag] from comment #28)
> Preeti, we need a *release driver* decision on this bug.

We already made a decision in triage. This was deemed as a blocker already. The snake is dead here - why are you flipping flags?

	Tony Chung [:tchung]
	Comment 33 • 11 years ago

Team, this is a highly impacted user area.  It's been researched that found to regression and affects 1.3 builds.  Finally, this is the Dialer App!   We can't be having crackling and lag tapping on the Highest visible app.
  
This needs to be taking in 1.3.  We can work with you if you need more testing, but please fix this.

	Jason Smith [:jsmith]
	Comment 34 • 11 years ago

(In reply to Etienne Segonzac (:etienne) from comment #31)
> Created attachment 8368498 [details]
> Recording of the tone player on a 1.3 buri
> 
> for comparison

For the record - those videos aren't representing the bug here. The sound I'm hearing on 1.3 sounds like the dial keypad sound is chopped in half when I type sometimes.

	Chris Lee [:clee]
	Comment 35 • 11 years ago

Hi everyone, 

Happy to help chime in here. 

I read through all the comments and perspectives here and here's the Product team's take - we view this as a blocker for the following reasons:

a) certification blocker per comment 29 -- we have direct input from our partners
b) regression from pre-1.3 builds.  Unfortunately, the recordings in comment 30 and 31 aren't fully representative of what's happening based on the 1.3 builds I'm hearing.  
c) lastly, the dialer is one of those apps that will *always* be used so we need to keep a high bar of quality and watch the regressions closely (easier said than done I know).  

Can someone help enumerate the options we have here?  Thanks.

	Preeti Raghunath(:Preeti)
	Comment 36 • 11 years ago

From relman perspective, we'll block on this for 1.3

	Etienne Segonzac (:etienne)
	Comment 37 • 11 years ago

(In reply to Chris Lee [:clee] from comment #35)
> Can someone help enumerate the options we have here?  Thanks.

We don't have one patch that we can be backed out and make everything ok.
So we need a custom patch anyway.

* We can rewrite the current TonePlayer to use mozAudio
* We can figure out what's wrong with the current implementation and fix it
* We can go forward with Ben's file-based proposal and see if it beats the 2 previous options

I have no preference since I don't know what exactly we want to fix.
The sound quality? The latency?

If it's the latency why did we close https://bugzilla.mozilla.org/show_bug.cgi?id=937505#c73 in the first place?

	Ben Kelly [:bkelly, not reviewing]
	Comment 38 • 11 years ago

Roc also suggested bumping up the priority of the MSG thread in bug 848954 comment 14.

	Jason Smith [:jsmith]
	Comment 39 • 11 years ago

(In reply to Etienne Segonzac (:etienne) from comment #37)
> (In reply to Chris Lee [:clee] from comment #35)
> > Can someone help enumerate the options we have here?  Thanks.
> 
> We don't have one patch that we can be backed out and make everything ok.
> So we need a custom patch anyway.
> 
> * We can rewrite the current TonePlayer to use mozAudio
> * We can figure out what's wrong with the current implementation and fix it
> * We can go forward with Ben's file-based proposal and see if it beats the 2
> previous options
> 
> I have no preference since I don't know what exactly we want to fix.
> The sound quality? The latency?
> 
> If it's the latency why did we close
> https://bugzilla.mozilla.org/show_bug.cgi?id=937505#c73 in the first place?

Latency isn't the issue here. The issue being seen here is chopped audio, not delayed audio.

	Jason Smith [:jsmith]
	Comment 40 • 11 years ago

FYI - this is confirmed to reproduce on a Buri device & ZTE Open device running 1.3. But I can't reproduce this on a TCL Soul device. It might be hardware dependent.

	Ben Kelly [:bkelly, not reviewing]
	Comment 41 • 11 years ago

Jason, do you see any difference in delay between the buri and the sora?

	Jason Smith [:jsmith]
	Comment 42 • 11 years ago

(In reply to Ben Kelly [:bkelly] from comment #41)
> Jason, do you see any difference in delay between the buri and the sora?

Sora actually works fine - I don't hear any delay & any chopped audio. Buri I hear occasionally a sound that's chopped in half (sound + nothing + sound).

	Ben Kelly [:bkelly, not reviewing]
	Comment 43 • 11 years ago

Hmm, ok.  I thought perhaps there was increased buffering in the gonk layer for JB or something.  That would explain no broken tones here and increased audio delay for the keyboard in bug 962932.  But it sounds like thats not the case.

If we are under-running, it could just be that the sora CPU is faster.

	Jason Smith [:jsmith]
	Comment 44 • 11 years ago

(In reply to Jason Smith [:jsmith] from comment #42)
> (In reply to Ben Kelly [:bkelly] from comment #41)
> > Jason, do you see any difference in delay between the buri and the sora?
> 
> Sora actually works fine - I don't hear any delay & any chopped audio. Buri
> I hear occasionally a sound that's chopped in half (sound + nothing + sound).

Actually, I'm not fully right here. Chris noticed when testing the Soul device in run of me that the chopped sound was still present, but it was more faint on that device. On a Buri device in comparison, had a far louder chopped sound.

	David Flanagan [:djf]
	Comment 45 • 11 years ago

I'm running a 1.4 nightly from Friday on my hamachi phone. I can hear the audio problem that Jason describes 100% of the time on the first dialer pad tone after launching the dialer app.  I can't ever get it to repeat as it does in the attached video. But if I kill and restart the dialer, the first tone is always "chopped" as Jason describes.

	David Flanagan [:djf]
	Comment 46 • 11 years ago

On my 1.4 hamachi, I can only see this bug on the first tone when the dialer is first launched.  This one-line patch fixes that bug for me by generating a 1ms sound when the keypad is first initialized. I suppose this somehow gets the web audio hardware or software warmed up and ready to go.

I'm not proposing this as a serious fix, but maybe it is good enough for MWC, in a pinch.

I think that this is a Web Audio bug and we ought to have gecko or gonk engineers working on it.  I don't think this is a Dialer bug.

	David Flanagan [:djf]
	Comment 47 • 11 years ago

CJ,

Is there anyone on your team who could look into this?

	Gabriele Svelto [:gsvelto] Assignee
	Comment 48 • 11 years ago

I can look into this in the afternoon, I've already worked on a similar problem

	Etienne Segonzac (:etienne)
	Comment 49 • 11 years ago

Comment on attachment 8369635 [details]
link to patch on github

I don't hear any changes with or without the patch, but again I'm apparently not very good at hearing this bug. And the patch looks perfectly safe!

IMHO, the change that has the most impact is simply to change the volume constants to:
```
const kMasterVolume = 0.3;
const kToneVolume = 0.5;
```
At a higher volume I can feel a slight vibration while holding the phone and taping of the keypad. At this volume it's gone.

	Jason Smith [:jsmith]
	Comment 50 • 11 years ago

Note - if you read the dependent bug & the bugs that block it, you'll notice that this happens cross-platform that can occur multiple times, not once. Just noting this as I don't think a fix here to prevent this on the first dialpad press is going to fix the root cause here.

The true fix here is in the dependent bug, but I don't know if that's going to happen in the 1.3 timeframe.

	Gabriele Svelto [:gsvelto] Assignee
	Comment 51 • 11 years ago

I'm looking into this but I really cannot get it to reproduce on my hamachi on master. The only time when I can hear the tones being interrupted is when I artificially generate some significant CPU load, for example by quickly switching apps while typing and then going back into the dialer and even then it doesn't always happen. I'm now doing a 1.3 build to verify if it reproduces there.

	Gabriele Svelto [:gsvelto] Assignee
	Comment 52 • 11 years ago

I've just tried this out on v1.3 and I still can't seem to be able to reproduce it on my hamachi. I can make the tone screech sometimes but only when typing numbers *really fast*, like rapidly hammering the same key over and over which doesn't seem like a very relevant scenario.

However I've noticed a weird issue: if I go to the settings app and switch between long and short tones w/o killing the dialer, once I go back to the latter the tones are systematically screechy and the app is consuming a fairly high amount of CPU even if it isn't doing anything apparently.

	Ben Kelly [:bkelly, not reviewing]
	Comment 53 • 11 years ago

Gabriele, what version of the OEM firmware are you using?  I've had no difficulty reproducing this with the 11/14 firmware.

	Marcus Cavanaugh [:mcav]
	Comment 54 • 11 years ago

I can confirm that David's one-line patch resolves the glitchy audio on a ZTE Open (master).

	Gabriele Svelto [:gsvelto] Assignee
	Comment 55 • 11 years ago

(In reply to Ben Kelly [:bkelly] from comment #53)
> Gabriele, what version of the OEM firmware are you using?  I've had no
> difficulty reproducing this with the 11/14 firmware.

I'm on the one labeled V1.2-device.cfg

	Gabriele Svelto [:gsvelto] Assignee
	Comment 56 • 11 years ago

I've managed to reproduce this consistently but only when tapping the first key after opening the dialer. I've taken a profile of the event and it seems that redrawing the number during the first tap takes over 4 times as long as for the following taps. I'm now trying to figure out why that's happening.

Another scenario in which I consistently get scratchy sound is if I type the same key, or two keys very quickly. Thing is, to hear the glitch you have to type so fast that nobody in his sane mind would do it in a normal situation. Still it would be nice to make it more robust even in this case.

	Etienne Segonzac (:etienne)
	Comment 57 • 11 years ago

(In reply to Marcus Cavanaugh [:mcav] <mcav@mozilla.com> from comment #54)
> I can confirm that David's one-line patch resolves the glitchy audio on a
> ZTE Open (master).

If we go with that fix we should do it not only onload but each time we get a mozvisibilitychange where we are visible.

Apparently going to the card view and back to the app helps reproduce the bug.

	Gabriele Svelto [:gsvelto] Assignee
	Comment 58 • 11 years ago

(In reply to Etienne Segonzac (:etienne) from comment #57)
> If we go with that fix we should do it not only onload but each time we get
> a mozvisibilitychange where we are visible.

I've tried modifying the patch to do this and it solves the glitch when tapping the first key after going into the cards view and back. Unfortunately some touches remain glitchy even in the presence of the patch.

> Apparently going to the card view and back to the app helps reproduce the
> bug.

Yes, in my testing every time I hear the glitch corresponds to a spike in CPU usage. Going into the cards view and back seems to cause a bunch of gralloc'd layers to be dropped (probably related to our memory saving features for background applications). Once we tap on the key again those surfaces a recreated causing a CPU spike which in turn causes the glitch. The origin of the CPU spike is however unimportant, we have to figure why it's interfering with the audio.

I'll try to see if the suggestion in bug 848954 comment 14 helps and if this is somehow related to bug 962932.

	Gabriele Svelto [:gsvelto] Assignee
	Comment 59 • 11 years ago

Quick update, I've played with raising the priority of the MediaStreamGrph thread but it doesn't seem to fix the issue.

However while testing this I noticed that all thread priorities for applications that have been in the background seem wrong. I'm trying to figure if this is really the case or if it's just a glitch in b2g-info. If we got our priorities wrong in the first place then that might be what's causing the issue here.

	Gabriele Svelto [:gsvelto] Assignee
	Comment 60 • 11 years ago

End-of-day update: bug 967440 would definitely help here because this bug seems to be mostly caused by bug 967587 (i.e. whenever you hear the tone being glitchy we're building a zillion layers which causes the CPU to get really loaded).

I haven't had much luck adjusting priorities of the various threads but mostly because they are all over the place on my phone. I'm not sure what's causing this, Nuwa being re-enabled is a possibility and I'd like to investigate it further.

	u459114
	Comment 61 • 11 years ago

ni to b2g media playback team

	Gabriele Svelto [:gsvelto] Assignee
	Comment 62 • 11 years ago

I've done further investigation on this bug but without getting to the root of the issue. For anybody who'd like to test this please apply the patch I've attached in bug 968297 before building gecko otherwise the various process priorities will be all jumbled up and the results won't be very meaningful.

Now back to the problem at hand, this is a profile of the communication app during the first two key strikes:

http://people.mozilla.org/~bgirard/cleopatra/#report=2b1b2651d313599b63a5d1d07773122579708d34

As you'll notice the first key press generates ~2.5x times the activity of the second one. If you zoom in you'll notice that a lot of time is spent in IPDL::PLayerTransaction::SendPGrallocBufferConstructor, most of it waiting for the main process. From what I could gather this is caused by the creation of layers but I'm still not sure how it's affecting the sound playback. Lowering the priority of the app doesn't seem to affect the issue so the CPU time spent there is probably not significant. I have to figure out where the time is being spent in the main process to figure out what's really causing this problem.

	Preeti Raghunath(:Preeti)
	Comment 63 • 11 years ago

Minusing from 1.3 as MWC blockers will not be counted against 1.3

Faramarz,

Please chime in with priorities.

	Faramarz Rashed
	Comment 64 • 11 years ago

This bug will not be counted against 1.3 as a blocker.  Needing this for MWC does not justify holding up our 1.3 release.

	Jason Smith [:jsmith]
	Comment 65 • 11 years ago

(In reply to Faramarz (:faramarz) from comment #64)
> This bug will not be counted against 1.3 as a blocker.  Needing this for MWC
> does not justify holding up our 1.3 release.

Sorry, but I disagree. This problem happens on general use of the device across multiple devices, is a confirmed regression, and happens on a basic phone use case. This bug is triaged independently of MWC & still is mission critical to ensuring a successful release of the phone & already has been deemed a cert blocker by one of our partners. Product also already ruled this as a blocker previously. This is not up for negotiation. Moving to back to blocking+.

	Gabriele Svelto [:gsvelto] Assignee
	Comment 66 • 11 years ago

I did some modifications to the app today and shrunk the amount of CPU required to reflow the phone number view (not resizing the font, skipping the ellipsis addition, etc...) and the first tone is still scratchy :-/ I need some help to fix this as I'm running out of leads as to what causes the problem.

At this stage I've got three different procedures to reproduce the issue:

1. Open the app for the first time
2. Press a key
3. The tone is scratchy

Or:

1. Open the app and go back to the homescreen or into the cardsview (the result is the same, the application is sent into the background)
2. Bring the application to the foreground again
3. Press a key
4. The tone is scratchy

Or:

1. Open the app
2. Start tapping keys *very fast*
3. Some tones become scratchy

All other situations including tapping keys at reasonable speed after the first one sound clear.

	Ben Kelly [:bkelly, not reviewing]
	Comment 67 • 11 years ago

Ugly but possible work around:  Use media files for dialer tones pre-call, and then web audio generator during the call.  It seems we only need the flexibility of the variable length tones during the call?

I don't expect anyone to like that option, but just throwing it out there if we're under the gun.

	Gabriele Svelto [:gsvelto] Assignee
	Comment 68 • 11 years ago

(In reply to Ben Kelly [:bkelly] from comment #67)
> I don't expect anyone to like that option, but just throwing it out there if
> we're under the gun.

It sounds like I might have to go there if I cannot find a solution. I've been timing the MediaStreamGraph thread and it seems to run with consistently even when the sound is scratchy; I cannot measure any significant delays in the thread's main loop so the problem must be happening at a lower level.

	Faramarz Rashed
	Comment 69 • 11 years ago

Jason, general problem across the board and a confirmed regression is a different story. In that case it should be resolved. However, if we're fixing *any* bug in 1.3 branch because we need it for MWC, I'm going to unblock us.

	Jason Smith [:jsmith]
	Comment 70 • 11 years ago

(In reply to Faramarz (:faramarz) from comment #69)
> Jason, general problem across the board and a confirmed regression is a
> different story. In that case it should be resolved. However, if we're
> fixing *any* bug in 1.3 branch because we need it for MWC, I'm going to
> unblock us.

I understand that. Sorry if that wasn't clear. Chris's summary in https://bugzilla.mozilla.org/show_bug.cgi?id=917193#c35 is a good perspective of why we blocked on this.

	Gabriele Svelto [:gsvelto] Assignee
	Comment 71 • 11 years ago

One further update. My original thought that this was caused by a CPU usage spike appears to be wrong. I've changed the code and removed all UI updates effectively dropping the CPU usage spike when typing a key to almost zero and I can still reproduce the issue.

The MediaStreamGraph thread reports a global underrun whenever I hear the scratchy tone, the issue is that it reports underruns even when the tones are sounding just fine. The only hint I have is that the bug is almost always reproducible when typing the key right after the app was brought to the foreground (either by going back to the homescreen or the cards view and then bringing up the app again).

I really need help from somebody more familiar with Web Audio's implementation to look into this as I'm having a hard time figuring out what exactly is going wrong. I've spent most of last Thursday and Friday peering into Web Audio's implementation but still haven't got a clue about what's causing this.

	Jason Smith [:jsmith]
	Comment 72 • 11 years ago

I believe the web audio work to fix this would be in the dependency - bug 848954. This bug was to determine if a Gaia workaround was possible, which based on the above comment, doesn't seem possible to do with Web Audio. The only option for a Gaia workaround here is what Ben suggests - use comment 67's proposed solution.

Does that make sense? Can we look into a solution by using comment 67's solution?

	Gabriele Svelto [:gsvelto] Assignee
	Comment 73 • 11 years ago

(In reply to Jason Smith [:jsmith] from comment #72)
> Does that make sense? Can we look into a solution by using comment 67's
> solution?

Yes, that's definitely doable but I would need someone to provide me with the appropriate audio assets.

	Ben Kelly [:bkelly, not reviewing]
	Comment 74 • 11 years ago

Gabriele, see comment 13 for a link to the ogg and mp3 files.  I would go with the "liquid" files.

I have the wav files in my email at the moment.  I want to encode those as .opus if we go ahead with this approach.  You can start with the .ogg files, though.

	Gabriele Svelto [:gsvelto] Assignee
	Comment 75 • 11 years ago

(In reply to Ben Kelly [:bkelly] from comment #74)
> Gabriele, see comment 13 for a link to the ogg and mp3 files.  I would go
> with the "liquid" files.

Excellent, I'll cook up a patch right away.

	Gabriele Svelto [:gsvelto] Assignee
	Comment 76 • 11 years ago

Here's a WIP patch that uses samples instead of generated waveforms for short tones. I'm using the OGG files provided in comment 13 as suggested by :bkelly.

This patch works, the tones are clean and the sound plays out cleanly without glitches...

BUT

... it sounds different, the samples probably use slightly different frequencies than what we use; the length is also slightly different. More importantly once we get into in-call mode with long tones then the glitchy tone for the first key we tap comes back and it sounds different than the sounds one has when not in a call.

	Gabriele Svelto [:gsvelto] Assignee
	Comment 78 • 11 years ago

Here's an alternative workaround that doesn't use pre-recorded tones. The logic here is to play a very short, virtually silent sound when the app is started or when it becomes visible. This ensures that the glitch we hear when tapping the first key appears in this particular sound and the following samples are clean and glitch-free. It works as some kind of "pipe cleaner" for issues that happen in the Web Audio implementation while playing the first sound.

I tested this in different situations and it seems to me a better solution than the pre-recorded sounds as it keeps the sounds consistent between long and short tones and between in-call and non in-call scenarios.

	Etienne Segonzac (:etienne)
	Comment 79 • 11 years ago

Hey Gabriele, thanks for the thorough work.

First of all, feedback+ for all the code changes, very clear and contained.

Spent some time playing with the different proposals and the current 1.3 for comparison.

* the pre-recorded tons are super weird :) I think the duration should probably be cut in half, and the descend phase (especially for the 4 key) sounds "weird".
But the latency is good, and I wasn't able to make the sound choppy.

* the "djf-hack" is nicely implemented, but I was able to make the sound choppy. (Go to cardview, go back to the dialer, wait 5 seconds, press a key.)

All in all I think we should move forward with the pre-recorded tones patch if we can find shorter plainer versions of those.

(we have wave base64 versions of tones in the gaia history https://github.com/mozilla-b2g/gaia/blob/be26bd842a25802e3120a771fe0c127dcf03c578/apps/dialer/dialer.html#L43 ;))

	Gabriele Svelto [:gsvelto] Assignee
	Comment 82 • 11 years ago

Here's an updated patch. I've taken the samples from the base64 encoded versions that you pointed out in the source, de-base64-ified them and then encoded them with opus as suggested by :bkelly. The resulting sounds are much closer to the ones we generate at runtime except for a slight "pinch" I can hear at the end of keys 4, 5, 6, 7, 8 and 9.

	Etienne Segonzac (:etienne)
	Comment 83 • 11 years ago

Comment on attachment 8374039 [details] [diff] [review]
[PATCH v2] Use samples instead of waveform generators for short audible tones

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

r=me with something like 

|sample.volume = kMasterVolume;| before playing the sample.

I'd also like to have QA give this patch a spin before we land/uplift, just to be sure the issue is considered fixed by everybody.

	Gabriele Svelto [:gsvelto] Assignee
	Comment 84 • 11 years ago

Now sets the volume properly, carrying over the r+ flag.

	Gabriele Svelto [:gsvelto] Assignee
	Comment 85 • 11 years ago

I've tested the patch in attachment 8374084 [details] [diff] [review] on my buri and it works fine there. Using samples guarantess there's no glitchy/scratchy sounds while typing. Jason do you now who could test this on more devices and ensure that the result is acceptable?

	Jason Smith [:jsmith]
	Comment 86 • 11 years ago

(In reply to Gabriele Svelto [:gsvelto] from comment #85)
> I've tested the patch in attachment 8374084 [details] [diff] [review] on my
> buri and it works fine there. Using samples guarantess there's no
> glitchy/scratchy sounds while typing. Jason do you now who could test this
> on more devices and ensure that the result is acceptable?

I can put someone on here to test this if you can give me a custom gaia branch with this patch. Can you point me at a custom gaia branch with this patch so that I can test this?

	Jason Smith [:jsmith]
	Comment 87 • 11 years ago

(In reply to Jason Smith [:jsmith] from comment #86)
> (In reply to Gabriele Svelto [:gsvelto] from comment #85)
> > I've tested the patch in attachment 8374084 [details] [diff] [review] on my
> > buri and it works fine there. Using samples guarantess there's no
> > glitchy/scratchy sounds while typing. Jason do you now who could test this
> > on more devices and ensure that the result is acceptable?
> 
> I can put someone on here to test this if you can give me a custom gaia
> branch with this patch. Can you point me at a custom gaia branch with this
> patch so that I can test this?

Or you could land this on master only, we verify it there, then uplift it post verification. Either or works.

	Gabriele Svelto [:gsvelto] Assignee
	Comment 88 • 11 years ago

(In reply to Jason Smith [:jsmith] from comment #86)
> (In reply to Gabriele Svelto [:gsvelto] from comment #85)
> I can put someone on here to test this if you can give me a custom gaia
> branch with this patch. Can you point me at a custom gaia branch with this
> patch so that I can test this?

You can use this branch: https://github.com/gabrielesvelto/gaia/tree/bug-917193-tones-glitch-workaround

(In reply to Jason Smith [:jsmith] from comment #87)
> Or you could land this on master only, we verify it there, then uplift it
> post verification. Either or works.

I'll hold it off for now as Etienne suggested in comment 83, just to reduce churn in case the patch needs to be changed.

	Jason Smith [:jsmith]
	Comment 89 • 11 years ago

Okay - I'll have someone test with that custom branch.

	gbennett
	Comment 90 • 11 years ago

Individual keys sound great minus the pound key which doesn't make a sound unless you're in a call. While you're in a call everything sounds great as well even if pressed rapidly. Are there specific cases you would like me to check or just overall wellness?

	Jason Smith [:jsmith]
	Comment 91 • 11 years ago

(In reply to gbennett from comment #90)
> Individual keys sound great minus the pound key which doesn't make a sound
> unless you're in a call. While you're in a call everything sounds great as
> well even if pressed rapidly. Are there specific cases you would like me to
> check or just overall wellness?

That's a bug if the pound key isn't playing sound. That works fine without this patch, so it sounds like this patch caused a regression.

	Gabriele Svelto [:gsvelto] Assignee
	Comment 92 • 11 years ago

(In reply to gbennett from comment #90)
> Individual keys sound great minus the pound key which doesn't make a sound
> unless you're in a call. While you're in a call everything sounds great as
> well even if pressed rapidly.

Apparently Gecko didn't like the fact that there was an hash character in the filename of the tone used by the hash/pound key. I've modified the patch to replace the character and now the key is working properly. Thanks for testing this!

> Are there specific cases you would like me to
> check or just overall wellness?

I think this is goo enough; I'll wait for Travis to turn green and merge/uplift the revised patch since it seems to work correctly now.

	Gabriele Svelto [:gsvelto] Assignee
	Comment 93 • 11 years ago

Travis is green, merged in gaia/master 3c2e748e4d57551527366eba00428a0f5ccc4012

	Gabriele Svelto [:gsvelto] Assignee
	Comment 94 • 11 years ago

Cherry picked to gaia/v1.3 58b3fb5feab32c324617d2a0847de0e8917a9218

	Gabriele Svelto [:gsvelto] Assignee
	Comment 95 • 11 years ago

Cherry picked to gaia/v1.3t 4575261ef9f0fdb74d7324440e57797ee74fa8ca