[MADAI][Bluetooth] avrcp metadata doesn't update on carkit.

RESOLVED FIXED in Firefox 34

Status

defect
--
blocker
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ssstyle83, Assigned: shawnjohnjr)

Tracking

unspecified
2.1 S5 (26sep)

Firefox Tracking Flags

(blocking-b2g:2.0+, firefox33 wontfix, firefox34 fixed, firefox35 fixed, b2g-v2.0 fixed, b2g-v2.0M fixed, b2g-v2.1 fixed, b2g-v2.2 fixed)

Details

(Whiteboard: [p=3])

Attachments

(4 attachments, 7 obsolete attachments)

4.64 MB, application/x-zip-compressed
Details
1.39 KB, patch
Details | Diff | Splinter Review
1.41 KB, patch
Details | Diff | Splinter Review
1.41 KB, patch
Details | Diff | Splinter Review
User Agent: Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 6.1; Trident/4.0; CNS_UA; AD_LOGON=4C47452E4E4554; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0; .NET4.0C; InfoPath.3; .NET4.0E; Tablet PC 2.0; CNS_UA; AD_LOGON=4C47452E4E4554)

Steps to reproduce:

1. pair with carkit
2. play music
3. check AVRCP metadata on carkit.


Actual results:

metadata doesn't update (100%)


Expected results:

metadata update
Severity: normal → blocker
Is that a regression?
Component: General → Bluetooth
Can you provide sniffer log?
Flags: needinfo?(ssstyle83)
I think Madai is taking v2.0 gecko.
Posted file air sniff.zip
Flags: needinfo?(ssstyle83)
I attached sniff log.

this issue is not reproduced specific carkit.
you can check any carkit which supported metadata.
Thanks! I will try on our real car kit.
Assignee: nobody → shuang
Whiteboard: [p=3] → [p=3][ETA:9/16]
Sorry for late reply. I shall have some update on 9/12 anyway.
Flags: needinfo?(shuang)
I tried with gecko v2.0 on Nexus5, it seems metadata will be sent, but on flame-kk (CAF bluedroid stack) release, it doesn't work.
Flags: needinfo?(shuang)
I suspect something wrong with SDP. I tried to manually retrieve all elements (Title, album, author) and it looks fine. However, the car kit never gets capability and query avrcp metadata, or GetPlayStatus.
I agree about the Shawn's opinion.

I have found the broken AVRCP attribute log.
Thus I am reviewing the AVRCP sdp codes.
(In reply to ILBEOM KIM from comment #11)
> I agree about the Shawn's opinion.
> 
> I have found the broken AVRCP attribute log.
> Thus I am reviewing the AVRCP sdp codes.
Ya, this just my suspect, if this is something related to CAF bluedroid?
Per Comment 11, does this bug also happen at your side?
Flags: needinfo?(bhargavg1)
(In reply to Shawn Huang [:shawnjohnjr] from comment #13)
> Per Comment 11, does this bug also happen at your side?

MADAI sent the wrong playstatus when register notification.

I modified the initialized playstatus variable.

mPlayStatus = ControlPlayStatus::PLAYSTATUS_UNKNOWN;
--->
mPlayStatus = ControlPlayStatus::PLAYSTATUS_STOPPED;

bluedroid hasn't the exception handler about the unknown(0x05) value.

avrc_bld_tg.c
    case AVRC_EVT_PLAY_STATUS_CHANGE:       /* 0x01 */
        /* p_rsp->param.play_status >= AVRC_PLAYSTATE_STOPPED is always TRUE */
        if ((p_rsp->param.play_status <= AVRC_PLAYSTATE_REV_SEEK) ||
            (p_rsp->param.play_status == AVRC_PLAYSTATE_ERROR) )
        {
            UINT8_TO_BE_STREAM(p_data, p_rsp->param.play_status);
            len = 2;
        }
        else
        {
            AVRC_TRACE_ERROR0("bad play state");
            status = AVRC_STS_BAD_PARAM;
        }
        break;

If received the unknown value(0x05), stack returns the BAD_PARAM.

I have tested after applied patch and carkit successfully display the metadata.

If you reproduced same case, please check the initialized playstatus value.

Thanks.
Flags: needinfo?(bhargavg1)
mPlayStatus = ControlPlayStatus::PLAYSTATUS_UNKNOWN;
--->
mPlayStatus = ControlPlayStatus::PLAYSTATUS_STOPPED;

Above codes is located in the BluetoothA2dpManager.cpp.
function : resetAVRCP()
I agreed with you. But I wondered is there anything wrong with SDP part?
[Blocking Requested - why for this release]:
I tried to change the initialized value and test with our Pioneer car kit system, I looks like car kit still doesn't fetch AVRCP element.
Could you check the AVRCP SDP version?

Default value of the bluedroid is the 1.5.

I changed the "#define SDP_AVRCP_1_5 FALSE" in the bdroid_buildcfg.h or b2g_bdroid_buildcfg.h

You can find the SDP_AVRCP_1_5 in the bt_target.h(external/bluetooth/bluedroid/include)

#ifndef SDP_AVRCP_1_5
#define SDP_AVRCP_1_5               TRUE
(In reply to ILBEOM KIM from comment #20)
> Could you check the AVRCP SDP version?
> 
> Default value of the bluedroid is the 1.5.
> 
> I changed the "#define SDP_AVRCP_1_5 FALSE" in the bdroid_buildcfg.h or
> b2g_bdroid_buildcfg.h
> 
> You can find the SDP_AVRCP_1_5 in the
> bt_target.h(external/bluetooth/bluedroid/include)
> 
> #ifndef SDP_AVRCP_1_5
> #define SDP_AVRCP_1_5               TRUE

Yes, now it works. This looks like this is something relate to AVRCP 1.5 change.
https://www.codeaurora.org/cgit/quic/la/platform/external/bluetooth/bluedroid/tree/include/bt_target.h?h=LNX.LF.3.5.1_RB1.2&id=5626eaf8d2997a0e3e20ad49705910eda91b22e4#n3523

We will disable avrcp 1.5 SDP from b2g_bdroid_buildcfg.h.
Thanks!
Attachment #8491427 - Attachment description: bug1062697.patch (v2.1) → Bug 1062697 - Initialize avrcp play status using the proper value, disable AvRCP 1.5 (v2.1)
[Blocking Requested - why for this release]:
For me, this seems AVRCP 1.3 metadata function breaks on certain avrcp 1.3 devices. Based on Comment 14, and Comment 20. We need to override bluedroid setting and disable avrcp 1.5 feature from stack side. Otherwise, car kits won't send any request to get metadata elements.
blocking-b2g: --- → 2.0?
Keep ni flag. I will rebase patch for master, and v2.1 today.
Flags: needinfo?(shuang)
Hi Shawn, thanks for your efforts. 
As discussed, please kindly provide patch for 2.0 and master. Thank you very much.
blocking-b2g: 2.0? → 2.0+
Flags: needinfo?(shuang)
Attachment #8493594 - Attachment description: bug1062697-v2.0.patch → Bug 1062697 - Initialize avrcp play status using the proper value, disable AvRCP 1.5 (v2.0)
I'm not sure why many car kits cannot handle AVRCP 1.5 SDP record as Comment 20. It seems some IOT problems can be observed after adding AVRCP 1.5 SDP based on my test result. Have you seen the same result on your reference platform? I afraid that even we disabled AVRCP 1.5, reference platform might have the same problem.
Flags: needinfo?(bhargavg1)
Attachment #8493607 - Attachment description: Bug 1062697 - Initialize avrcp play status using the proper value, disable AvRCP 1.5 → Bug 1062697 - Initialize avrcp play status using the proper value, disable AvRCP 1.5 (master)
Attachment #8493607 - Flags: review?(btian)
Comment on attachment 8493607 [details] [diff] [review]
Bug 1062697 - Initialize avrcp play status using the proper value, disable AvRCP 1.5 (master)

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

r=me with nit addressed.

::: dom/bluetooth/bluedroid/b2g_bdroid_buildcfg.h
@@ +37,5 @@
>                               BTA_AG_FEAT_EXTERR)
>  
>  /* CHLD values */
>  #define BTA_AG_CHLD_VAL    "(0,1,2,3)"
> +/* SDP AVRCP 1.5 feature */

nit: add newline before and after the definition.
Attachment #8493607 - Flags: review?(btian) → review+
Comment on attachment 8493594 [details] [diff] [review]
Bug 1062697 - Initialize avrcp play status using the proper value, disable AvRCP 1.5 (v2.0)

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

r=me with nit addressed.

::: dom/bluetooth/bluedroid/b2g_bdroid_buildcfg.h
@@ +37,5 @@
>                               BTA_AG_FEAT_EXTERR)
>  
>  /* CHLD values */
>  #define BTA_AG_CHLD_VAL    "(0,1,2,3)"
> +/* SDP AVRCP 1.5 feature */

nit: add newline before and after the definition.
Attachment #8493594 - Flags: review?(btian) → review+
https://hg.mozilla.org/mozilla-central/rev/031c36114094
Status: UNCONFIRMED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S5 (26sep)
Please nominate this for aurora and b2g32 approval when you get a chance.
Flags: needinfo?(shuang)
Comment on attachment 8494439 [details] [diff] [review]
Bug 1062697 - Initialize avrcp play status using the proper value, disable AvRCP 1.5, r=btian (v2.0)

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 939500 and change from CAF bluedroid stack
User impact if declined: Car kit doesn't get metadata album/title/artist sync
Testing completed:Car kit query metadata and displayed
Risk to taking this patch (and alternatives if risky):Low risk, the patch only change default play status value as bluedroid can handle and disable AVRCP 1.5 SDP record.
String or UUID changes made by this patch:None
Attachment #8494439 - Flags: approval-mozilla-b2g32?
Flags: needinfo?(shuang)
Comment on attachment 8494443 [details] [diff] [review]
Bug 1062697 - Initialize avrcp play status using the proper value, disable AvRCP 1.5, r=btian (v2.1)

Approval Request Comment
[Feature/regressing bug #]: 939500 and change from CAF bluedroid stack
[User impact if declined]:Car kit doesn't get metadata album/title/artist sync
Testing completed:Car kit query metadata and displayed
[Describe test coverage new/current, TBPL]: This can only tested with real device and interact with bluedroid stack
[Risks and why]: Low risk, the patch only change default play status value as bluedroid can handle and disable AVRCP 1.5 SDP record.
[String/UUID change made/needed]: None



[Approval Request Comment]
Bug caused by (feature/regressing bug #):
User impact if declined: 
Risk to taking this patch (and alternatives if risky):Low risk, the patch only change default play status value as bluedroid can handle and disable AVRCP 1.5 SDP record.
String or UUID changes made by this patch:None
Attachment #8494443 - Flags: approval-mozilla-aurora?
Attachment #8494439 - Flags: approval-mozilla-b2g32? → approval-mozilla-b2g32+
Attachment #8494443 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: needinfo?(bhargavg1)
You need to log in before you can comment on or make changes to this bug.