Closed Bug 1037359 Opened 8 years ago Closed 8 years ago

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

Categories

(Firefox OS Graveyard :: AudioChannel, defect)

Other
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(b2g-v1.3 affected, b2g-v1.3T affected, b2g-v1.4 affected, b2g-v2.0 ?)

RESOLVED FIXED
2.1 S1 (1aug)
Tracking Status
b2g-v1.3 --- affected
b2g-v1.3T --- affected
b2g-v1.4 --- affected
b2g-v2.0 --- ?

People

(Reporter: angelc04, Assigned: xwaynec)

References

Details

(Whiteboard: [sprd328513])

Attachments

(3 files, 1 obsolete file)

Attached file 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
Whiteboard: [sprd328513][partner-blocker]
blocking-b2g: --- → 1.4?
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?
blocking-b2g: 1.4? → 2.1?
Flags: needinfo?(echou)
Star / Wayne,

IIRC we have dealt with another issue which was very similar to this. Could you guys give some feedback?
Flags: needinfo?(waychen)
Flags: needinfo?(scheng)
Flags: needinfo?(echou)
(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
Flags: needinfo?(scheng)
Attached patch bug-1037359-fix.patch (obsolete) — Splinter Review
When user turn off bt headset, delay routing to avoid leakage of audio from speaker
Attachment #8459480 - Flags: review?(mwu)
Flags: needinfo?(waychen)
According to comment 4, Wayne has provided the patch. So assign owner to him.
Assignee: nobody → waychen
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.
Attachment #8459480 - Flags: review?(mwu)
Whiteboard: [sprd328513][partner-blocker] → [sprd328513]
Hi Michael,

I'm sorry for trouble you again.

Sincerely,
Wayne
Attachment #8462367 - Flags: review?(mwu)
Attachment #8462367 - Flags: review?(mwu) → review+
status-b2g-v2.1: ? → ---
Attachment #8462367 - Attachment description: bug-1037359-fix-v2.patch → [central][v2.0]bug-1037359-fix-v2.patch
Attachment #8459480 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/e6190e3220b6
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S1 (1aug)
Please land this patch to 1.4.
Flags: needinfo?(ryang)
[Blocking Requested - why for this release]:

Change to 1.4 ?
Per comment9, the patch is ready on 1.4.Thanks !
blocking-b2g: 2.1? → 1.4?
Flags: needinfo?(ryang)
Minor issue and same symptom observed on many android devices too.
blocking-b2g: 1.4? → ---
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.
Flags: needinfo?(wchang)
[Blocking Requested - why for this release]:

It will be included in 2.1.

Reverting the flag to pre-comment 13 for triage.
blocking-b2g: --- → 2.1?
Flags: needinfo?(wchang)
blocking-b2g: 2.1? → ---
Duplicate of this bug: 1000649
Depends on: 1070919
You need to log in before you can comment on or make changes to this bug.