872054 - [Inari] Busy line tone is heard through the speaker

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Dialer, defect)

Description

[isabelrios]

Device Inari/Ikura
Build id: 20130514090719
AU V1.01.00.01.019.105

PROCEDURE
1. From Inari call to a busy number (or call to a number which rejects your call)
2. Listen to the busy line tone

EXPECTED
The busy line tone is heard with a regular volume

ACTUAL
The busy line tone is heard through the speaker

This is a critical bug for certification. Adding dep and nominating it

Comment 1

[Anshul]

Busy tone is played by Gaia so adding gtorodelvalle who implemented this feature.

Comment 2

[Alex Keybl [:akeybl]]

Please let us know if you're not the right assignee gtorodelvalle.

Comment 3

[Germán Toro del Valle (:gtorodelvalle)]

I am, I am ;-) Thanks!

Comment 4

[Germán Toro del Valle (:gtorodelvalle)]

Attachment: Associated PR.

Sorry for the delay but this bug almost drove my crazy ;-999

The patch included solves the problem. This is, the busy tone is now played on the ear speaker. Anyhow, I observed a colateral issue I was trying to circumvent. The problem is that if the user makes a call to a busy line and just afterwards dials a new number, the key tones are played on the ear speaker for some seconds until they are played by the main speaker (so to call it).

(Technical info for Etienne) In advance, this has no sense since the Audio object created by tone_player.js (this file is linked by oncall.html) on which I set the mozAudioChannelType to 'telephony' should be destroyed once the oncall.html window is closed, but it seems to have influence on the other Audio object created also by tone_player.js (this time as a consequence of the linking by index.html, the main Communication app HTML file). I have tried almost any imaginable option (forcing the creation and destruction of the Audio objects, forcing to have only 1 Audio object in the whole Communications app, setting and unsetting the mozAudioChannelType, etc.) with no success. Should we file a bug to the platform? It seems there are no related bugs as far as I have been able to see.

Comment 5

[Germán Toro del Valle (:gtorodelvalle)]

Platform bug 813426 could be related with this one although in our case we are supposed to instantiate Audio objects in distinct scopes. And as I said, after some seconds everything starts to work as expected playing the key tones on the main speaker.

Comment 6

[Etienne Segonzac (:etienne)]

Comment on attachment 750505 [details]
Associated PR.

Wait, don't we want _all_ the tones played during a call to be played on the telephony channel?

If yes I'd much rather have a simple |setChannel| method on the |TonePlayer| and call it _once_ when we initialize it if we're "oncall".

If not, we need to keep the precedent mozAudioChannelType value in a private instance variable in order to put back the channel as it was once the sequence is done playing.

Comment 7

[Germán Toro del Valle (:gtorodelvalle)]

Hi Etienne, yeah in fact that was a previous option I tried: a setAudioChannelType() method on TonePlayer but it did not convince me ;-) The main reason was to avoid creating the Audio object instance if it is not needed and postpone its creation all we can (we create an Audio object instance for the ring tone but that it a different one). Right now, in the case of oncall.js, the Audio object instance is created when TonePlayer.playSequence() is called (via the TonePlayer.ensureAudio() function). If we use the setChannel() function we should be sure that the audio is created there if it does not exists (it is as simple as invoking ensureAudio() from setChannel() before setting the channel :-) ) In oncall.js we would have something like this:

TonePlayer.setChannel('telephony'); // To postpone its creation as much as possible.
TonePlayer.playSequence(sequence);

Instead of:
TonePlayer.playSequence(sequence, 0, 'telephony'); // As proposed in my patch.

Do you still prefer the setChannel option? No problem since it is straight-forward ;-)

Comment 8

[Germán Toro del Valle (:gtorodelvalle)]

As commented on IRC, we have included a setChannel() function in TonePlayer. The PR is already updated including this changes: https://github.com/mozilla-b2g/gaia/pull/9818

Comment 9

[John Ford [:jhford] CET/CEST Berlin Time]

I merged this onto master, before I realised that it wasn't r+'d
master: 57568a2

Backed out with: 94e5adf

Sorry for the noise!

Comment 10

[Etienne Segonzac (:etienne)]

Comment on attachment 750505 [details]
Associated PR.

Commented on github.
We misunderstood each other on IRC there is still a small change to make to this patch.

Please don't forget to set the r?etienne flag here once the patch is updated otherwise it doesn't get on my radar.

Comment 11

[Etienne Segonzac (:etienne)]

Comment on attachment 750505 [details]
Associated PR.

r=me (d21626efd42174d25046daf338c0916b711c1222)

Comment 12

[Etienne Segonzac (:etienne)]

https://github.com/mozilla-b2g/gaia/commit/619d7ec87dce1aaecb82b6100662c304183077bd

Comment 13

[John Ford [:jhford] CET/CEST Berlin Time]

v1-train: 115b5f8d
v1.0.1: 812d606

Comment 14

[(inactive after 6/18) Alive Kuo [:alive]]

This patch causes a regression:
The tone would output through speaker when earphone is plugged and "not during call".
What we should do here isn't set ringer channel to it but the normal channel.

The channel of dtmf tone has been set to ringer in bug A and roll back in bug B because the same reason. Sorry I forgot the bug number..

Creating a followup bug to fix this per partner request.

Comment 15

[(inactive after 6/18) Alive Kuo [:alive]]

BTW,
you couldn't set an audio element to different channel if you'd set it once. This is a gecko limitation. So if you want to 'change' the mozAudioChannelType, you should destroy it and have a new one.

Comment 16

[Germán Toro del Valle (:gtorodelvalle)]

Good to know! ;-) Thanks :alive