Closed Bug 1073494 Opened 10 years ago Closed 10 years ago

Audio is not routing to headset while playback is on BT

Categories

(Firefox OS Graveyard :: AudioChannel, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:2.1+, firefox33 wontfix, firefox34 fixed, firefox35 fixed, b2g-v2.0 affected, b2g-v2.1 verified, b2g-v2.2 verified)

VERIFIED FIXED
2.1 S6 (10oct)
blocking-b2g 2.1+
Tracking Status
firefox33 --- wontfix
firefox34 --- fixed
firefox35 --- fixed
b2g-v2.0 --- affected
b2g-v2.1 --- verified
b2g-v2.2 --- verified

People

(Reporter: vasanth, Assigned: rlin)

References

Details

(Whiteboard: [caf priority: p2][CR 729198])

Attachments

(1 file, 3 obsolete files)

[Blocking Requested - why for this release]:

STR
---
1. Pair a BT headset and connect it
2. Start playback on BT
3. plug the wired headset while playback is on BT
4. Audio playback is not routing to wired headset after inserting it.

Is this an expected behavior?
The behavior in Android seems like playback switches to last inserted headset.

Device and build Details:
Device msm8x26 QRD KK branch

Gaia : 93a99bea0b40d81bd063f7d8b1964dc1ba35ba7b
Gecko: c896bc5d04b8c08dcddd78195901503a3578a08f
OS Version: 2.1.0
Platform Version : 34.0a2
Build ID: 20140925185330
Whiteboard: [CR 729198] → [caf priority: p2][CR 729198]
Qawanted to check on 2.0 to confirm the STR in description to confirm this is be design.
Keywords: qawanted
I am also sure Eric may know if this is by design, so NI him here.
Flags: needinfo?(echou)
Issue occurs on Flame 2.1 KK, Flame 2.0 KK and the Flame KK v180 2.0 base.

Actual results: Audio volume lowers slighty but continues to come from the bluetooth headset.  The volume level warning shows on screen.

Environmental Variables:
Device: Flame 2.1
BuildID: 20140930072157
Gaia: e9cbcbb12c1ec76e0ccdc88f1962f2afa3dd17b2
Gecko: 6b224957f0bd
Version: 34.0a2 (2.1) 
Firmware Version: L1TC10011800
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Issue also occurs on Flame 2.2 KK with slightly different results.

Actual results: Audio continues to come from bluetooth headset unchanged.

Environmental Variables:
Device: Flame 2.2
BuildID: 20140930061521
Gaia: 77ef35f5429bc3dfe9ca192b9aacc3c0bf8857de
Gecko: 2ae57957e4bb
Version: 35.0a1 (2.2) 
Firmware Version: L1TC10011800
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
Keywords: qawanted
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell)
Hi Steven,
   Do you have any idea?
Flags: needinfo?(slee)
Assignee: nobody → echou
Target Milestone: --- → 2.1 S6 (10oct)
Per offline discussion with Steven, he'll help handle this bug.

Steven, set assignee to you. Thanks!
Assignee: echou → slee
clear ni per comment 5
Flags: needinfo?(echou)
blocking-b2g: 2.1? → 2.1+
Randy is working on this.
Assignee: slee → rlin
Flags: needinfo?(slee)
dump the n5 audio policy setting. 

a:Plug-in headset then connect A2DP
 A2DP device address: 00:1E:DC:A2:99:EF
 SCO device address: 00:1E:DC:A2:99:EF
 USB audio ALSA 
 Output devices: 000000ab
 Input devices: 0000018c
 Phone state: 0
 Force use for communications 0
 Force use for media 0 <---
 Force use for record 0
b: connect A2DP then Plug-in headset
 A2DP device address: 00:1E:DC:A2:99:EF
 SCO device address: 
 USB audio ALSA 
 Output devices: 000000ab
 Input devices: 0000018c
 Phone state: 0
 Force use for communications 0
 Force use for media 10  <---
 Force use for record 0
 Force use for dock 8
 Force use for system 0

It shows that gecko side need to invoke the 
AudioPolicyManagerBase::setForceUse to set the AudioSystem::FORCE_NO_BT_A2DP flag for this user intend.
Attached patch patch v1 (obsolete) — Splinter Review
Refer to android design and try to setForceUse(AUDIO_POLICY_FORCE_FOR_MEDIA, AUDIO_POLICY_FORCE_NO_BT_A2DP) if headset has been plugged.
Attachment #8500309 - Flags: review?(mwu)
Attached patch patch v1.1 (obsolete) — Splinter Review
Change the uuid in nsIAudioManager.idl.
Attachment #8500309 - Attachment is obsolete: true
Attachment #8500309 - Flags: review?(mwu)
Attachment #8500311 - Flags: review?(mwu)
Comment on attachment 8500311 [details] [diff] [review]
patch v1.1

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

Looks fine, though I think the AudioManager.cpp changes can be simplified a little bit.

::: dom/system/gonk/AudioManager.cpp
@@ +245,5 @@
>      NS_NOTREACHED("Doesn't support audio routing on GB version");
>    }
>  }
>  
> +static nsresult SetForceUse(int32_t aUsage, int32_t aForce)

I don't think you need to move this function out here - AudioManager::SetForceForUse should be available from all the places you want to call it from.

@@ +427,5 @@
>      } else if (aEvent.status() != SWITCH_STATE_OFF) {
>        InternalSetAudioRoutes(aEvent.status());
>        sSwitchDone = true;
>      }
> +    // Handle the coexist of a2dp / headset device, latest one wins.

s/coexist/coexistence/

@@ +429,5 @@
>        sSwitchDone = true;
>      }
> +    // Handle the coexist of a2dp / headset device, latest one wins.
> +#if ANDROID_VERSION >= 17
> +    if ((aEvent.status() != SWITCH_STATE_OFF) && sBluetoothA2dpEnabled) {

Parenthesis for the switch state check shouldn't be necessary. (and doesn't match the style of the rest of the function)
Attachment #8500311 - Flags: review?(mwu)
Attached patch patch v1.2 (obsolete) — Splinter Review
Fix nits, 
About the 
    if ((aEvent.status() != SWITCH_STATE_OFF) && sBluetoothA2dpEnabled) {
I add more check if previous FORCE_FOR_MEDIA has set to AUDIO_POLICY_FORCE_NO_BT_A2DP, 
The policy for media can reset to force_none if user plug-out headset.
Attachment #8500311 - Attachment is obsolete: true
Attachment #8500821 - Flags: review?(mwu)
Comment on attachment 8500821 [details] [diff] [review]
patch v1.2

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

::: dom/system/gonk/AudioManager.cpp
@@ +417,5 @@
> +    // Handle the coexistence of a2dp / headset device, latest one wins.
> +#if ANDROID_VERSION >= 17
> +    nsCOMPtr<nsIAudioManager> amService = do_GetService(NS_AUDIOMANAGER_CONTRACTID);
> +    NS_ENSURE_TRUE_VOID(amService);
> +    AudioManager *am = static_cast<AudioManager *>(amService.get());

Initialize this class with a pointer to its AudioManager so you can avoid all this.

@@ +420,5 @@
> +    NS_ENSURE_TRUE_VOID(amService);
> +    AudioManager *am = static_cast<AudioManager *>(amService.get());
> +    int32_t forceUse = 0;
> +    am->GetForceForUse(AUDIO_POLICY_FORCE_FOR_MEDIA, &forceUse);
> +    if ((aEvent.status() != SWITCH_STATE_OFF) && sBluetoothA2dpEnabled) {

Remove the extra parenthesis.

@@ +422,5 @@
> +    int32_t forceUse = 0;
> +    am->GetForceForUse(AUDIO_POLICY_FORCE_FOR_MEDIA, &forceUse);
> +    if ((aEvent.status() != SWITCH_STATE_OFF) && sBluetoothA2dpEnabled) {
> +      am->SetForceForUse(AUDIO_POLICY_FORCE_FOR_MEDIA, AUDIO_POLICY_FORCE_NO_BT_A2DP);
> +    } else if (forceUse == AUDIO_POLICY_FORCE_NO_BT_A2DP){

Space after the )
Attachment #8500821 - Flags: review?(mwu)
Attached patch patch v1.3Splinter Review
Thanks for review that. This one fix two nits and pass audioManager to HeadphoneSwitchObserver.
Attachment #8500821 - Attachment is obsolete: true
Attachment #8500867 - Flags: review?(mwu)
Comment on attachment 8500867 [details] [diff] [review]
patch v1.3

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

Looks close. r=me with this one issue fixed.

::: dom/system/gonk/AudioManager.cpp
@@ +429,3 @@
>    }
> +private:
> +  nsRefPtr<AudioManager> mAudioManager;

Use a regular pointer. Otherwise you have a loop here that will leak both objects.
Attachment #8500867 - Flags: review?(mwu) → review+
https://hg.mozilla.org/mozilla-central/rev/d9d3423d4230
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment on attachment 8500867 [details] [diff] [review]
patch v1.3

Approval Request Comment
[Feature/regressing bug #]:NA
[User impact if declined]:
User can't hear sound from headset if they plug-in it while device has been  paired with BT devices.
[Describe test coverage new/current, TBPL]:I have tested by my self on the coexistence of headset and BT device, latest one wins.
[Risks and why]: CAF wants to solve this bug.
[String/UUID change made/needed]:NA
Attachment #8500867 - Flags: approval-mozilla-aurora?
vasanth, can you please help verify the patch fixes the issue you reported?
Flags: needinfo?(vasanth)
Attachment #8500867 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Keywords: verifyme
(In reply to bhavana bajaj [:bajaj] from comment #19)
> vasanth, can you please help verify the patch fixes the issue you reported?

One minor issue I see is when both BT headset and wired headset are connected and playback is going on through wired headset (last connected), then if we disconnect BT headset, playback through wired headset pauses.

Playback through BT headset doesn't pause when wired headset is disconnected.
Flags: needinfo?(bbajaj)
Not sure if it is ok?
Flags: needinfo?(vasanth)
(In reply to vasanth from comment #21)
> (In reply to bhavana bajaj [:bajaj] from comment #19)
> > vasanth, can you please help verify the patch fixes the issue you reported?
> 
> One minor issue I see is when both BT headset and wired headset are
> connected and playback is going on through wired headset (last connected),
> then if we disconnect BT headset, playback through wired headset pauses.
> 
> Playback through BT headset doesn't pause when wired headset is disconnected.

rlin, can help check this case.
Flags: needinfo?(bbajaj) → needinfo?(rlin)
(In reply to vasanth from comment #21)
> (In reply to bhavana bajaj [:bajaj] from comment #19)
> > vasanth, can you please help verify the patch fixes the issue you reported?
> 
> One minor issue I see is when both BT headset and wired headset are
> connected and playback is going on through wired headset (last connected),
> then if we disconnect BT headset, playback through wired headset pauses.
> 
> Playback through BT headset doesn't pause when wired headset is disconnected.
I think Music player application doesn't handle both wired headset and BT headset connected.
ni music player dkuo for further confirmation. I think this is music player behavior.
Flags: needinfo?(rlin) → needinfo?(dkuo)
The audio used to only route to the bluetooth headset even when the wired headset is inserted, so there is an implied behaviour that, when bluetooth headset(A2DP) is disconnected, music will pause itself to prevent the audio come out from the other speakers(the wired headset or the phone speaker).

This bug has changed the routing rules in audio manager, which means the above behaviour is no more suitable for those cases, we need to adjust the ux to fit the gecko/gonk changes.

We should file a new bug to fix it, and it's a regression...
Flags: needinfo?(dkuo)
Created follow up bug 1083129
(In reply to vasanth from comment #26)
> Created follow up bug 1083129

Before fix the follow up in bug 1083129, we should inform these audio routing changes to ux people, though we already landed this patch.

Jenny, please read this bug then give us some inputs in bug 1083129, thanks!
It will be better to have UX documents to record the correct behaviors.
Blocks: 1083129
(In reply to Shawn Huang [:shawnjohnjr] from comment #28)
> It will be better to have UX documents to record the correct behaviors.

Thanks Shawn, I have needinfoed Jenny in bug 1083129, let's discuss then trace in it :)
Flags: needinfo?(jelee)
This issue is verified fixed on Flame 2.2 and 2.1.

The audio is routed to the wired headset (last inserted) while the device is paired with a BT headset. 

Flame 2.2 

Device: Flame 2.2 Master (319mb)(Kitkat Base)(Full Flash)
BuildID: 20141021040206
Gaia: 457a54fc3200b80e4f5e1cd3acaa062309230732
Gecko: 29fbfc1b31aa
Gonk: 05aa7b98d3f891b334031dc710d48d0d6b82ec1d
Version: 36.0a1 (2.2 Master)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Flame 2.1 

Device: Flame 2.1 (319mb)(Kitkat Base)(Full Flash)
BuildID: 20141021001201
Gaia: e458f5804c0851eb4e93c9eb143fe044988cecda
Gecko: ee86921a986f
Gonk: 05aa7b98d3f891b334031dc710d48d0d6b82ec1d
Version: 34.0 (2.1)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Keywords: verifyme
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.