Closed
Bug 1184482
Opened 9 years ago
Closed 9 years ago
[B2G] Keep the audio channel competing even if there is no ringer sound in the vibration mode
Categories
(Firefox OS Graveyard :: AudioChannel, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: alwu, Assigned: alwu)
References
Details
Attachments
(1 file, 3 obsolete files)
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•9 years ago
|
||
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•9 years ago
|
||
Alastor and I discussed that offline. We think we can just let Callscreen play the ringer when the volume is zero.
Assignee | ||
Comment 3•9 years ago
|
||
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 :)
Attachment #8635225 -
Flags: feedback?(etienne)
Comment 4•9 years ago
|
||
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.
Attachment #8635225 -
Flags: feedback?(etienne) → feedback?(gsvelto)
Comment 5•9 years ago
|
||
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.
Attachment #8635225 -
Flags: feedback?(gsvelto) → feedback+
Assignee | ||
Updated•9 years ago
|
Summary: [B2G] Add new event to handle special audio competing case → [B2G] Keep the audio channel competing even if there is no ringer sound in the vibration mode
Comment 6•9 years ago
|
||
Assignee | ||
Comment 7•9 years ago
|
||
Comment on attachment 8638300 [details]
[gaia] alastor0325:bug-1184482-silenceRingtone > mozilla-b2g:master
Hi, Gabriele,
Could you help me review this patch?
Very appreciate :)
Attachment #8638300 -
Flags: review?(gsvelto)
Comment 8•9 years ago
|
||
Comment on attachment 8638300 [details]
[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.
Attachment #8638300 -
Flags: review?(gsvelto) → review-
Assignee | ||
Comment 9•9 years ago
|
||
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
Attachment #8635225 -
Attachment is obsolete: true
Attachment #8638300 -
Attachment is obsolete: true
Attachment #8639250 -
Flags: review?(gsvelto)
Comment 10•9 years ago
|
||
Comment on attachment 8639250 [details]
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.
Attachment #8639250 -
Flags: review?(gsvelto) → review-
Assignee | ||
Comment 11•9 years ago
|
||
New patch, wait for the try-server result.
https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=781dee5aa8f539e8b49fbdcb28f120e8848433ef
Attachment #8639250 -
Attachment is obsolete: true
Assignee | ||
Comment 12•9 years ago
|
||
Assignee | ||
Comment 13•9 years ago
|
||
Comment on attachment 8639781 [details]
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 :)
Attachment #8639781 -
Flags: review?(gsvelto)
Updated•9 years ago
|
Status: NEW → ASSIGNED
Comment 14•9 years ago
|
||
Comment on attachment 8639781 [details]
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.
Attachment #8639781 -
Flags: review?(gsvelto) → review+
Assignee | ||
Comment 15•9 years ago
|
||
Thanks Gabriele for reviewing, I already modify the codes :)
---
Hi, Evan,
Could you help me merge this patch into master?
Thanks!
Flags: needinfo?(evan)
Comment 17•9 years ago
|
||
Weird, the Gij38 task is not finished yet. I'll land the code after the task is also good.
Comment 18•9 years ago
|
||
Now all tasks are passed. Let's land the code.
Comment 19•9 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•