897882 - [Music] Support bluetooth AVRCP 1.0 + 1.3

Status: RESOLVED FIXED

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

Description

[Dominic Kuo [:dkuo]]

This bug is filed for the integration of bluetooth AVRCP 1.0 and Music app.

Since the proposed MediaRemoteControl api is not ready yet, and bluetooth team and QA need AVRCP 1.0 to be landed on gaia master so that they can test, the new features in music app for AVRCP 1.0 will be implemented and traced by this bug.

Comment 1

[Dominic Kuo [:dkuo]]

Update:

There will be a new webapi called "Inter App Communication" instead of MediaRemoteControl api, and we probably should consider to use it to handle the AVRCP commands.
https://wiki.mozilla.org/WebAPI/Inter_App_Communication_Alt_proposal

Comment 2

[Dominic Kuo [:dkuo]]

Update:

The commands for AVRCP 1.0 are all handled and can interact with the music app in my test branch, but before landing it, the code must be cleaned up and some unit tests will be probably needed.

Comment 5

[Dominic Kuo [:dkuo]]

This bug now covers the integration of AVRCP 1.0 + 1.3 and will be landed together.

Comment 6

[Dominic Kuo [:dkuo]]

Attachment: Support bluetooth AVRCP 1.0 + 1.3

Jim,

This patch basically implemented the remote media controls with AVRCP commands enabled that I proposed, I am requesting a feedback from you because it's better that you can take a look on it first then give some feedbacks before I finalize it, such as how you think we can modify to adapt your design of the music controls in system app, after that, I will request for a formal review and you can review in detail.

Comment 7

[Jim Porter (:squib)]

Comment on attachment 798963 [details]
Support bluetooth AVRCP 1.0 + 1.3

This generally seems good. I have a bunch of minor comments about the API. It might also make sense to have a few other people who like designing APIs take a look at this. Since we're putting this in /shared/js/, we definitely want to make sure we make the API as easy to use as possible so that other people* can take advantage of it without too much trouble.

* Probably just media team people for now, but I expect third parties will want to use this if they make their own media apps.

Comment 8

[Dominic Kuo [:dkuo]]

Comment on attachment 798963 [details]
Support bluetooth AVRCP 1.0 + 1.3

Jim,

Okay, I think I have addressed all the issues you mentioned on github, now the modified media remote controls should fit your code on the IAC part, before I finished rewriting the unit tests, can you please take a look on it again? hope I can make it before you go to Oslo, I will ping you again after I done the tests, thanks.

David,

This patch is the media remote controls that I mentioned in the email, I have discussed with Jim and came out this design pattern, and we both think it will be nice that you can also have a look on it, can you also give us some feedback if you are probably available? thanks.

Comment 9

[Dominic Kuo [:dkuo]]

I finished rewriting the unit tests, basically I tested all the commands from AVRCP and IAC, and make sure they all fired to the command handlers and the received commands in events are correct.

Comment 10

[Dominic Kuo [:dkuo]]

Attachment: https://dl.dropboxusercontent.com/u/23245590/fxos12_avrcp13_demo.MTS

Thanks to Shawn's help, we got a fantastic demo video!

Jim, while you are reviewing, if you don't have a bluetooth device that supports AVRCP 1.0 or 1.3, you can see how the patch actually works and interacts with the bluetooth car kit, hope this helps on the review.(We used the car kit's remote control to demo because we don't want the fingers to cover the display screen)

Comment 11

[Jim Porter (:squib)]

Comment on attachment 798963 [details]
Support bluetooth AVRCP 1.0 + 1.3

This is looking pretty good! I only have a couple of minor comments on Github about this. r- for now just because some of the changes might be subtle, and I want to take a quick look after you make them, but this is really close now. It should be a good basis for the IAC code I'll be landing soon.

Comment 12

[Dominic Kuo [:dkuo]]

Comment on attachment 798963 [details]
Support bluetooth AVRCP 1.0 + 1.3

Okay, I think I got all the issues addressed(For those I couldn't I have commented why) and this version should be good to go. Jim, can you please take another look on the patch? I hope everything looks good to you, thanks!

Comment 13

[Jim Porter (:squib)]

Comment on attachment 798963 [details]
Support bluetooth AVRCP 1.0 + 1.3

This looks good, and the unit tests pass. Unfortunately, I can't test it for real, since I lack a Bluetooth remote, but the video is good enough for me!

Comment 14

[Dominic Kuo [:dkuo]]

Thanks Jim! really appreciate all the reviews and suggestions.

Landed on master: ebbb325958c66c3e68e1f190472d5f6e9076d83a

Comment 15

[Dominic Kuo [:dkuo]]

Comment on attachment 798963 [details]
Support bluetooth AVRCP 1.0 + 1.3

Cancelling the feedback request since the patch is landed.