Closed
Bug 1079974
Opened 10 years ago
Closed 10 years ago
[Loop] Ringer should be played through the main speaker
Categories
(Firefox OS Graveyard :: AudioChannel, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: borjasalguero, Unassigned)
References
Details
(Whiteboard: [mobile app][blocking][tef-triage][loop in v1.1])
Attachments
(2 files, 1 obsolete file)
Sometimes is not working as expected, so we need to force to route properly the sound.
Reporter | ||
Updated•10 years ago
|
Assignee: nobody → borja.bugzilla
Reporter | ||
Comment 1•10 years ago
|
||
Attachment #8501857 -
Flags: review?(josea.olivera)
Comment 2•10 years ago
|
||
Please file bugs about the FxOS client under the Firefox OS product in the Gaia:Loop component. Thanks.
Component: Client → Gaia::Loop
Product: Loop → Firefox OS
QA Contact: anthony.s.hughes
Updated•10 years ago
|
Updated•10 years ago
|
Whiteboard: [mobile app][blocking][tef-triage] → [mobile app][blocking][tef-triage][patch available]
Updated•10 years ago
|
Status: NEW → ASSIGNED
Comment 3•10 years ago
|
||
Comment on attachment 8501857 [details]
Pull Request
I'm not able to reproduce the issue. The ringer audio channel is supposed to play the ringtone through the speaker (*always*) and if this is not happening we have a gecko bug around. The patch tries to fix the issue through the MozSpeakerManager API which forces to route the audio through the speaker.
Actually it forces to route when the phone audio mode is IN_CALL or IN_COMMUNICATION which happens *only* when we are playing through the telephony audio channel (not the ringer one).
Massimo, are you able to reproduce this issue? Thanks!
Attachment #8501857 -
Flags: review?(josea.olivera)
Flags: needinfo?(mbarone976)
Updated•10 years ago
|
Assignee: borja.bugzilla → josea.olivera
Updated•10 years ago
|
Attachment #8501857 -
Attachment is obsolete: true
Updated•10 years ago
|
Whiteboard: [mobile app][blocking][tef-triage][patch available] → [mobile app][blocking][tef-triage]
Comment 4•10 years ago
|
||
Had a chat with Andrea [:baku] about this issue yesterday. The plan is to debug with his help a bit next Monday. The issue is not also happening in the Loop app but in the Callscreen app as well. I'll change the component then. Basically on GSM/CDMA calls the RIL plumbing plays the audio on the telephony channel through the audio manager. In parallel the Callscreen app plays silence through Web Audio on the telephony audio channel as well. There are chances in which the ringer tone of the next GSM/CDMA call plays on the built-in earpiece instead of playing it on the speaker. That happens in the Loop app as well so after a WebRTC call there are chances in which the ringer tone of the next WebRTC call plays on the built-in earpiece. Apparently something's fishy here. Andrea pointed out to bug 1073615 but it doesn't fix the issue (not for me at least). I'll double-check today again.
I'll add to CC list some folks that might help and I left a ni? at Andrea as a reminder for the next Monday.
Assignee: josea.olivera → nobody
Component: Gaia::Loop → AudioChannel
Updated•10 years ago
|
Flags: needinfo?(amarchesini)
Updated•10 years ago
|
Flags: needinfo?(mbarone976)
Comment 5•10 years ago
|
||
Right after device receives an incoming webrtc call the Loop app plays silence through Web Audio on the telephony audio channel. Note it happens even before playing the ringtone. So if we start playing silence on the telephony channel and after that the app plays the ringtone on the ringer channel in parallel the ringtone goes into the built-in earpiece. I dropped the code for playing silence and the ringer goes into the speaker. Sadly we cannot remove that code as we need it to set the GSM call (if any) on hold. So the ringer doesn't go into the speaker if we play audio con the telephony audio channel through Web Audio. Andrea, how is that possible?
Comment 6•10 years ago
|
||
Flash 2.0 10/20 PVT build on flame
On ringing mode on loop application and try to run
#adb shell dumpsys media.audio_policy
===
AudioPolicyManager Dump: 0xb8be7f28
Primary Output: 2
A2DP device address:
SCO device address:
USB audio ALSA
Output devices: 00000003
Input devices: 000001c4
Phone state: 0 <--right
===
Outputs dump:
- Output 2 dump:
Sampling rate: 48000
Format: 00000001
Channels: 00000003
Latency: 80
Flags 00000002
Devices 00000001 <-- NG, it should be 2 normally.(speaker)
So the ringer would come out from earpiece.
The device choose logic is in android HAL and output device shouldn't keep in earpiece.
Comment 7•10 years ago
|
||
Do we force the phone state when we are in a loop call?
Flags: needinfo?(amarchesini) → needinfo?(josea.olivera)
Comment 8•10 years ago
|
||
(In reply to Andrea Marchesini (:baku) from comment #7)
> Do we force the phone state when we are in a loop call?
Yes, we do. Somehow actually. On bug 1006380 we added a few changes in the audio channel service and in the audio manager logic. The goal was to allow app using the telephony audio channel to switch between the earpiece and the speaker by using the MozSpeakerManager API. The MozSpeakerManager API allows to switch between them when the audio phone state is PHONE_STATE_IN_CALL or PHONE_STATE_IN_COMMUNICATION. The change added on bug 1006380 was to set the audio phone state to PHONE_STATE_IN_COMMUNICATION when the app is playing audio through the telephony audio channel.
Flags: needinfo?(josea.olivera)
Comment 9•10 years ago
|
||
I suspect that bug 1006380 can be the origin of this issue. Look at this piece of code:
https://mxr.mozilla.org/mozilla-central/source/dom/system/gonk/AudioManager.cpp#332
Here we check if we have an active telephony channel. But we have a webAudio context active when the ringer plays for the AudioChannel competing policy. So, that check will be always true and that'll always make the phone in PHONE_STATE_IN_COMMUNICATION state also when it's not.
Let me know if I'm wrong.
But, if this is correct I see 2 options:
1. we cannot use the telephony active state to change the PhoneState and we have to find a different approach.
2. we have to find a different solution for the audiochannel competing policy GSM vs WebRTC.
Comment 10•10 years ago
|
||
Andrea, the logs you requested me. I added the |adb shell dumpsys media.audio_policy| command output I ran during the test.
Comment 11•10 years ago
|
||
It looks like when b2g creates track as telephony and keep it alive cause this problem.
I try to modify the cubeb_stream_type ConvertChannelToCubebType() to avoid convert to telephony stream type and it works well..
I did a compare test on nexus 5 with skype, the result is:
=====Ringing:
Phone state: 1 <--- MODE_RINGTONE,
----------
- Output 2 dump:
Sampling rate: 48000
Format: 00000001
Channels: 00000003
Latency: 50
Flags 00000002
Devices 00000002
Stream volume refCount muteCount
00 0.063 00 00
01 0.032 00 00
02 0.026 01 00 <---STREAM_TYPE=RING
==== connected.
Phone state: 3 <--- MODE_IN_COMMUNICATION,
Outputs dump:
- Output 2 dump:
Sampling rate: 48000
Format: 00000001
Channels: 00000003
Latency: 50
Flags 00000002
Devices 00000001
Stream volume refCount muteCount
00 1.000 01 00 <---STREAM_TYPE= VOICE
01 0.084 00 00
02 0.007 00 00
Comment 12•10 years ago
|
||
In AudioPolicyManagerBase.cpp,
<https://android.googlesource.com/platform/hardware/libhardware_legacy/+/android-4.4.3_r1.1.0.1/audio/AudioPolicyManagerBase.cpp>
If the voice stream type is active, this module would prefer to select earpiece as the default output device.
ex: when stream voice call is active. (telephony), this function call would return STRATEGY_PHONE.
AudioPolicyManagerBase::routing_strategy AudioPolicyManagerBase::getStrategy() (L2277)
case AudioSystem::VOICE_CALL:
case AudioSystem::BLUETOOTH_SCO:
return STRATEGY_PHONE; then this function would like to choose earpiece.
AudioPolicyManagerBase::getDeviceForStrategy() (L2315)
...
would goto this case:default: // FORCE_NONE case, then fail into this line.
device = mAvailableOutputDevices & AUDIO_DEVICE_OUT_EARPIECE;
So if possible, I think we should avoid to create the CUBEB_STREAM_TYPE_VOICE_CALL stream type if device has not entered the Voip usecase.
Comment 14•10 years ago
|
||
(In reply to Randy Lin [:rlin] from comment #13)
> Hi Baku,
> Any ideas for that?
Unfortunately yes. We create a AudioContext('telephony') also when the call is not active.
This is done to know when the app is muted by the AudioChannelService but it seems this introduces this issue.
What I would propose is:
can we revert the AudioContext('telephony') hack and introduce a new API just for this priority "loop vs gsm" issue?
I'm thinking about something extremely simple like;
[Constructor(Window window, USVString name), Exposed=Window]
interface TelephonyAgent : EventTarget {
attribute EventHandler oninterruptbegin;
attribute EventHandler oninterruptend;
};
If any 'telephony' app creates this object with a different name, this can be used to count them, and send events when the visibility of the window changes (if we have more than 1 agent active).
Of course when oninterruptbegin, the app has to interrupt the call, or put it in hold state.
(probably oninterruptbeginneeded would be a better name...)
This can be an approach. It would take a couple of days to be implemented.
Another approach can be to introduce mozinterruptbegin/end in the telephony API. But there are some issues, exposed by mchen in bug 1016277
Flags: needinfo?(amarchesini) → needinfo?(jonas)
Comment 15•10 years ago
|
||
I had a call with Jonas about this issue. And here a better proposal:
At the moment we create a AudioContext('telephony') when the loop and gsm apps start. Instead doing this what about if:
1. we receive a GSM or loop call. The GSM or loop app goes in foreground and it just creates a MediaElement for the ringer using 'ringer' AudioChannel. We use onmozinterruptbegin/end on this ringer mediaElement to know if we had to drop the call.
2. The user answers the call, and we create a AudioContext('telephony') for the audio competing policy.
We use onmozinterruptbegin/end to know if we have to put the call in hold.
3. At this point we are still in a call. But we receive another call using a different protocol (if we were in loop, now it's a GSM or viceversa). The other app goes in foreground and it creates a MediaElement for the ringer, using the 'ringer' audio Channel. - At this point the ringer has to be 'softer' and this can be done using some existing API, or with some new API. We need to know if this API already exists... probably it doesn't.
4. The other call is still active because we didn't create any 'telephony' audio channel, so this is still the highest active audioChannel.
5. the user answers the new call. The app creates a AudioContext('telephony') and this will mute the other app (the active one). Then the previous call is set to hold and the new call will be active one.
I think that the removing of AudioContext('telephony') when the call is not actually active will fix this issue.
But I want to know if this is doable and I have covered all the possible corner-cases.
Flags: needinfo?(jonas) → needinfo?(josea.olivera)
Comment 16•10 years ago
|
||
(In reply to Andrea Marchesini (:baku) from comment #15)
> I had a call with Jonas about this issue. And here a better proposal:
Thank you guys for providing the output of that call here.
> At the moment we create a AudioContext('telephony') when the loop and gsm
> apps start.
We only do that for the Loop app. The callscreen one doesn't create it until the GSM call is connected. Actually we added this hack recently due a issue we had for the Loop app (see [1] please).
> Instead doing this what about if:
What you guys propose bellow is actually what we have implemented more or less.
> 1. we receive a GSM or loop call. The GSM or loop app goes in foreground and
> it just creates a MediaElement for the ringer using 'ringer' AudioChannel.
> We use onmozinterruptbegin/end on this ringer mediaElement to know if we had
> to drop the call.
>
> 2. The user answers the call, and we create a AudioContext('telephony') for
> the audio competing policy.
> We use onmozinterruptbegin/end to know if we have to put the call in hold.
>
> 3. At this point we are still in a call. But we receive another call using a
> different protocol (if we were in loop, now it's a GSM or viceversa). The
> other app goes in foreground and it creates a MediaElement for the ringer,
> using the 'ringer' audio Channel. - At this point the ringer has to be
> 'softer' and this can be done using some existing API, or with some new API.
> We need to know if this API already exists... probably it doesn't.
>
> 4. The other call is still active because we didn't create any 'telephony'
> audio channel, so this is still the highest active audioChannel.
>
> 5. the user answers the new call. The app creates a
> AudioContext('telephony') and this will mute the other app (the active one).
> Then the previous call is set to hold and the new call will be active one.
>
> I think that the removing of AudioContext('telephony') when the call is not
> actually active will fix this issue.
Yes, I thought I'd shared this findings with you. The issue gets fixed. Read bellow please.
> But I want to know if this is doable and I have covered all the possible
> corner-cases.
It's doable for sure but we need to find a fix for the issue we reported on bug 1069793. On bug 1069793 the issue was basically a race condition. We needed a way to force setting the GSM call on hold as soon as possible because the gUM request we made for showing a local video preview was always failing. I really prefer to revert the patch from bug 1069793 and try to find a different hack for it. Hacking on the FxOS audio plumbing seems more complicated.
Lets meet tomorrow and discuss about it. Thanks!
[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1069793#c12
Flags: needinfo?(josea.olivera)
Do you really want to put the GSM call on hold automatically as soon as there's an incoming loop call? Wouldn't we want the user to have to answer the loop call before the GSM call is put on hold. As a user it seems like I'd want to be able to keep talking on the GSM call and just ignore the loop call if I choose to.
That's also more similar to what happens if I'm on a GSM call and there's another incoming GSM call. All I hear is a small "do-do-do" sound, which I can ignore if my current call is more important.
Comment 18•10 years ago
|
||
(In reply to Jonas Sicking (:sicking) from comment #17)
> Do you really want to put the GSM call on hold automatically as soon as
> there's an incoming loop call?
No we don't but sadly we rely on mozinterrupt{begin, end} events. Let me explain this. While the GSM call is connected the Loop app listens for them. On a incoming WebRTC call the Loop app plays the ringtone. As the priority of the ringer channel is higher than the telephony one the events are fired and the GSM call is set on hold. The channel of the audio interrupting is not known so there is no way to differentiate if we really need/want to set the GSM call on hold when the ringtone plays.
Comment 19•10 years ago
|
||
Jonas, any particular reason why 'ringer' is higher than 'telephony'?
If 'ringer' is lower than telephony, we can have the loop app ringing, while a GSM call is ongoing.
We just need to inform the app that we have an ongoing call to play volume at a low level (the famous "doo-doo-doo" issue).
Flags: needinfo?(jonas)
Comment 20•10 years ago
|
||
Comment 21•10 years ago
|
||
Comment on attachment 8511519 [details]
Pointer to Github PR https://github.com/mozilla-b2g/firefoxos-loop-client/pull/242
Cristian, this PR is about re-ordering a bit the calls to the audio competing helper. Lets pair up for the review when you have time. Thanks!
Attachment #8511519 -
Flags: review?(crdlc)
Comment 22•10 years ago
|
||
Comment on attachment 8511519 [details]
Pointer to Github PR https://github.com/mozilla-b2g/firefoxos-loop-client/pull/242
LGTM Jose, I left a couple of comments but minor things, thanks
Attachment #8511519 -
Flags: review?(crdlc) → review+
Comment 23•10 years ago
|
||
Landed on master branch at:
https://github.com/mozilla-b2g/firefoxos-loop-client/commit/1b6a1f04ea428271b4a6b6afa06eab79a1102cc7
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
I always thought that telephony was higher than ringer, so no, i have no idea why it isn't.
That said, I'm totally fine with adding a TelephonyAgent API. I could see something like that being used in order to play the "doo doo doo" sound.
Flags: needinfo?(jonas)
Comment 25•10 years ago
|
||
(In reply to Jonas Sicking (:sicking) from comment #24)
> That said, I'm totally fine with adding a TelephonyAgent API. I could see
> something like that being used in order to play the "doo doo doo" sound.
When we defined the telephony audio channel usage policy we were in a hurry because we needed it for v2.0 release. IMHO the current policy needs some improvements so we should discuss about these improvements and how to support them. I'll file a bug for that. Thanks!
Comment 26•10 years ago
|
||
(In reply to Jonas Sicking (:sicking) from comment #24)
> I always thought that telephony was higher than ringer, so no, i have no
> idea why it isn't.
We don't need the telephony Agent, and jaoo just fixed this bug in gaia.
But yes, we should file a bug to have ringer < telephony and discuss the implementation of this (small - in gecko) change. Jonas, can you file the bug and CC anyone who should know about this change?
For this 'doo doo doo' sound, instead implementing a Telephony Agent, I would like to extend AudioChannelManager adding this:
readonly attribute telephonyChannelActive;
attribute EventHandler ontelephonyChannelChanged;
This should be enough to select the correct ringer sound. But not for v2.0 I guess.
Flags: needinfo?(jonas)
Comment 27•10 years ago
|
||
(In reply to Andrea Marchesini (:baku) from comment #26)
> For this 'doo doo doo' sound, instead implementing a Telephony Agent, I
> would like to extend AudioChannelManager adding this:
>
> readonly attribute telephonyChannelActive;
> attribute EventHandler ontelephonyChannelChanged;
>
> This should be enough to select the correct ringer sound. But not for v2.0 I
> guess.
Another approach could be to pass the audio channel interrupting the current audio channel in use on the onmozinterruptbegin event. That way once the ringer channel interrupts the telephony one we Loop/callscreen app could check whether either it should play the 'do-do-do' tone or set the WebRTC/GSM call on hold. Right now only the system app knows about the current audio channel in use.
Comment 28•10 years ago
|
||
After some testing I've not been able to reproduce the bug, and as this is a blocker issue for 1.1 Loop mobile version, let's land it in 1.1 branch.
Jose Antonio, could you please do the uplifting?
Thanks a lot!
Flags: needinfo?(josea.olivera)
Whiteboard: [mobile app][blocking][tef-triage] → [mobile app][blocking][tef-triage][loop not in v1.1][loop approved for 1.1]
Comment 29•10 years ago
|
||
Landed on 1.1 branch at:
https://github.com/mozilla-b2g/firefoxos-loop-client/commit/f16828c0e14f2e7fe008af7737e9f51ca2397baa
Flags: needinfo?(josea.olivera)
Whiteboard: [mobile app][blocking][tef-triage][loop not in v1.1][loop approved for 1.1] → [mobile app][blocking][tef-triage][loop in v1.1]
(In reply to Andrea Marchesini (:baku) from comment #26)
> But yes, we should file a bug to have ringer < telephony and discuss the
> implementation of this (small - in gecko) change. Jonas, can you file the
> bug and CC anyone who should know about this change?
Bug 1089742
Flags: needinfo?(jonas)
You need to log in
before you can comment on or make changes to this bug.
Description
•