Closed Bug 961984 Opened 7 years ago Closed 7 years ago

Music shall handle play-pause avrcp key event if music launches at the first time

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:1.4+, b2g-v1.4 verified)

VERIFIED FIXED
1.4 S2 (28feb)
blocking-b2g 1.4+
Tracking Status
b2g-v1.4 --- verified

People

(Reporter: echang, Assigned: dkuo)

References

Details

Attachments

(2 files)

Device: Nexus-5
Build: master build
Gaia defe57f89ef5f6d51a6d0ac0815ff6ce0367b50d
Gecko 82a055c447b89f1db919a0f617e7a8b39f6ef584
BuildID 20140117190211
Version 29.0a1

1. Connect Nexus 5 to an AVRCP supported device, a car kit in this case.
2. Start playing music.
3. Music played through the car kit.
4. Play/Pause working on phone.
5. Play/Pause not working on car kit.
Blocks: gonk-kk
OS: Linux → Gonk (Firefox OS)
Hardware: x86_64 → ARM
I wonder that JB4.3 + 1.3 has the same problem??
Flags: needinfo?(echang)
Nexus 4 + v1.3 - not stable, FF/REV/PLAY/PAUSE sometimes failed, mostly okay. 
https://pvtbuilds.mozilla.org/pvt/mozilla.org/b2gotoro/nightly/mozilla-central-mako/2013/12/2013-12-09-05-34-02/
Flags: needinfo?(echang)
But follow the same steps, on KK, bug is 100% reproducible?
Flags: needinfo?(echang)
blocking-b2g: --- → 1.4+
After checking this bug. This bug is not related to KK specific bug, but it's bluetooth profile AVRCP 1.3 problem (bluedroid). I tested with Pioneer AVH-X5550BT. The problem is related to AVRCP 1.3 metadata notification.
Summary: [Bluetooth][AVRCP][Kitkat] Music played through car kit, controlling from phone okay, controlling from AVRCP supported device no responding → [Bluetooth][bluedroid]Music played through car kit, controlling from phone okay, controlling from AVRCP supported device no responding
Hi Eric,
I can see sometimes car kit status bar is not sync, but i did not hit the avrcp command is not responding. Can you confirm this bug again? I guess the older build, the media player (I guess stage play/pause is not correct. Thanks.
Flags: needinfo?(echang)
Sorry. I find the way to reproduce the bug on nexus5.
Flags: needinfo?(echang)
STR:
1. Connect with AVH-X5550BT
2. AVH-X5550BT would automatically send play command once AVRCP get connected
3. Music player cannot be launched automatically

Note: 
1.bluedroid stack seems indeed send out key event.
2. If Music player launched and play music first and then connect with AVH-X5550BT, AVRCP play/pause can work normally.
> Note: 
> 1.bluedroid stack seems indeed send out key event.
> 2. If Music player launched and play music first and then connect with
> AVH-X5550BT, AVRCP play/pause can work normally.
I did some further clarification here:
If Music player launched without selecting songs and play, then connect with AVH-X5550BT, AVRCP won't work.
If Music player launched and play, then connect with AVH-X5550BT, AVRCP can work.
I further checked shell.js broadcasts media-button event, but I saw error from music player.

02-24 03:54:33.662 I/GeckoDump( 2264): !!!!!! avrcp media button!!!!!!!!!!!!!
02-24 03:54:33.802 E/GeckoConsole( 2911): [JavaScript Warning: "Unknown property 'will-change'.  Declaration dropped." {file: "app://music.gaiamobile.org/style/music.css" line: 293 column: 13 source: "  will-change: scroll-position;"}]
02-24 03:54:33.802 E/GeckoConsole( 2911): [JavaScript Warning: "Unknown property 'will-change'.  Declaration dropped." {file: "app://music.gaiamobile.org/style/music.css" line: 357 column: 13 source: "  will-change: scroll-position;"}]
02-24 03:54:33.972 E/GeckoConsole( 2911): [JavaScript Error: "PlayerView is not defined" {file: "app://music.gaiamobile.org/js/communications.js" line: 45}]
02-24 03:54:33.982 E/GeckoConsole( 2911): [JavaScript Error: "NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS: [JavaScript Error: "PlayerView is not defined" {file: "app://music.gaiamobile.org/js/communications.js" line: 45}]'[JavaScript Error: "PlayerView is not defined" {file: "app://music.gaiamobile.org/js/communications.js" line: 45}]' when calling method: [nsIDOMSystemMessageCallback::handleMessage]" {file: "jar:file:///system/b2g/omni.ja!/components/SystemMessageManager.js" line: 96}]
Hey Shawn, as we discussed offline, the PlayerView probably was not ready when receiving the playpause command from the remote BT device, would you please try this WIP first? thanks.
Flags: needinfo?(dkuo)
Music player seems still cannot be launched via the very first play-pause-button press/release.

02-24 05:40:04.942 I/GeckoDump( 5639): !!!!!! avrcp media button!!!!!!!!!!!!!
02-24 05:40:04.952 I/Gecko   ( 5639): [Parent 5639] WARNING: waitpid failed pid:6460 errno:10: file ../../../gecko/ipc/chromium/src/base/process_util_posix.cc, line 254
02-24 05:40:04.952 I/Gonk    ( 5639): Setting nice for pid 6460 to 7
02-24 05:40:04.952 I/Gonk    ( 5639): Changed nice for pid 6460 from 1 to 7.
02-24 05:40:04.972 I/GeckoDump( 5639): !!!!!! avrcp media button!!!!!!!!!!!!!
02-24 05:40:05.012 E/GeckoConsole( 6460): [JavaScript Warning: "No meta-viewport tag found. Please explicitly specify one to prevent unexpected behavioural changes in future versions. For more help https://developer.mozilla.org/en/docs/Mozilla/Mobile/Viewport_meta_tag" {file: "app://music.gaiamobile.org/index.html#mix" line: 0}]
02-24 05:40:05.052 E/GeckoConsole( 6460): [JavaScript Warning: "Unknown property 'will-change'.  Declaration dropped." {file: "app://music.gaiamobile.org/style/music.css" line: 293 column: 13 source: "  will-change: scroll-position;"}]
02-24 05:40:05.052 E/GeckoConsole( 6460): [JavaScript Warning: "Unknown property 'will-change'.  Declaration dropped." {file: "app://music.gaiamobile.org/style/music.css" line: 357 column: 13 source: "  will-change: scroll-position;"}]
02-24 05:40:05.232 I/GeckoDump( 6460): MediaRemoteControls: media-play-pause-button-press
02-24 05:40:05.232 I/GeckoDump( 6460): MediaRemoteControls: media-play-pause-button-release
(In reply to Shawn Huang [:shuang] [:shawnjohnjr] from comment #12)
> Music player seems still cannot be launched via the very first
> play-pause-button press/release.
> 
I also found if launching Music but stay at main menu, instead of going into Play view. Music application will not play.
I have tested play-pause key patch for Music. It seems works. So change this bug category to Music application.
Assignee: shuang → dkuo
Component: Bluetooth → Gaia::Music
Summary: [Bluetooth][bluedroid]Music played through car kit, controlling from phone okay, controlling from AVRCP supported device no responding → [Bluetooth]Music shall handle play-pause key event if music launches at the first time
Summary: [Bluetooth]Music shall handle play-pause key event if music launches at the first time → Music shall handle play-pause avrcp key event if music launches at the first time
Blocks: 976427
Set target milestone for this is included for 2/28 set.
Target Milestone: --- → 1.4 S2 (28feb)
Attachment #8380552 - Attachment description: WIP → Check the play status then play for both play and playpause commands
Comment on attachment 8380552 [details] [review]
Check the play status then play for both play and playpause commands

Jim,

This issue is caused by the different bluetooth stacks, it seems the Bluedroid stack(in kitkat) will send the |playpause| command instead of the |play| command after A2DP is first connected and try to play the music immediately. The music communication expected to receive play and in the playpause function we didn't handle the condition that PlayerView is not ready, to fix this I have wrapped the logic for the play command into a function, for play and playpause we will both do the same logic, would you please review it? thanks.

Shawn,

I have addressed the issues we found and the patch should be good to go, would you please test it again on nexus 5 to make sure it does solve the problems? thanks.
Attachment #8380552 - Flags: review?(squibblyflabbetydoo)
Attachment #8380552 - Flags: feedback?(shuang)
I tested again your patch. It works.
Comment on attachment 8380552 [details] [review]
Check the play status then play for both play and playpause commands

I tested the patch, it works with car kit.
Attachment #8380552 - Flags: feedback?(shuang) → feedback+
(In reply to Shawn Huang [:shuang] [:shawnjohnjr] from comment #18)
> Comment on attachment 8380552 [details] [review]
> Check the play status then play for both play and playpause commands
> 
> I tested the patch, it works with car kit.

I found 'Music paused during the call' text always shows up after apply this patch. Please check again.
(In reply to Shawn Huang [:shuang] [:shawnjohnjr] from comment #19)
> (In reply to Shawn Huang [:shuang] [:shawnjohnjr] from comment #18)
> > Comment on attachment 8380552 [details] [review]
> > Check the play status then play for both play and playpause commands
> > 
> > I tested the patch, it works with car kit.
> 
> I found 'Music paused during the call' text always shows up after apply this
> patch. Please check again.

Thanks Shawn, I will check my patch and see what's going on here, I am not going to cancel the review because we can catch Jim's attention first while I am finding the issue you mentioned.
Comment on attachment 8380552 [details] [review]
Check the play status then play for both play and playpause commands

r=me with my comments.

It always seemed a bit weird to me that "play" and "playpause" meant the same thing, though. I'd expect "play" to mean "if we're paused, start playing; if we're playing, do nothing". Oh well.
Attachment #8380552 - Flags: review?(squibblyflabbetydoo) → review+
Also, make sure you fix comment 19, but you already know that. :)
(In reply to Jim Porter (:squib) from comment #21)
> Comment on attachment 8380552 [details] [review]
> Check the play status then play for both play and playpause commands
> 
> r=me with my comments.
> 
> It always seemed a bit weird to me that "play" and "playpause" meant the
> same thing, though. I'd expect "play" to mean "if we're paused, start
> playing; if we're playing, do nothing". Oh well.

Thanks for the review Jim, I don't know the exact reason why Bluedroid convert the |play| command to |playpause|, but as I commented in the patch, I believe Bluedroid is trying to be more compatible for all the bluetooth devices in the market, because some of them do not keep the play status, they will just send play or playpause via AVRCP requesting the player to play, so it just convert the commands for "please play my music" to playpause from the stack side, no matter it's |play| or |playpause|.

(In reply to Jim Porter (:squib) from comment #22)
> Also, make sure you fix comment 19, but you already know that. :)

I have found the issue in comment 19 and addressed in https://github.com/dominickuo/gaia/commit/c0a9791638d1e737957805dcbc96be363db67689, and I and doing the final check to make sure I didn't break anything, will land the patch later.
Update:

I have also added the tests and waiting for the travis to get green.
Landed on master: 1a47899c90212669b1b3ac0f067f181f2db3ba64
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Keywords: verifyme
Thansk, working fine with these 2 devices.

Nexus 4 - JB - v1.4
Gaia      a62c8df80732f9b640bfd90add5cdce71a39105c  │  
Gecko     cb6ce085e9672073c276a07bd6b1323c394d4c76  │  
BuildID   20140307043054                            │  
Version   30.0a1  

Buri v1.4
Gaia      98cf46d6623b164845fe1fdc99a2a7bf64aa667d                         │
Gecko     https://hg.mozilla.org/mozilla-central/rev/8095b7dd8f58          │
BuildID   20140306160203                                                   │
Version   30.0a1 ]
1.4 tested using Arctic P253BT Blue tooth headset.
Headset was able to play, pause, stop, forward, backward and volume up/down.

1.4 Environmental Variables:
Device: Buri 1.4 MOZ
BuildID: 20140527000202
Gaia: 0542778892a294d224e75af4a76be5d42938bc90
Gecko: d583ae109f54
Version: 30.0
Firmware Version: v1.2-device.cfg
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.