953045 - [Bluetooth][Certification][PTS][BlueZ][1.3] AVRCP TC_TG_RCR_BV_02_C Failed

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Description

[Eric Chang [:ericcc] [:echang]]

Attachment: TC_TG_RCR_BV_02_C.zip

### ENV
Buri/BlueZ
[1] https://pvtbuilds.mozilla.org/pvt/mozilla.org/b2gotoro/nightly/mozilla-b2g26_v1_2-hamachi/2013/12/2013-12-17-00-40-01/

### STR
1. PTS 5.0
2. AVRCP TC_TG_RCR_BV_02_C

### Expected
Pass 

### Actual
Test case : TC_TG_RCR_BV_02_C started
	- MTC: SDP Service record was successfully registered.
	- A2DP SNK SDP Service record successfully registered
	- A2DP SNKsuccessfully registered
	- MTC: IUT successfully responded to AVDTP CONNECT.
	- MTC: IUT successfully accepted AVCTP channel connection.
	- IUT disconnected AVDTP SIGNALING channel.
	- MTC INCONC: IUT did not respond with enough data to trigger the continuing response, please configure the IUT to respond with more data.
	- AVDTP: Exiting AVDTP1 PTC
	- AVCTP: Exiting AVCTP1 PTC
	- CM_PTC_EXIT
	- MTC: Test case ended
Final Verdict : Inconclusive

Comment 1

[Jamin Liu [:jaliu][:Jamin]]

Root cause:
The PTS test case ask TG(Target Device) to play a 512KB worth of metadata media item.
However, the length of metadata are limited by bluez and it can't exceed 512KB.

The maximum of metadata strings is defined at line 155 bluez/audio/control.c.

Comment 2

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

(In reply to Jamin Liu [:jaliu] from comment #1)
> Root cause:
> The PTS test case ask TG(Target Device) to play a 512KB worth of metadata
> media item.
> However, the length of metadata are limited by bluez and it can't exceed
> 512KB.
> 
> The maximum of metadata strings is defined at line 155 bluez/audio/control.c.

It shall NOT be 512KB, shall be 512B. The reason why we cannot pass, because it depends on how we generate a track with 512 bytes metadata during testing with PTS. Even though METADATA_MAX_STRING_LEN is 150, METADATA_MAX_NUMBER_LEN is 40, you can have multiple fields (title=150, artist=150, album=150, media number=40, total media numer=40, genre=150, playingtime=40, with over 150 bytes), in this way, I believe we can pass it?

Comment 3

[Jamin Liu [:jaliu][:Jamin]]

Hi Shawn,
Thank you, you are correct.
It's been verified on v1.3 with my sample media file.

Comment 4

[Jamin Liu [:jaliu][:Jamin]]

How to run this test?
1. Prepare a media file with 512 bytes worth of metadata. (Generate a new one or download the sample test file for https://mana.mozilla.org )
2. Make sure you device has been connected with PTS A2DP profile before.
3. Start to run test case TC_TG_RCR_BV_02_C and follow the instruction.

Comment 6

[Jamin Liu [:jaliu][:Jamin]]

The verdict of this test case is "Pass" on Inari, however, the verdict is "Inconclusive" on Hamachi even they have same codebase of gecko and gaia.

The reason is that Inari & Hamachi have different bluez version.
Hamachi can't find method "UpdateMetaData" with signature "ssssss", since the interface was changed.

Inari:
UpdateMetaData(title, artist, album, media_number, total_media_count, playing_time)

Hamachi:
UpdateMetaData(title, artist, album, media_number, total_media_count, playing_time, genre)

Comment 7

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Hi Dominic,
As far as I know, Music player does not provide genre, but it's possible that other music might provide this attribute, right?
Due to AVRCP spec defined genre field, I wonder our API shall support this field, even though currently our Music application does not support.
Of course, we can reply empty string for genre argument from gonk layer, but I will like to feasibility in general how application will do (includes 3rd party application).

Comment 8

[Dominic Kuo [:dkuo]]

(In reply to Shawn Huang [:shuang] [:shawnjohnjr] from comment #7)
> Hi Dominic,
> As far as I know, Music player does not provide genre, but it's possible
> that other music might provide this attribute, right?
> Due to AVRCP spec defined genre field, I wonder our API shall support this
> field, even though currently our Music application does not support.
> Of course, we can reply empty string for genre argument from gonk layer, but
> I will like to feasibility in general how application will do (includes 3rd
> party application).

Yes you are correct, but currently the metadata parser does not parse the genre field, this sounds like a new feature for music app. And if the gecko api or the gonk layer can fix the genre field automatically, that will be great because we are encouraged not to restrict the implementation of 3rd party apps, also we can pass the certification. And if it's going to happen, should it apply to all the other fields(album, artist...) as well?

And if the genre field is a must-have in AVRCP 1.3 spec, then we should fix this to pass the certification, let me know if you guys are blocked by this, thanks.

Comment 9

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Hi Dominic, Thanks for your comments. I think let's skip genre field this first.

Comment 10

[Jamin Liu [:jaliu][:Jamin]]

Attachment: [AVRCP] Fix a bug of bluetooth AVRCP to support media metadata updating correctly. (v1.0)

Comment 11

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

Comment on attachment 8361546 [details] [diff] [review]
[AVRCP] Fix a bug of bluetooth AVRCP to support media metadata updating correctly. (v1.0)

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

::: dom/bluetooth/bluez/linux/BluetoothDBusService.cpp
@@ +3138,5 @@
> +  // We currently don't support genre field in music player.
> +  // In order to send media metadata through AVRCP, we set genre to an empty
> +  // string to match the BlueZ method "UpdateMetaData" with signature "sssssss",
> +  // which takes genre field as the last parameter.
> +  const char* genre = EmptyCString().get();

I don't think genre would be a valid pointer in this case. EmptyCString() creates a temporary instance and it could be free'd any time after this line. Please use a nsCString to hold a reference to the empty string.

Comment 12

[Jamin Liu [:jaliu][:Jamin]]

Attachment: [AVRCP] Fix a bug of bluetooth AVRCP to support media metadata updating correctly. (v2)

Eric,
Thank you.
I uploaded a new patch to fix the problem.

Comment 13

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

Comment on attachment 8361567 [details] [diff] [review]
[AVRCP] Fix a bug of bluetooth AVRCP to support media metadata updating correctly. (v2)

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

LGTM. Thanks.

Comment 14

[Jamin Liu [:jaliu][:Jamin]]

It looks fine on try server.
https://tbpl.mozilla.org/?tree=Try&rev=9eb435b8bfbc

I've verified this patch on Hamachi with test cases by running test cases C_TG_RCR_BV_02_C and C_TG_RCR_BV_04_C.

Comment 15

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

Nominate as 1.3+ since it's a cert blocker.

Comment 16

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

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

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/123dc70cd915

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7ed35166821a

Comment 19

[Eric Chang [:ericcc] [:echang]]

Okay with this build.
│ Your Target Build: PVT.v130.hamachi                                        │
│ URL:                                                                       │
│ https://pvtbuilds.mozilla.org/pvt/mozilla.org/b2gotoro/nightly/mozilla-aur │
│ ora-hamachi-eng/latest/                                                    │
│ ENG Ver: true                                                              │
│ Flash: Gaia, Gecko,                                                        │
│                                                                            │
│ Gaia      6fbeac2415f07f10de181f0877ddf67ee299b885                         │
│ Gecko     http://hg.mozilla.org/releases/mozilla-aurora/rev/e8f6bdf8db3d   │
│ BuildID   20140123004001                                                   │
│ Version   28.0a2                                                           │
│ ro.build.version.incremental=eng.archermind.20131114.105818                │
│ ro.build.date=Thu Nov 14 10:58:33 CST 2013

Comment 20

[Pham Van Huynh]

Hello, 

Please send for me meta data file to test for this test case. I can't download it in mozilla.org, cause I can't create account in this website. 

many thanks