923742 - [B2G][Bluetooth] Rocketfish headset audio is rerouted to the device when user takes call waiting

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Description

[Matthew Vaughan]

Attachment: LogCat for answering a call waiting with BT headset and call audio is rerouted.

Description:
When a user takes a call waiting, the bluetooth headset (Rocketfish model:RF-QX4) audio is rerouted to the device/phone even though the headset is still connected. 

Repro Steps:
1) Update Buri to Build ID: 20131004004003
2) Connect a bluetooth(BT) headset to your phone
3) Have another phone call your phone
4) Answer the first call with BT headset
5) Have a second phone call your phone
6) Attempt to answer the call waiting with BT headset
7) Observe first call is ended
8) Attempt to answer the call waiting with BT headset

Actual:
Call waiting is answered but the call audio is rerouted to the phone.

Expected:
Call waiting is answered with the call audio remaining on the BT headset.

Environmental Variables
Device: Buri
Build ID: 20131004004003
Gecko: http://hg.mozilla.org/releases/mozilla-aurora/rev/a4b7282df517
Gaia: 9e21b6bea92fdafcb6787120a8cde0eb25a50495
Platform Version: 26.0a2

Notes:
Repro frequency: 100%
See attached: logcat.txt
Notes: Product website- http://www.rocketfishproducts.com/products/mobile-phones-gps/RF-QX4.html

Comment 1

[Jason Smith [:jsmith]]

Is this reproduce on 1.1?

Comment 2

[Matthew Vaughan]

This bug does not occur on 1.1. However, it does start occurring on the 7/31/13 1.2 build. 

- Works -
Environmental Variables
Build ID: 20130730030200
Gecko: http://hg.mozilla.org/mozilla-central/rev/3d40d270c031
Gaia: ba5ff211fbf6a930326cc6a0d4a1205a7528630b
Platform Version: 25.0a1

- Broken -
Environmental Variables
Build ID: 20130731030205
Gecko: http://hg.mozilla.org/mozilla-central/rev/c2b375f3a909
Gaia: 9bfceaa90e8b92a379432b67121afa3cd3f14c90
Platform Version: 25.0a1

Comment 3

[Preeti Raghunath(:Preeti)]

Vicamo

Can you please take a look, as this seems to be a Bluetooth issue?

Comment 4

[Eric Chou [:ericchou] [:echou]]

Hi Preeti,

(In reply to Preeti Raghunath(:Preeti) from comment #3)
> Vicamo
> 
> Can you please take a look, as this seems to be a Bluetooth issue?

Please assign Bluetooth related issues to me. Vicamo is the owner of RIL. Thanks.

- Eric

Comment 5

[Eric Chou [:ericchou] [:echou]]

I just filed a bug (bug 925663) and it may be related to this one. Matthew, I'd like to ask for your help to test again after the patch of bug 925663 lands. It would be even better if you could provide hcidump log for us debugging. Thank you.

Comment 6

[Eric Chou [:ericchou] [:echou]]

Hi Matthew,

Since bug 925663 has landed, would you mind using the latest build and giving it another try?

Thanks!

Comment 7

[Matthew Vaughan]

Hey Eric,

I attempted the repro steps again and the bug is still occurring on the latest 1.2 build.

Environmental Variables:
BuildID: 20131015004002
Gaia: 94e3088c56c7348655ae76613defd126426cc722
Gecko: f80fd27d367f
Version: 26.0a2

Comment 8

[Eric Chou [:ericchou] [:echou]]

Thanks, I'll take a look. :)

Comment 9

[Eric Chou [:ericchou] [:echou]]

Issue confirmed. I can reproduce on both the latest 1.2 and m-c.

The root cause is that "Force use for communications" was set to 0 when the active call was dropped (step 6). After step 6, phone state would be set to 0. Furthermore, "Force use for communications" wouldn't be set back to Bluetooth_Sco again because the status of SCO connection didn't change since the first call was picked up.

Just had a discussion with Marco, audio manager should not change the value of "Force use for communications" when SCO exists. We're going to revise AudioManager.cpp, and make sure the mechanism of SCO establishment / disconnection is correct in BluetoothHfpManager.cpp.

Redirect to Star Cheng to work on AudioManager.cpp.

Comment 10

[Joe Cheng [:jcheng] (please needinfo)]

triage: only happening on older BT headsets, not blocking. please ask for approval to land in v1.2

Comment 11

[Star Cheng [:scheng]]

Attachment: Bug923742.patch

Comment 12

[Star Cheng [:scheng]]

Comment on attachment 820206 [details] [diff] [review]
Bug923742.patch

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

While the phone status is changed, AudioManager probably SetForceUse as FORCE_NONE according to phone status & BT SCO link. In this case, the phone status is hang up and SOC link is still connected. So AudioManager will SetForceUse as FORCE_NONE. Per discussion with BT team, we decide to SetForceUse as FORCE_NONE or FORCE_BT_SCO base on observing BT status changed.

Comment 13

[Michael Wu [:mwu]]

Comment on attachment 820206 [details] [diff] [review]
Bug923742.patch

Marco is probably a better reviewer for this.

Comment 14

[Jason Smith [:jsmith]]

(In reply to Joe Cheng [:jcheng] from comment #10)
> triage: only happening on older BT headsets, not blocking. please ask for
> approval to land in v1.2

This is a regression, so that's not acceptable by any means. This is a basic flow that should be working. Renoming.

Comment 15

[Eric Chou [:ericchou] [:echou]]

Comment on attachment 820206 [details] [diff] [review]
Bug923742.patch

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

Looks good to me. Please make sure Marco will take a look at this patch as well. Thanks.

Comment 16

[Marco Chen [:mchen]]

Comment on attachment 820206 [details] [diff] [review]
Bug923742.patch

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

Refer to the comment from Randy on Bug 897364, we need to remove FORCE_BT_SCO before set phone state to normal.
Or the wrong flow will cause wrong A2DP resuming behaviour in Audio HAL.

Could you verify that the disconnecting of SCO will be sent before SetPhoneState?
If not, we will reopen the other bug. Thanks.

Comment 17

[Star Cheng [:scheng]]

I have verified on the bug(Bug 842934). It looks fine.

Comment 18

[Preeti Raghunath(:Preeti)]

Eric,

Where is it a regression from? it says 18 is not affected and B2G 1.2 is impacted.

Please state if a new feature in 1.2 is broken.

Comment 19

[Jason Smith [:jsmith]]

(In reply to Preeti Raghunath(:Preeti) from comment #18)
> Eric,
> 
> Where is it a regression from? it says 18 is not affected and B2G 1.2 is
> impacted.
> 
> Please state if a new feature in 1.2 is broken.

It's not related to a new feature being broken. It's a regression - see comment 2 which cites this last worked on 7/30/2013, first broke on 7/31/2013.

Comment 20

[Eric Chou [:ericchou] [:echou]]

(In reply to Preeti Raghunath(:Preeti) from comment #18)
> Eric,
> 
> Where is it a regression from? it says 18 is not affected and B2G 1.2 is
> impacted.
> 
> Please state if a new feature in 1.2 is broken.

It's a regression of bug Bug 842934, which was merged to m-c on 7/30.

Comment 21

[Tony Chung [:tchung]]

blocking koi+.  regression.

Comment 22

[Star Cheng [:scheng]]

Attachment: bug-923742_20131025_04.txt

10-25 16:52:19.799 E/AudioManager(  144): AudioManager::SetPhoneState aState 1
...
10-25 16:52:24.669 E/AudioManager(  144): AudioManager::SetPhoneState aState 2
...
10-25 16:52:25.199 E/AudioManager(  144): AudioManager::HandleBluetoothStatusChanged SOC is available
...
10-25 16:52:30.539 E/AudioManager(  144): AudioManager::SetPhoneState aState 0
...
10-25 16:52:30.559 E/AudioManager(  144): AudioManager::HandleBluetoothStatusChanged SOC link is disconnect


[star] the status emulation value as followings: (@ AudioSystem.h)
    AUDIO_MODE_NORMAL           = 0,
    AUDIO_MODE_RINGTONE         = 1,
    AUDIO_MODE_IN_CALL          = 2,

According to the log, the SOC link is disconnected after SetPhoneStatus as 0(Normal status). But It works fine. I will trace code to find why it works fine.

Comment 23

[Star Cheng [:scheng]]

The root cause of Bug 897364 & Bug 842934 is follow Android's flow to setDeviceConnectionState() change BT SCO device (in HandleBluetoothStatusChanged())instead of SetForceForUse() (in SetPhoneState()). Whatever we use SetForceForUse() or not, the A2DP will resume normally but SOC link is disconnected. So we use this patch to fix this bug.

Comment 24

[Jason Smith [:jsmith]]

scheng - Can you clarify what's left to be implemented here in order to fix this? I see we've got a patch here r+, so wondering what's left to be worked on.

Comment 25

[Star Cheng [:scheng]]

Per my investigation, the setDeviceConnectionState(), SetPhoneState(), and SetForceForUse() will use the checkA2dpSuspend() to check the A2DP status and decide the status of A2DP will be suspended or resumed. The Firefox OS flow is following:

1.HFP connect and setDeviceConnectionState(AUDIO_DEVICE_OUT_BLUETOOTH_SCO_HEADSET)
  setDeviceConnectionState() -> checkA2dpSuspend() -- do nothing for A2DP
2.A2DP connect and setDeviceConnectionState(AUDIO_DEVICE_OUT_BLUETOOTH_A2DP)
  setDeviceConnectionState() -> checkA2dpSuspend() -- do nothing for A2DP
3.play mp3 media file on BT headset through A2DP
4.The call is coming, SetPhoneState(MODE_RINGTONE) 
  SetPhoneState() -> checkA2dpSuspend() -- A2DP is suspended (because it satisfy (mScoDeviceAddress != "") && (mPhoneState == AudioSystem::MODE_RINGTONE) )
5.In Call, SetPhoneState(MODE_IN_CALL) and SetForceForUse(FORCE_BT_SCO)
  SetPhoneState() -> checkA2dpSuspend() -- do nothing for A2DP
  SetForceForUse() -> checkA2dpSuspend() -- do nothing for A2DP
6.End the call, SetPhoneState(MODE_NORMAL) and SetForceForUse(FORCE_NONE)
  SetPhoneState() -> checkA2dpSuspend() -- do nothing for A2DP
  SetForceForUse() -> checkA2dpSuspend() --- A2DP is resumed

the function can check the A2DP status and let A2DP to work normally.

Comment 26

[Marco Chen [:mchen]]

(In reply to Jason Smith [:jsmith] from comment #24)
> scheng - Can you clarify what's left to be implemented here in order to fix
> this? I see we've got a patch here r+, so wondering what's left to be worked
> on.

Hi Jason,

This patch lacks a review+ from me because I asked Star to figure out the detail of A2DP suspend/resume flow. Or we fixed this issue but don't why.

Since Star already ran though the A2DP flow and make sure ours is correct then I would give a r+.

Comment 27

[Marco Chen [:mchen]]

Comment on attachment 820206 [details] [diff] [review]
Bug923742.patch

Hi Star,

Thanks for your investigation on A2DP audio control flow. That study ensures our flow now is correct.

Comment 28

[Star Cheng [:scheng]]

Attachment: Bug923742.patch-reviewed

the patch was reviewed by echou and mchen.

Comment 29

[Eric Chou [:ericchou] [:echou]]

https://hg.mozilla.org/integration/b2g-inbound/rev/b6458ec09a39

Comment 30

[Carsten Book [:Tomcat]]

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

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/fb7ee9e369ec