1184482 - [B2G] Keep the audio channel competing even if there is no ringer sound in the vibration mode

Status: RESOLVED FIXED

(Firefox OS Graveyard :: AudioChannel, defect)

Description

[Alastor Wu [:alwu]]

Here is one special case of the audio competing policy, although we don't play any new audio, we still need to interrupt the current playing audio.

One actual case is like that,

STR.
(1) User plays music during the vibration mode
(2) New incoming call
    - the ringtone would not be playback because of the vibration mode

Expected.
(3) The music is paused

Actual.
(3) The music is still playing
    - because we don't play any audio tag, so the system app can't handle the audio competing

---

Therefore, in preliminary draft, we would use the new event to notify the system app when we have a incoming call and the sound mode is vibration.

The event might be included two things,
(1) event name
(2) come-in audio channel type

ex. in above case, we might have these information in event
"activestatechangeinvibrationmode", "ringtone"

Comment 1

[Evan Tseng [:evanxd]]

I don't think we should have the new "activestatechangeinvibrationmode" event for the audio channel service module. If we do this, we might need to do some workaround there.

Audio channel service only focus on audio channel competing, it doesn't make sense to do audio channel competing when no audio channel is active/inactive.

I think we need to find a new way to fix this.

Comment 2

[Evan Tseng [:evanxd]]

Alastor and I discussed that offline. We think we can just let Callscreen play the ringer when the volume is zero.

Comment 3

[Alastor Wu [:alwu]]

Attachment: Draft : play silence ringtone during vibration mode

After discuss with Evan, we think this kind of implementation is not a good way.
Therefore, we propose the new method, that is to play a silent ringtone during the vibration mode.

In present design, the dialer agent would not play the audio element during the vibration mode [1]. I think we can still play this element at that time, because the audio volume has already been changed to ZERO. 

If we do that, the system app can know there are incoming ringtone so that it can pause other playing audio. Meanwhile, the ringtone is silence, it also accords with the goal of the vibration mode.

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/dialer_agent.js#L205

---

Hi, Etienne,
Sorry to bother you.
Could you give me some suggestion about this proposal?
Could we playback the silence ringtone during the vibration mode?
Very appreciate :)

Comment 4

[Etienne Segonzac (:etienne)]

Comment on attachment 8635225 [details] [diff] [review]
Draft : play silence ringtone during vibration mode

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

Sorry for the delay!
Redirecting to Gabriele who's currently doing refactorings in that area.

I think we might want to keep around a explicit "_silentPlayer" and of course add tests :), but I have no issue with playing a silent tone for incoming calls when in silence mode.

Comment 5

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8635225 [details] [diff] [review]
Draft : play silence ringtone during vibration mode

I've not been super-active in the system app but this looks like it does the trick. However to the casual (code) reader it's going to be quite unclear why we're acting like this, you should probably add a comment explaining why we're deliberately playing a silent sound.

Comment 6

[GitHub Autolander Bot]

Attachment: [gaia] alastor0325:bug-1184482-silenceRingtone > mozilla-b2g:master

Comment 7

[Alastor Wu [:alwu]]

Comment on attachment 8638300 [details] [review]
[gaia] alastor0325:bug-1184482-silenceRingtone > mozilla-b2g:master

Hi, Gabriele,
Could you help me review this patch?
Very appreciate :)

Comment 8

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8638300 [details] [review]
[gaia] alastor0325:bug-1184482-silenceRingtone > mozilla-b2g:master

Looking good but you need to update the unit-tests to accommodate for the change and to cover it. With your patch applied as-is one is failing right now. I've also left a nit in the PR.

Comment 9

[Alastor Wu [:alwu]]

Attachment: Play silent ringtone during the vibration mode.

Hi, Gabriele,
Could you help me review this patch?
Very appreciate :)

---

I modified the unit test, checking whether play the ringtone when the ringtone volume is ZERO.
Another modification is that the second call should vibrate when the vibration mode is on. It's defined in the UX spec [1]

[1] https://bug1068219.bmoattachments.org/attachment.cgi?id=8579177#12

Comment 10

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8639250 [details] [review]
Play silent ringtone during the vibration mode.

Nice catch with the vibration part, however you only changed the related unit-tests but not the actual code so they're failing now. Feel free to decide if you want to fix the vibration part in this bug or open another one and remove the updated unit-tests. The silent ringtone part on the other hand is fine.

Comment 11

[Alastor Wu [:alwu]]

Attachment: Play silent ringtone during the vibration mode.

New patch, wait for the try-server result.
https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=781dee5aa8f539e8b49fbdcb28f120e8848433ef

Comment 12

[Alastor Wu [:alwu]]

New try-server result.
https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=556d3016e2093904ce2539d6bfb17db54edcf52b

Comment 13

[Alastor Wu [:alwu]]

Comment on attachment 8639781 [details] [review]
Play silent ringtone during the vibration mode.

Hi, Gabriele,
Could you help me review this patch?
In this patch, we would call the _startAlerting() when there is the incoming call, and call _stopAlerting() when the call is connected.
I also modified the tests to accommodate the change.
Very appreciate :)

Comment 14

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8639781 [details] [review]
Play silent ringtone during the vibration mode.

Looks good to me, I've left only a few nits regarding how the test assertions are written but they're purely stylistic so I leave it up to you to decide if you want to apply those changes or land the patch as-is. Either way is fine.

Comment 15

[Alastor Wu [:alwu]]

Thanks Gabriele for reviewing, I already modify the codes :)

---

Hi, Evan,
Could you help me merge this patch into master?
Thanks!

Comment 16

[Evan Tseng [:evanxd]]

Sure, CI looks good, let's land the code.

Comment 17

[Evan Tseng [:evanxd]]

Weird, the Gij38 task is not finished yet. I'll land the code after the task is also good.

Comment 18

[Evan Tseng [:evanxd]]

Now all tasks are passed. Let's land the code.

Comment 19

[Evan Tseng [:evanxd]]

master: https://github.com/mozilla-b2g/gaia/commit/f3d7dc02b918f8c9c68d350d5f35b0199b9cd59a