Closed Bug 1131968 Opened 6 years ago Closed 6 years ago

[FFOS2.0][Woodduck][Music][Call]Music on loudspeaker and incoming call

Categories

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

defect

Tracking

(blocking-b2g:2.0M+, b2g-v2.0 wontfix, b2g-v2.0M fixed, b2g-v2.1 wontfix, b2g-v2.1S wontfix, b2g-v2.2 wontfix, b2g-master wontfix)

RESOLVED FIXED
blocking-b2g 2.0M+
Tracking Status
b2g-v2.0 --- wontfix
b2g-v2.0M --- fixed
b2g-v2.1 --- wontfix
b2g-v2.1S --- wontfix
b2g-v2.2 --- wontfix
b2g-master --- wontfix

People

(Reporter: sync-1, Assigned: alwu)

References

Details

Attachments

(2 files, 4 obsolete files)

DEFECT DESCRIPTION:
 MUSIC on loudspeaker and incoming call, device  can't stop the music when an incoming call is present.
 
  REPRODUCING PROCEDURES:
 
 1.- Arrange to play an audio file  using loudspeaker could be using music app or youtube streaming
 2.- make phone receive a call
 3.- phone rings and audio plays at the same time- NOK
 4.- after a few moments after audio is stopped.
 
  EXPECTED BEHAVIOUR:
 If user is playing audio files or streaming device should stop playing the audio at the moment the incoming call is received.
 
 
 open music app. I found that, when playing the music on the foreground. the music will still play for a moment when receiving a call(also when receiving a call, the music's playing status icon on statusbar disappeared, but the music is still playing). but playing music on the background. it will stop immediately when receiving a call.
norry, could you please have someone to do branch check?
blocking-b2g: --- → 2.0M?
Flags: needinfo?(fan.luo)
Keywords: qawanted
This issue occurs on all Flame KK branches, but is worst on the 2.0 branch.  All branches will continue to play music for about 1 second after the incoming call starts to ring, 2.0 continues for about 3 seconds instead.
	
Environmental Variables:
Device: Flame 3.0
BuildID: 20150212064620
Gaia: 2a2b008f9ae957fe19ad540d233d86b5c0b6829e
Gecko: 81f979b17fbd
Version: 38.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

Environmental Variables:
Device: Flame 2.2
BuildID: 20150211165940
Gaia: 791e53728cd8018f1d7cf7efe06bbeb1179f0370
Gecko: 5dec207fcbeb
Version: 37.0a2 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Environmental Variables:
Device: Flame 2.1
BuildID: 20150211170050
Gaia: 88084bc7ef5ba6627dd09c774ef2f7fa96cbed71
Gecko: e395bfad7bc9
Version: 34.0 (2.1) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Environmental Variables:
Device: Flame 2.0
BuildID: 20150210182253
Gaia: 2989f2b2bd12fcc0e9c017d2db766e76a55873b8
Gecko: d47ec000ee85
Version: 32.0 (2.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Keywords: qawanted
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Flags: needinfo?(fan.luo)
Hi Alastor,
Could you help to check this problem? Thanks!
Flags: needinfo?(alwu)
Hi, Josh,
This is due to the drawback of the audio channel design, it could be fixed after landing Bug1113086.
Flags: needinfo?(alwu)
Thanks Alastor,
Can you confirm that Bug 1113086 can be fix on 2.0M after it's completed? Thanks!
Flags: needinfo?(alwu)
Sorry for my previous reply,
Although this is due to the drawback of the audio channel design, the new design would not be landed on 2.0M.
I will get a workaround to fix it.
Flags: needinfo?(alwu)
(In reply to Alastor Wu [:alwu] from comment #6)
> Sorry for my previous reply,
> Although this is due to the drawback of the audio channel design, the new
> design would not be landed on 2.0M.
> I will get a workaround to fix it.

Hi Alastor,
No problem. I have modified bug 1113086 as well. Could you also take this bug? Thanks!
Flags: needinfo?(alwu)
Of course :)
Assignee: nobody → alwu
Flags: needinfo?(alwu)
Priority: P2 → P1
Status: NEW → ASSIGNED
[Root cause]
The visibility-changed event is delivered late, so the music and ringtone are both visible. That results in playing the music and ringtone at the same moment.
Hi Alastor,
Can partner try this patch? Thanks!
Flags: needinfo?(alwu)
Hi, Josh,
Maybe try this one.
Attachment #8571829 - Attachment is obsolete: true
Flags: needinfo?(alwu)
Hi JingMing,
Can you try patch per comment 11?
Flags: needinfo?(jingming.qiu)
hi
use the patch from comment 11, the playing music can stop immediately. but click the cancel button can't stop the calling phone. the phone rings all the time..
Flags: needinfo?(jingming.qiu)
Alastor, could you please take a look at the problem mentioned in comment 13?
Flags: needinfo?(alwu)
I will check it later, and keep ni for tracking.
Hi, JingMing,
I can't reproduce your problem. 
Can you provide more specific reproduce steps? 
Thanks!
Flags: needinfo?(alwu)
Hi Alastor,
after use the patch, the steps are:
1. open the music app and play some.
2. when music is playing , make phone receive a call, and the playing music can stop immediately.
3. on the callscreen, click the refuse button or accept button can't stop the calling ring.

if your phone can stop the playing music immediately , and the call app used normal ?
Flags: needinfo?(alwu)
Hi, Could someone help me to do the branch check in flame?
It need to be checked after applying the patch in comment11, thanks :)
Flags: needinfo?(alwu)
Keywords: qawanted
QA Contact: ktucker
This issue still occurs on the Flame 3.0, Flame 2.2, Flame 2.1 and Flame 2.0

This only happens when the music is being played in the foreground. The music can still be heard playing when the phone first begins to ring. 

Environmental Variables:
Device: Flame 3.0(Full Flash)(KK)(319mb)
BuildID: 20150312010235
Gaia: 0c4e8b0b330757e261b031b7e7f326ef419c9808
Gecko: 5334d2bead3e
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
Version: 39.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

Environmental Variables:
Device: Flame 2.2(Full Flash)(KK)(319mb)
BuildID: 20150312002501
Gaia: 572d60e0a440ee4af50bc6b6adad8876eadbdb4d
Gecko: 244e6ba3c20e
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
Version: 37.0 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Environmental Variables:
Device: Flame 2.1(Full Flash)(KK)(319mb)
BuildID: 20150312001452
Gaia: db751d9c200dce41cd03a61665746d245739a175
Gecko: 28ffee0d5b0c
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
Version: 34.0 (2.1) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Environmental Variables:
Device: Flame 2.0(Full Flash)(KK)(319mb)
BuildID: 20150312000204
Gaia: 896803174633fc6acd3fd105f81c349b8e9b9633
Gecko: e89ba447d264
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
Version: 32.0 (2.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
Keywords: qawanted
Flags: needinfo?(alwu)
(In reply to jingming from comment #17)
> Hi Alastor,
> after use the patch, the steps are:
> 1. open the music app and play some.
> 2. when music is playing , make phone receive a call, and the playing music
> can stop immediately.
> 3. on the callscreen, click the refuse button or accept button can't stop
> the calling ring.
> 
> if your phone can stop the playing music immediately , and the call app used
> normal ?

Hi, JingMing,
The ringtone would be stopped normally in my flame when I press the button. I can't reproduce this issue.
Could you give me your device info?
Thanks!

Here is my test environment,
Gaia-Rev        896803174633fc6acd3fd105f81c349b8e9b9633
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/eba4b0360351
Build-ID        20150313000203
Version         32.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.alastor.20150313.100251
FW-Date         五  3月 13 10:03:12 CST 2015
Bootloader      L1TC00011880
hi Alastor

I use the latest code of woodduck, if there is any woodduck phone for you to test this problem.
Sorry, JingMing,
I get the woodduck phone, but it can't work.
Now I'm waiting for the permission from Sein-Lin to access the woodduck repository. 

When my woodduck can work successfully, I will check your problem ASAP.
Hi Alastor:

Shawn has just fix the permission issue for you and woul you help check it with high priority? Like just mentioned this is one of the left issues in the list for the SW for TCL's customer test (to be released on Wednesday). Thank you!
Hi, JingMing,
I use the lastest code of woodduck, but I still can't reproduce your problem.

Here is the recoding video and my test steps.
(1) Open music app and play music
(2) Get an incoming call (music stop)
(3) Answer or reject incoming call 
(4) Ringtone stop

Answer : smb://fs1.mozilla.com.tw/public/Alastor/VID_20150316_214507.mp4
Reject : smb://fs1.mozilla.com.tw/public/Alastor/VID_20150316_215127.mp4
Flags: needinfo?(alwu)
blocking-b2g: 2.0M? → 2.0M+
Flags: needinfo?(sync-1)
Flags: needinfo?(jingming.qiu)
Hi JingMing,
We cannot repo this issue on our side. Can you try repo the issue with Mozilla Gaia/Gecko? Thanks!

Image link: http://pan.baidu.com/s/1kTiQeRt 
Access code: dyeq
unzip password: erT14moz
hi

sorry, after repo to get the latest code,it seems like the problem already fixed now..

thanks.
Flags: needinfo?(jingming.qiu)
Hi Alastor,
I think we can still land this workaround for 2.0M and leave formal refacto for audio channel on future version. Thanks!
Flags: needinfo?(sync-1) → needinfo?(alwu)
Rebase the patch for v2.0m.
Attachment #8573045 - Attachment is obsolete: true
Flags: needinfo?(alwu)
Comment on attachment 8579095 [details] [diff] [review]
[v2.0m] visible audio need to check whether there has higher priority audio

Hi, Ehsan,
Sorry to bother you,
Could you help me review this patch?

This is a high priority bug of our partner, we need someone to review it.
very appreciate :)
Attachment #8579095 - Flags: review?(ehsan)
Sorry I forgot to describe the details!
Here is the description.

[Root cause]
When the audio is visible, it could always be playable. The notification of the visibility changing event was too late for the music app so that the AudioChannelService can't mute it even when the ringtone started playing. At that time, the music and ringtone were both visible. (But the music app actually was in the background)

[Solution]
Even if the audio is visible, it still need to check whether there is more higher priority audio.
we will mute the lower priority audio.

Thanks :)
Comment on attachment 8579095 [details] [diff] [review]
[v2.0m] visible audio need to check whether there has higher priority audio

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

Please write a test for this too, if possible.  Minusing because of the comment in the .cpp file.

::: dom/audiochannel/AudioChannelService.cpp
@@ +1088,5 @@
> +  else if (mCurrentHigherChannel == (int32_t) AudioChannel::Notification ||
> +           mCurrentHigherChannel == (int32_t) AudioChannel::Publicnotification) {
> +    return false;
> +  } else {
> +    return mCurrentHigherChannel != (int32_t) aChannel;

Don't you need to see if mCurrentHigherChannel is actually higher than aChannel here, rather than just checking if they are equal?

::: dom/audiochannel/AudioChannelService.h
@@ +219,5 @@
>    // This returns the number of agents from this aWindow.
>    uint32_t CountWindow(nsIDOMWindow* aWindow);
>  
> +  bool
> +  ExistPlayingHigherPriorityChannel(AudioChannel aChannel,

Nit: please call this PlayingHigherPriorityChannelExists().
Attachment #8579095 - Flags: review?(ehsan) → review-
Hi, Ehsan,
Here is my revised patch, 
Thanks for your help :)

---

Btw, 
Could I add the test case later? Because now I need to handle other bugs first.
Thanks!
Attachment #8579095 - Attachment is obsolete: true
Attachment #8579985 - Flags: review?(ehsan)
Comment on attachment 8579985 [details] [diff] [review]
[v2.0m] visible audio need to check whether there has higher priority audio

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

Adding a test later is fine as long as you don't forget.  :-)  r=me
Attachment #8579985 - Flags: review?(ehsan) → review+
The test case is filed to the bug1145476.
Attachment #8579985 - Attachment is obsolete: true
Attachment #8580436 - Flags: review+
Hi Kai-Zhen,
Could you help to land this on 2.0M? Thanks!
Flags: needinfo?(kli)
Hi JingMing,
Can you confirm the patch works for you?
Flags: needinfo?(jingming.qiu)
(In reply to Josh Cheng [:josh] from comment #37)
> Hi JingMing,
> Can you confirm the patch works for you?

hi josh
my colleage had test the patch, and it works, so it's ok now.
thanks.
Flags: needinfo?(jingming.qiu)
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Hi Alastor,

After merged the patch, we got a critical problem when testing recevier: there's no sound comes from it. We write test codes as below to force audio comes from the receiver and it works well before.
> const nsIAudioManager = Ci.nsIAudioManager;
> XPCOMUtils.defineLazyGetter(this, "gAudioManager", function getAudioManager() {
>   try {
>     return Cc["@mozilla.org/telephony/audiomanager;1"]
>             .getService(nsIAudioManager);
>   } catch (ex) {
>     return null;
>   }
> });
> gAudioManager.phoneState = nsIAudioManager.PHONE_STATE_IN_CALL;
but now when checking with PlayingHigerPriorityChannelExists, we found aChannel==0, mCurrentHigherChannel==4 (telephony), and will return "AUDIO_CHANNEL_STATE_MUTED". I wonder if we can add more conditions here:
>  else if (mCurrentHigherChannel == (int32_t) AudioChannel::Notification ||
>           mCurrentHigherChannel == (int32_t) AudioChannel::Publicnotification ||
>           mCurrentHigherChannel == (int32_t) AudioChannel::Telephony) {
>    return false;
Or is there any other solution as a workaround?

Thanks.
Flags: needinfo?(alwu)
Hi, Jin,

If the normal type sound is on the foreground, it can be playable by this condition check.
> // The audio from background to foreground should always be playable.
> if (aElementWasHidden && !aElementHidden) {
>   return false;
> }

If the normal type sound is on the background and there is another higher priority channel. 
I think we should mute it.

---

What condition the sound can't come out from the receiver?
Could you describe your testing steps?
Thanks :)
Flags: needinfo?(alwu) → needinfo?(jz.zhangjin)
As in comment#39, in the test codes we
> 1) get an Audio element
>     <audio id="audio-element-id" style="display: none;" loop="loop" src="../resource
>     /mojito_112_signal_30s_9db.wav"></audio>
> 2) navigator.mozTelephony.speakerEnabled = false;
>    gAudioManager.phoneState = nsIAudioManager.PHONE_STATE_IN_CALL;
>           to force audio come out from the recevier
> 3) audio.play();
no other sound when we do the test, and we stay at the audio page. I add some logs in "PlayingHigherPriorityChannelExists", and found it run three times and finally return true.
> I/cgq     (  164): PlayingHigherPriorityChannelExists, aElementWasHidden: 1, aElementHidden: 0 
> I/cgq     (  164): PlayingHigherPriorityChannelExists, Channel: 4, mCurrentHigherChannel: 4 
> I/cgq     (  164): PlayingHigherPriorityChannelExists, aElementWasHidden: 1, aElementHidden: 0 
> I/cgq     (  164): PlayingHigherPriorityChannelExists, Channel: 0, mCurrentHigherChannel: 4 
> I/cgq     (  164): PlayingHigherPriorityChannelExists, aElementWasHidden: 0, aElementHidden: 0 
> I/cgq     (  164): PlayingHigherPriorityChannelExists, Channel: 0, mCurrentHigherChannel: 4
Flags: needinfo?(jz.zhangjin) → needinfo?(alwu)
Hi, Jin, 
I am testing the new patch now. I would update the available patch after finishing.
Keep ni for tracking.
Thanks!
Hi, Jin,
First, sorry for this regression error.

Use the the priority to decide whether the visible sound is playable is not a very good solution, but this is the only way we can do in the present audio channel architecture.

To add more constraints to the visible audio have high risk, it would result other bug issues.
ex. we can't play the normal during the playing content audio.  

Therefore, I think we should only focus on this condition to modify the workaround, such like..
> if (mCurrentHigherChannel == (int32_t) AudioChannel::Ringer &&
>     mCurrentHigherChannel > (int32_t) aChannel) {
>   return AUDIO_CHANNEL_STATE_MUTED;
> }

The root cause of this issue is that the visibility-changed event is delivered late. Since we can't decide the delivering time in Gecko, I think this patch probably is the only solution for us.

If it still have problems, please contact me!
Thanks!
Flags: needinfo?(alwu)
Test case has been added in moztrap:
https://moztrap.mozilla.org/manage/case/2388/
Flags: in-moztrap+
You need to log in before you can comment on or make changes to this bug.