Attachment 8335570 [details] demonstrates a change in dialer behavior. Bug 937505 and bug 919215 cover the delay, but this bug is about the change in ADSR envelopes. I'll attach plots of the tones before and after the change. The changes are believed to be due to https://github.com/mozilla-b2g/gaia/commit/add12c96c3e9281baa7f483f7ba751ce63df5749

This sounds like it starts and ends suddenly.

I expect the lack of an attack curve in the web audio gain node implementation is due to the issues described at http://lists.w3.org/Archives/Public/public-audio/2013OctDec/0286.html In short, the Web Audio API is not designed to be used in this way. Unfortunately it doesn't provide an elegant way to do this, and envelopes need to be applied to each sample in JS. One option might be to use AudioBufferSourceNodes scheduled to play end on end. These will only merge seemlessly when the buffer sampleRate is the same as that of the AudioContext. The MediaSource interface looks like it is better designed for the purposes here.

There are some unexplained observations here: Before add12c96 the code specified kKeyToneFrames = 1200 _sampleRate = 8000 implying a duration of 150 ms (consistent with comment), but the recorded tones are 80 - 100 ms (With the web audio implementation the short tones of 60 to 85 ms instead of 250 could be explained by missing the start of the tone, but the observations before change show gradual attack and release portions, so apparently the ends of the tone are not truncated.) The web audio implementation shortened the release phase from 50 to 25 ms, but in the recordings this is only 10 ms, and there is a sudden drop in amplitude at the beginning of this phase.

The Web Audio API does provide what we want here if we create a new AudioContext for each tone. If the context is created and the oscillators are started before returning to the event loop then they can start at time zero, and so phases can be scheduled accurately. However, in Gecko (and Chromium too, I think), each (online) AudioContext has its own system audio stream and these may take some time to create, which was what introduced the latency in bug 919215. (In reply to Karl Tomlinson (:karlt) from comment #5) > The MediaSource interface looks like it is better designed for the purposes > here. I spoke with Matthew and he pointed out that MediaSource is intended for use with encoded media and so does not seem right here. > One option might be to use AudioBufferSourceNodes scheduled to play end on > end. > These will only merge seemlessly when the buffer sampleRate is the same as > that of the AudioContext. And this is only possible if the buffers are scheduled far enough in the future that they will not fall off schedule. It doesn't work for a do-this-as-soon-as-possible approach. Short presses could be produced by a single AudioBuffer containing ADSR-shaped samples at a sample rate that can represent the frequencies. These could be used with AudioBufferSourceNode on which start(0) is called. The audio will start at the beginning of the next processing block, which is as soon as possible, and will have the appropriate intervals between effects. OfflineAudioContext provides a callback API to render this buffer, but it may be better to generate it in JS. However long presses couldn't be handled this way because we don't know how long the note will be - it stops on finger release.

The best option I think is to use a BufferSource with a ramp for the transient effects on the gain AudioParam. This leaks due to bug 897796, but that can be worked around.

Comment on attachment 8338351 [details] asr-2.html > // To ramp to full volume (1), set the destination volume, then subtract a > // ramp down to zero. > gain.gain.value = 1; > rampSource = context.createBufferSource(); > rampSource.buffer = rampUp; > rampSource.start(); > rampSource.connect(gain.gain); Actually, this is not quite right. The spec says "The implementation must make gain changes to the audio stream smoothly, without introducing noticeable clicks or glitches", which in practice means that changes from setting AudioParam.value have a transition. If the change to gain.value does not cause a step function change, then the initial summed value of the AudioParam will not be zero. I think setValueAtTime() can be used with currentTime or any value in the past (including 0) to ensure the change is a step function, but this should be tested.

This is recorded on a Unagi, tapping 1 2 3 4 5 6 7 8 9 in order. The taps can be heard before the tones. The tones start and stop abruptly.

This is with the changes at https://github.com/karlt/gaia/tree/943138, which include the fix for bug 942751. I'll submit a pull request after some tidying up.

(In reply to Karl Tomlinson (:karlt) from comment #6) > There are some unexplained observations here: > The web audio implementation shortened the release phase from 50 to 25 ms, > but > in the recordings this is only 10 ms, and there is a sudden drop in > amplitude at the beginning of this phase. This is probably due to stopping the tone on touchEnd, addressed in bug 942751 comment 1.

Comment on attachment 8345567 [details] pull request Thanks for the clear comments it helped a lot, and it's working well! r=me (but it probably needs rebasing)

(In reply to Etienne Segonzac (:etienne) from comment #14) Thanks. > (but it probably needs rebasing) These changes were on top of 6725558, which was merged in https://github.com/mozilla-b2g/gaia/commit/577f46a9a8f3f16540c007fa420bd5630c5f2ae3 so I assume this can be merged without conflicts. I pulled from mozilla-b2g and "git merge 943138" was successful: Merge made by the 'recursive' strategy. apps/communications/dialer/js/tone_player.js | 146 +++++++++++++++++++++++++++++++++-------------- 1 file changed, 104 insertions(+), 42 deletions(-) There are some seemingly unrelated test failures at https://travis-ci.org/mozilla-b2g/gaia/builds/15256047 I'm learning my way around here, so please let me know if there is something more I need to do.

triage: blocking a 1.3+ blocker. 1.3+

(In reply to Karl Tomlinson (:karlt) from comment #15) > I'm learning my way around here, so please let me know if there is something > more I need to do. No problem. We usually do 1 commit by bug, can you rebase and squash those into one. (this will also kick off a new travis build)

P1 since this is 1.3+ (needed to improve the Dialer).

Rebased and squashed, and Travis threw the winning numbers.

Perfect, thanks! https://github.com/mozilla-b2g/gaia/commit/3330e45ecd79a12241663db8bd1799cdfccd1451

Uplifted 3330e45ecd79a12241663db8bd1799cdfccd1451 to: v1.3: 16e9309a10388efe7c78fa8bd2c9144b089f3740