834200 - [Music] When we get a incoming call , Music still continues to play untill we answer the call

Status: VERIFIED FIXED

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

Description

[Leo]

1. Title : Music plays to continue, even though there is a incoming call notification.
2. Precondition : Have music files/songs in device, Set the device into vibration mode
3. Tester's Action : Launch music app ->select any song and play->when we get a incoming call->music plays and also there is vibration from incoming call->the music continues to play along with the vibration till we answer the call.
4. Detailed Symptom (ENG.) : 
                          1. Launch a Music app and play any song
                          2. when we get an incoming call, music and phone vibrations are overlapped.
                          3. Music plays to continue along with vibration until we answer or receive the call.
5. Gaia Revision # :   "7b8dea7e7ec377a2143151bbe5e9998d87f7b36d"
6. Expected :  We will get a incoming call, music should stop or pause, and phone vibration should be started.
7.Reproducibility: Yes
                1)Frequency Rate : 100%
8.Comparison Results : 
                1)Model Comparing : 
9. Attached files: 
                1)Log : 
                2)Test Contents : 
                3)Video file :

Comment 1

[Marco Chen [:mchen]]

Since user set device to vibration mode, that caused no other audio source with ringer channel type which can block the music with content channel type.

There are two solutions here.

1. Gaia: Dialer app still play a muted ring tone.
2. Gecko: Detect ringing state from audio manager then joining into audio channel service as ringer type.

Comment 2

[Casey Yee]

I think the current behavior is pretty good.  I think the only improvement we could make is to have the content volume turn down when receiving a incoming calls with the headset plugged in.    This way users won't miss calls even with music turned up and not noticing their phone is vibrating.

Comment 3

[Marco Chen [:mchen]]

I think the most easiest way now is to mute the content channel when 
  1. ringer channel is exist 
  2. AudioManager reports that now is in PHONE_STATE_RINGTONE. (to cover vibration mode).

Will update a patch later.

Comment 4

[Marco Chen [:mchen]]

I found that 
  1. Ring is ongoing and phone state in audio manager will be set to ring.
  2. If user doesn't accept the call, then phone state doesn't be set back to normal.

Need to clarify this issue then submit patch.

Hi Hsin-Yi,

Could you help to clarify the issue I described as above? Thanks.

Comment 5

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Attachment: updateAudioState when there's no active call

update Audio State to PHONE_STATE_NORMAL when rejecting an incoming call and there's no active call

Comment 6

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Marco Chen [:mchen] from comment #4)
> I found that 
>   1. Ring is ongoing and phone state in audio manager will be set to ring.
>   2. If user doesn't accept the call, then phone state doesn't be set back
> to normal.
> 
> Need to clarify this issue then submit patch.
> 
> Hi Hsin-Yi,
> 
> Could you help to clarify the issue I described as above? Thanks.

Hi Marco,

thanks for reporting this :) We missed something for updating audio state. I've provided a patch correcting that. Please help verify whether it helps resolve the problem here. 

B.t.w, is the audio state error the root cause of this music issue?  Should I file another bug for my patch or leaving the patch here is just right?

Comment 7

[Marco Chen [:mchen]]

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #6)
> Hi Marco,
> 
> thanks for reporting this :) We missed something for updating audio state.
> I've provided a patch correcting that. Please help verify whether it helps
> resolve the problem here. 
> 
> B.t.w, is the audio state error the root cause of this music issue?  Should
> I file another bug for my patch or leaving the patch here is just right?

Hi Hsin-Yi,

No, this is not the root cause on this bug. So please fire another one to process this patch.
Thanks for your help.

Comment 8

[Marco Chen [:mchen]]

Hi Hsin-Yi,

It is workable. Great. Thanks.

Comment 9

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Marco Chen [:mchen] from comment #7)
> (In reply to Hsin-Yi Tsai [:hsinyi] from comment #6)
> > Hi Marco,
> > 
> > thanks for reporting this :) We missed something for updating audio state.
> > I've provided a patch correcting that. Please help verify whether it helps
> > resolve the problem here. 
> > 
> > B.t.w, is the audio state error the root cause of this music issue?  Should
> > I file another bug for my patch or leaving the patch here is just right?
> 
> Hi Hsin-Yi,
> 
> No, this is not the root cause on this bug. So please fire another one to
> process this patch.
> Thanks for your help.
Filed Bug 836168!

Comment 10

[Marco Chen [:mchen]]

Attachment: Patch v1

When device is in vibrate state, the ring doesn't be played by dialer app. So there is no ringer channel registered into AudioChannelService. In this patch, I hook the AudioManager::SetPhoneState to get ring & in_call state then register corresponding channel into AudioChannelService.

Comment 11

[Andrea Marchesini [:baku]]

Comment on attachment 708003 [details] [diff] [review]
Patch v1

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

::: dom/system/gonk/AudioManager.cpp
@@ +310,5 @@
> +    mPhoneAudioAgent->StopPlaying();
> +    mPhoneAudioAgent = nullptr;
> +  }
> +
> +  if (aState == PHONE_STATE_IN_CALL || aState == PHONE_STATE_RINGTONE) {

What about the other states? PHONE_STATE_IN_COMMUNICATION and PHONE_STATE_CURRENT

@@ +323,4 @@
>  
> +    // Telephony can always play.
> +    bool canPlay;
> +    mPhoneAudioAgent->StartPlaying(&canPlay);

MOZ_ASSERT(canPlay);

Comment 12

[Marco Chen [:mchen]]

(In reply to Andrea Marchesini (:baku) from comment #11)
> Comment on attachment 708003 [details] [diff] [review]
> Patch v1
> 
> Review of attachment 708003 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/system/gonk/AudioManager.cpp
> 
> What about the other states? PHONE_STATE_IN_COMMUNICATION and
> PHONE_STATE_CURRENT

COMMUNICATION is used on VOIP liked application and that is not used on B2G currently.
CURRENT is not a real mode.
Please refer to 
http://developer.android.com/reference/android/media/AudioManager.html#MODE_IN_COMMUNICATION.

> 
> @@ +323,4 @@
> >  
> > +    // Telephony can always play.
> > +    bool canPlay;
> > +    mPhoneAudioAgent->StartPlaying(&canPlay);
> 
> MOZ_ASSERT(canPlay);

Will add it.

Comment 13

[Marco Chen [:mchen]]

Wait for try.
https://tbpl.mozilla.org/?tree=Try&rev=a7829d85f89d

Comment 14

[Marco Chen [:mchen]]

[Approval Request Comment]
Bug caused by (feature/regressing bug #): The ring tone with ringer channel will pause the lower channels including music & FM. But once user set device to silent mode then dialer app doesn't play ring tone so no ringer channel can pause other low channels.

User impact if declined: If user set device to silent mode and have headset plugged, user may not aware there is an incoming call. Because there is no ring tone and music is still playing as normal.

Testing completed: Test incoming call in silent mode with music/FM/Video.

Risk to taking this patch (and alternatives if risky): No.

Comment 15

[Lukas Blakk [:lsblakk] use ?needinfo]

Blocking since we this is a regression, a user should be able to hear incoming calls through headset when listening to music.

Comment 16

[Marco Chen [:mchen]]

Attachment: Patch Checkin-Version

Try is all green and Add reviewer and tef+.

Comment 17

[Andrea Marchesini [:baku]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e0cdaff13f73

https://hg.mozilla.org/releases/mozilla-b2g18/rev/450fd3b8496b

Comment 18

[Andrea Marchesini [:baku]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_0/rev/ee5bf23c9464

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e0cdaff13f73

Comment 20

[Jeni]

Verified issue fixed on Unagi 
Build ID 20130214070203
Kernel Dec 5
Gecko:http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/d1288313218e
Gaia: 6544fdb8dddc56f1aefe94482402488c89eeec49