Return-Path: MIME-Version: 1.0 In-Reply-To: References: <1318627712-9725-1-git-send-email-lucas.demarchi@profusion.mobi> <1318627712-9725-4-git-send-email-lucas.demarchi@profusion.mobi> From: Lucas De Marchi Date: Mon, 17 Oct 2011 10:13:15 -0200 Message-ID: Subject: Re: [PATCH 3/5] AVRCP: respond with UINT32_MAX if duration is not available To: Luiz Augusto von Dentz Cc: linux-bluetooth@vger.kernel.org Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-bluetooth-owner@vger.kernel.org List-ID: Hi Luiz, On Mon, Oct 17, 2011 at 5:57 AM, Luiz Augusto von Dentz wrote: > Hi Lucas, > > On Sat, Oct 15, 2011 at 12:28 AM, Lucas De Marchi > wrote: >> Section 5.4.1 of AVRCP 1.3 spec says: >> >> ? ? ? ?If TG does not support SongLength And SongPosition on TG, then TG shall >> ? ? ? ?return 0xFFFFFFFF. >> >> SongPosition is always available, but song length depends on user to >> provied it. >> --- >> ?audio/avrcp.c ? ? | ? ?2 +- >> ?doc/media-api.txt | ? ?4 +++- >> ?2 files changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/audio/avrcp.c b/audio/avrcp.c >> index 8b48e83..1b55e33 100644 >> --- a/audio/avrcp.c >> +++ b/audio/avrcp.c >> @@ -808,7 +808,7 @@ static uint8_t avrcp_handle_get_play_status(struct avrcp_player *player, >> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?AVRCP_MEDIA_ATTRIBUTE_DURATION, >> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?player->user_data)); >> >> - ? ? ? duration = htonl(duration); >> + ? ? ? duration = duration ? htonl(duration) : UINT32_MAX; >> ? ? ? ?position = htonl(position); > > I would prefer doing this on RegisterPlayer/parse_player_metadata as > we do with title, so if is not present then we set UINT32_MAX. Humn... I don't know if this is good. 'Duration' is replied on both GetElementAttributes and GetPlayStatus commands and they have different ways to inform about a unavailable attribute. While in the former we should use zero-length string, the later uses UINT32_MAX. > >> ? ? ? ?memcpy(&pdu->params[0], &duration, 4); >> diff --git a/doc/media-api.txt b/doc/media-api.txt >> index b8dcdbd..204f33e 100644 >> --- a/doc/media-api.txt >> +++ b/doc/media-api.txt >> @@ -114,7 +114,9 @@ Methods ? ? ? ? ? ? void RegisterEndpoint(object endpoint, dict properties) >> >> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?uint32 Duration: >> >> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? Track duration in milliseconds >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? Track duration in milliseconds. If it's >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? set to 0 or left blank, it will be >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? treated as not available. > > 0 might be a valid duration so I would just treat as not available if > Duration is not part of the metadata or if the value is actually > UINT32_MAX which can happen if the duration exceeds 32 bits. Ok. Lucas De Marchi