1037359 - [dolphin][flame] Music came out from device for 1~2s and then became paused when user turned off BT headset

Status: RESOLVED FIXED

(Firefox OS Graveyard :: AudioChannel, defect)

Description

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

Attachment: music.log

Steps to reproduce
----------------------------------------------------------------------
0. Pair a BT headset to device
1. Launch Music and play some song
   Music comes out from BT headset
2. Turn off BT headset or disconnect BT headset
   --> Music will come out from device for about 1-2s and then paused.

[expected] Music should directly pause.

Issue can be reproduced on both flame v1.4 an ddolphin.

Attached is adb log. Test starts: 07-11 16:48:02.101 

Test build
---------------------------------------------------------------
Gaia      b0e9b4bdb39c5eb93a6783a34624ffc84f62b126
Gecko     https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/ccabaf8826a4
BuildID   20140710000202
Version   30.0

Comment 1

[Ivan Tsay (:ITsay)]

Low user impact and not a regression. Move it to 2.1.

Hi Eric,

Mind if you can have someone to take a look this issue and see if it can be fixed in 2.1 time frame?

Comment 2

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

Star / Wayne,

IIRC we have dealt with another issue which was very similar to this. Could you guys give some feedback?

Comment 3

[Star Cheng [:scheng]]

(In reply to Eric Chou [:ericchou] [:echou] from comment #2)
> Star / Wayne,
> 
> IIRC we have dealt with another issue which was very similar to this. Could
> you guys give some feedback?

The root cause: switching audio routing path is earlier than notifying music app to pause itself. 
Wayne and I have discussed that issue and provided a solution to resolve it. 

Hi, Wayne

Could you help to provide the patch? 
Thanks

Comment 4

[Wayne Chen [:xwaynec]]

Attachment: bug-1037359-fix.patch

When user turn off bt headset, delay routing to avoid leakage of audio from speaker

Comment 5

[Star Cheng [:scheng]]

According to comment 4, Wayne has provided the patch. So assign owner to him.

Comment 6

[Michael Wu [:mwu]]

Comment on attachment 8459480 [details] [diff] [review]
bug-1037359-fix.patch

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

::: dom/system/gonk/AudioManager.cpp
@@ +238,5 @@
>    }
>  }
>  
> +static void
> +InternalSetA2dpRoutes(audio_policy_dev_state_t aState, const char *aAddress)

Inline this function into ProcessDelayedA2dpRoute since there is only one user.

@@ +242,5 @@
> +InternalSetA2dpRoutes(audio_policy_dev_state_t aState, const char *aAddress)
> +{
> +  if (static_cast<
> +      status_t (*)(audio_devices_t, audio_policy_dev_state_t, const char*)
> +      >(AudioSystem::setDeviceConnectionState)) {

This check wasn't necessary before - why add it now?

@@ +300,5 @@
> +      sA2dpSwitchDone = false;
> +      String8 cmd("bluetooth_enabled=false");
> +      AudioSystem::setParameters(0, cmd);
> +      cmd.setTo("A2dpSuspended=true");
> +      AudioSystem::setParameters(0, cmd);

What's the point of setting parameters here if you're also doing it in the runnable?

@@ +309,5 @@
>        String8 cmd("bluetooth_enabled=true");
>        AudioSystem::setParameters(0, cmd);
>        cmd.setTo("A2dpSuspended=false");
>        AudioSystem::setParameters(0, cmd);
> +    } 

Trailing whitespace here.

Comment 7

[Wayne Chen [:xwaynec]]

Attachment: [central][v2.0]bug-1037359-fix-v2.patch

Hi Michael,

I'm sorry for trouble you again.

Sincerely,
Wayne

Comment 8

[Wayne Chen [:xwaynec]]

https://tbpl.mozilla.org/?tree=Try&rev=7f51dda79d7d

Comment 9

[Wayne Chen [:xwaynec]]

Attachment: [1.3][1.3t][1.4]bug-1037359-fix-v2.patch

Comment 10

[Carsten Book [:Tomcat]]

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

Comment 11

[Ryan VanderMeulen [:RyanVM]]

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

Comment 12

[James Zhang (Spreadtrum)]

Please land this patch to 1.4.

Comment 13

[Rachelle Yang [:ryang][[e-mail]]]

[Blocking Requested - why for this release]:

Change to 1.4 ?
Per comment9, the patch is ready on 1.4.Thanks !

Comment 14

[Wayne Chang [:wchang]]

Minor issue and same symptom observed on many android devices too.

Comment 15

[soyeon]

Hi, Wayne Chang.

I want to affect the modified patch on b2g 2.0 , too.

Could you help to provide the patch?
Thanks for helping me.

Comment 16

[Wayne Chang [:wchang]]

[Blocking Requested - why for this release]:

It will be included in 2.1.

Reverting the flag to pre-comment 13 for triage.