1006413 - [Tarako][dolphin][Music][Call] While playing music through bluetooth, answer an incoming call and then hang up, music is paused

Status: RESOLVED FIXED

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

Description

[Bingqing Li]

Attachment: logcat

Environment:
---------------------------------------------------
Gaia      c067cc717c4a3bb8b412f28f37d224eb84b639df
Gecko     https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/8eebf8120dbd
BuildID   20140505164002
Version   28.1
ro.build.version.incremental=eng.cltbld.20140505.203623
ro.build.date=Mon May  5 20:36:32 EDT 2014


Steps to reproduce:
----------------------------------------------------
1. Pair a BT headset
2. Launch music, and choose a song to play in foreground
3. Make a MT call to the test device
4. Answer the MT call
5. Hang up the call


Actual result:
---------------------------------------
The music is paused.


Expected result:
--------------------------------------- 
Resume playing the music


Additional info:
--------------------------------------
Reproduce rate:  60%~70%

Comment 1

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Music through a regular headset or otherwise doesn't seem to reproduce this.  This seems only through bluetooth.

Comment 3

[Peipei Cheng (needinfo if you need my action)]

This can only be reproduced with BT headset.

Here is the analysis from Spreadtrum developper.
0. Pair BT headset and play music
1. Incoming call：
   case 'mozinterruptbegin':
   this.playStatus = INTERRUPT_BEGIN;
   It will trigger event mozinterruptbegin and modify playStatus
2. After answering the phone
   It will enter function pause and modify this._statusBeforeSCO:
     this._statusBeforeSCO = PlayerView.playStatus;
   which turns out to be this._statusBeforeSCO==mozinterruptbegin
3.After end call
  it invoke playpause function
  if (this._statusBeforeSCO === PLAYSTATUS_PLAYING)
     PlayerView.play();
  else
     PlayerView.pause();
  

Due to the result of 2, this._statusBeforeSCO === PLAYSTATUS_PLAYING will return false.
As a result, 3 will run PlayerView.pause();

Comment 4

[Peipei Cheng (needinfo if you need my action)]

Also reproducible on Flame v1.4.

Gaia      5527784b263755f55b620441fc95a66496fa18cf
Gecko     https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/334e9712f079
BuildID   20140528160208
Version   30.0
ro.build.version.incremental=94
ro.build.date=Tue May 20 09:29:20 CST 2014

Comment 5

[Preeti Raghunath(:Preeti)]

Moving to backlog been around earlier releases.

Comment 6

[Dominic Kuo [:dkuo]]

Thanks to Peipei and the partner's developer(comment 3), base on that information I found the root cause easily and fixed it right away, and apologize I didn't aware this might be a blocker for the certification so I haven't investigate it earlier.

I am attaching the patch and explaining in the next comment.

Comment 7

[Dominic Kuo [:dkuo]]

Attachment: patch

In bug 946556 we have disable the music player when the bluetooth headset is active(SCO is connected), and recover it after the bluetooth headset is inactive(SCO is disconnected). We assumed that before the SCO connection, the play status is |PLAYING|, so after SCO disconnected, the player should resume to |PLAYING|.

The reason this bug happens and not 100% reproducible is because, there are two possible play statuses when the call comes with bluetooth headset, |PLAYING| and |mozinterruptbegin|, while I was implementing that feature, I never encountered the |mozinterruptbegin| case, this might be another regression that caused by moving the ringer tone into system(bug 990003), cause looks like now |mozinterruptbegin| fires earlier then SCO is connected.

The fix is simple, just resume the player when the play status before SCO is |PLAYING| or |mozinterruptbegin|, and it should work on all branches, including 1.3, 1.3t, 1.4 and master.

Jim, would please review this patch? thanks!

(Also renominate this bug because I was requested by our partner to fix this bug.)

Comment 8

[Jim Porter (:squib)]

Comment on attachment 8435560 [details] [review]
patch

This seems like the right thing to do. Thanks, Dominic!

(I hope we can eliminate some of these issues when we rework audio channels, since it's very hard to get the logic right...)

Comment 9

[Dominic Kuo [:dkuo]]

Landed on master: 7a534f0afe16ccf38e8f17a73d0dc94ba1206ea6

Comment 10

[James Zhang (Spreadtrum)]

Please land on v1.3t and v1.4, thanks!

Comment 11

[Dominic Kuo [:dkuo]]

(In reply to James Zhang from comment #10)
> Please land on v1.3t and v1.4, thanks!

Can someone please make this blocker? I can help to uplift to 1.3t and 1.4 but need approval.

Comment 12

[James Zhang (Spreadtrum)]

(In reply to Dominic Kuo [:dkuo] from comment #11)
> (In reply to James Zhang from comment #10)
> > Please land on v1.3t and v1.4, thanks!
> 
> Can someone please make this blocker? I can help to uplift to 1.3t and 1.4
> but need approval.

Steven told me that we can merge anything to v1.3t without flag or review+.
Please let your patch get review+, I'll merge it to 1.3t.

Comment 13

[Dominic Kuo [:dkuo]]

v1.3t: fe0c0bcb572f23ea17170d5e9cac9f1a2415d6a6

Comment 14

[Preeti Raghunath(:Preeti)]

Is this a regression from 1.3T to Dolphin?

Also regression from 1.3 Buri to Flame?

This doesn't block the phone from shipping 1.4 release.

Comment 15

[Preeti Raghunath(:Preeti)]

Not taking in 1.4 as this is a minor user impact.

Comment 16

[James Zhang (Spreadtrum)]

Hi Yang, please land this WIP patch for dolphin. Thanks.

Comment 17

[yang.zhao]

Landed on our build by using patch on 6821 & 7715.