1052923 - [B2G][Dialer][Touch Tone] Dialer keypad does not produce touch tone sounds

Status: VERIFIED FIXED

(Core :: Web Audio, defect)

Description

[Martin Shuman [:Marty]]

Attachment: logcat-Dailer.txt

When the user inputs numbers into the Dialer keypad, the phone does not produce the touch tone sounds.

This occurs even if all the volume settings are all the way up, and if the user disables and re-enables Dial Pad sounds in the 'other sounds' menu of the Sound Settings.

This does NOT affect touch tone sounds being used during a call.

Repro Steps:
1) Update a Flame to 20140812040201
2) Load the Dialer app.
3) Input a phone number into the Dial pad


Actual:
Touch tone sounds are not heard when dialing a number.


Expected:
Touch tone sounds are heard when dialing a number.

Environmental Variables:
Device: Flame Master
Build ID: 20140812040201
Gaia: e78e62125eb43c3a28cdc047987ba54430694a2f
Gecko: b53c2753ce9a
Version: 34.0a1 (Master)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Note: This occurs on both 512MB and 319MB memory.

Keywords: Touch tone, keypad, dial pad, dialer, sound.


Repro frequency: 100%
See attached: logcat

Comment 1

[Martin Shuman [:Marty]]

This issue does NOT occur on Flame 2.0.
Dialer keypad produces touch tone sounds properly.

Environmental Variables:
Device: Flame 2.0
Build ID: 20140812000205
Gaia: 1144cdc3a544f81c9bf71598aba1cb67d6c95a29
Gecko: 6495a7bd61ed
Version: 32.0 (2.0)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 2

[Peter Bylenga [:PBylenga]]

[Blocking Requested - why for this release]:
Bad functional regression of a core feature.

Requesting a window.

Comment 3

[Gabriele Svelto [:gsvelto] (PTO)]

I've just checked this out on my Flame and on my Hamachi. When testing with a new gaia version on top of an old gecko (one week old roughly) the touch tones were still working, updating gecko to today's build caused the bug to reproduce. I suspect a gecko regression here because of this and because there haven't been any changes in the past two weeks to the dialer that might have caused this.

Comment 4

[Gabriele Svelto [:gsvelto] (PTO)]

Definitely a Gecko regression, I'm bisecting right now.

Comment 5

[Gabriele Svelto [:gsvelto] (PTO)]

Bisection led me to bug 1046592, precisely this commit:

https://hg.mozilla.org/mozilla-central/rev/7d830df01b20

Comment 6

[Pi Wei Cheng [:piwei] (inactive)]

Window found in comment 5; removing window-wanted tag.

Comment 7

[Paul Adenot (:padenot)]

The patch you pointed is a bustage fix on bug 1027713, that moves our volume handling to the server side of the audio IPC channel (so that, for example, volume change latency is reduced, and we eliminate some possible glitches on "play -> pause -> mute -> resume" kind of scenario).

Ok, now that the context is there, some possible reasons for this behavior:

(1) I completely screwed up my formula, it goes outside the boundaries for some reasons (a couple __android_log_print should tell you that), and Android decides to mute, just because ;
(2) I completely screwed up my formula, we need to do a log interpolation instead of a linear interpolation to compensate for the fact that decibels are a log unit to keep the same behavior. Looking at the file tone_player.js, and specifically at the workaround using <audio>, the volume is 0.5, that would very well, explain what we are seeing here.

I'm betting on (2), this is probably a one liner, it'll be fixed tomorrow, or worst case the day after.

Gabriele, would you mind taking this one (I can provide any help you need, we are in the same timezone) ? I'm in the process of landing bug 848954, that is the real fix for the dialer (improve latency and remove glitches from Web Audio API) and it's somewhat time consuming.

Comment 8

[Gabriele Svelto [:gsvelto] (PTO)]

(In reply to Paul Adenot (:padenot) from comment #7)
> The patch you pointed is a bustage fix on bug 1027713, that moves our volume
> handling to the server side of the audio IPC channel (so that, for example,
> volume change latency is reduced, and we eliminate some possible glitches on
> "play -> pause -> mute -> resume" kind of scenario).
> 
> Ok, now that the context is there, some possible reasons for this behavior:
> 
> (1) I completely screwed up my formula, it goes outside the boundaries for
> some reasons (a couple __android_log_print should tell you that), and
> Android decides to mute, just because ;
> (2) I completely screwed up my formula, we need to do a log interpolation
> instead of a linear interpolation to compensate for the fact that decibels
> are a log unit to keep the same behavior. Looking at the file
> tone_player.js, and specifically at the workaround using <audio>, the volume
> is 0.5, that would very well, explain what we are seeing here.
> 
> I'm betting on (2), this is probably a one liner, it'll be fixed tomorrow,
> or worst case the day after.
> 
> Gabriele, would you mind taking this one (I can provide any help you need,
> we are in the same timezone) ?

Sure, I'll handle it.

> I'm in the process of landing bug 848954,
> that is the real fix for the dialer (improve latency and remove glitches
> from Web Audio API) and it's somewhat time consuming.

Yes, I was following that bug and I'm eager for it to land so I can rip out the ugly-looking, three-meters-high pile of workarounds I put in place in tone_player.js :-)

Comment 9

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: [PATCH] Fix the volume conversion in millibels

This patch fixes two issues with the volume conversion in millibels, one which was introduced in bug 1046592 and the other one which was already present in the code:

- The first issue is that the conversion between the linear volume and millibels is logarithmic and not linear so I've reinstated the logarithmic conversion (100*dB -> 100 * 20 * log(volume))

- The second issue is that both log10f() and lroundf() might silently return the wrong value if their inputs were out of range. In the case of log10f() a negative input will yield a NaN which would make all following calculations erroneous. An input of 0 on the other hand yields -Infinity which previously was silently converted by lroundf() into 0 (the error was notified via the FP exception mechanism which we didn't check, this caused a linear volume of 0 to be interpreted as 0dB which was the maximum instead of mute). Similarly lroundf() would silently return 0 if the input was larger than a long integer's limits. I've properly guarded the code against all of these conditions to ensure that whatever volume we get yields a sensible result and no silent FP errors are thrown.

Comment 10

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: [PATCH v2] Fix the volume conversion in millibels

Slightly different patch with the clamping done entirely with straight-line code, this is somewhat simpler and more readable IMO.

Comment 12

[Gabriele Svelto [:gsvelto] (PTO)]

This is ready for landing. We don't have unit-tests covering this so I just made a Try build to ensure this compiles properly everywhere it might be used:

https://tbpl.mozilla.org/?tree=Try&rev=79a0c8575561

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5cdce85e2328

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5cdce85e2328

Comment 15

[Martin Shuman [:Marty]]

This issue is verified fixed in Flame 2.1 and 2.2

Flame 2.1

Environmental Variables:
Device: Flame 2.1 (319MB)
BuildID: 20141011000201 (Full Flash)
Gaia: f5d4ff60ffed8961f7d0380ada9d0facfdfd56b1
Gecko: d813d79d3eae
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Flame 2.2

Environmental Variables:
Device: Flame 2.2 Master (319MB)
BuildID: 20141011040204 (Full Flash)
Gaia: 95f580a1522ffd0f09302372b78200dab9b6f322
Gecko: 3f6a51950eb5
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 35.0a1 (2.2 Master)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Dialer keypad produces touch tone sounds properly.