* [PATCH 1/2] AVRCP: return empty string instead of rejecting
@ 2011-09-28 22:35 Lucas De Marchi
2011-09-28 22:35 ` [PATCH 2/2] AVRCP: fix case when only one setting is provided Lucas De Marchi
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Lucas De Marchi @ 2011-09-28 22:35 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Lucas De Marchi
If media attribute is not available for a certain media file, return an
empty string instead of rejecting the request. The spec is not so clear
if only the title should be handled as an empty string when not present,
but this is the only alternative to rejecting the request.
IOP tests showed that some CT devices don't like reject messages: they
never ask for an attribute again if they previously received a REJECTED
message for that attribute. They consider REJECTED as "TG doesn't
implement it these optional attributes" as opposed to what we had
before, "this attribute is currently not available".
---
audio/avrcp.c | 41 +++++++++++++++++++----------------------
1 files changed, 19 insertions(+), 22 deletions(-)
diff --git a/audio/avrcp.c b/audio/avrcp.c
index ba04a12..e7b0d1b 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -678,46 +678,43 @@ static int mp_get_media_attribute(struct media_player *mp,
valp = mi->title;
break;
case MEDIA_INFO_ARTIST:
- if (mi->artist == NULL)
- return -ENOENT;
-
valp = mi->artist;
break;
case MEDIA_INFO_ALBUM:
- if (mi->album == NULL)
- return -ENOENT;
-
valp = mi->album;
break;
case MEDIA_INFO_GENRE:
- if (mi->genre == NULL)
- return -ENOENT;
-
valp = mi->genre;
break;
case MEDIA_INFO_TRACK:
- if (!mi->track)
- return -ENOENT;
+ if (mi->track) {
+ snprintf(valstr, 20, "%u", mi->track);
+ valp = valstr;
+ } else {
+ valp = NULL;
+ }
- snprintf(valstr, 20, "%u", mi->track);
- valp = valstr;
break;
case MEDIA_INFO_N_TRACKS:
- if (!mi->ntracks)
- return -ENOENT;
+ if (mi->ntracks) {
+ snprintf(valstr, 20, "%u", mi->ntracks);
+ valp = valstr;
+ } else {
+ valp = NULL;
+ }
- snprintf(valstr, 20, "%u", mi->ntracks);
- valp = valstr;
break;
case MEDIA_INFO_PLAYING_TIME:
- if (mi->track_len == 0xFFFFFFFF)
- return -ENOENT;
+ if (mi->track_len == 0xFFFFFFFF) {
+ snprintf(valstr, 20, "%u", mi->track_len);
+ valp = valstr;
+ } else {
+ valp = NULL;
+ }
- snprintf(valstr, 20, "%u", mi->track_len);
- valp = valstr;
break;
default:
- return -EINVAL;
+ return -ENOENT;
}
if (valp) {
--
1.7.6.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/2] AVRCP: fix case when only one setting is provided
2011-09-28 22:35 [PATCH 1/2] AVRCP: return empty string instead of rejecting Lucas De Marchi
@ 2011-09-28 22:35 ` Lucas De Marchi
2011-09-30 14:55 ` Luiz Augusto von Dentz
2011-09-30 14:54 ` [PATCH 1/2] AVRCP: return empty string instead of rejecting Luiz Augusto von Dentz
2011-10-03 8:54 ` Johan Hedberg
2 siblings, 1 reply; 5+ messages in thread
From: Lucas De Marchi @ 2011-09-28 22:35 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Lucas De Marchi
If CT tries to change an Application Setting providing only one
setting, the size of the PDU will be 3 bytes. Therefore we should check
if the PDU is shorter than or equal 3, not only shorter.
---
audio/avrcp.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/audio/avrcp.c b/audio/avrcp.c
index e7b0d1b..ac9a107 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -1021,7 +1021,7 @@ static uint8_t avrcp_handle_set_player_value(struct media_player *mp,
uint16_t len = ntohs(pdu->params_len);
unsigned int i;
- if (len < 3)
+ if (len <= 3)
goto err;
len = 0;
--
1.7.6.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] AVRCP: fix case when only one setting is provided
2011-09-28 22:35 ` [PATCH 2/2] AVRCP: fix case when only one setting is provided Lucas De Marchi
@ 2011-09-30 14:55 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2011-09-30 14:55 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: linux-bluetooth
Hi Lucas,
On Thu, Sep 29, 2011 at 1:35 AM, Lucas De Marchi
<lucas.demarchi@profusion.mobi> wrote:
> If CT tries to change an Application Setting providing only one
> setting, the size of the PDU will be 3 bytes. Therefore we should check
> if the PDU is shorter than or equal 3, not only shorter.
> ---
> audio/avrcp.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/audio/avrcp.c b/audio/avrcp.c
> index e7b0d1b..ac9a107 100644
> --- a/audio/avrcp.c
> +++ b/audio/avrcp.c
> @@ -1021,7 +1021,7 @@ static uint8_t avrcp_handle_set_player_value(struct media_player *mp,
> uint16_t len = ntohs(pdu->params_len);
> unsigned int i;
>
> - if (len < 3)
> + if (len <= 3)
> goto err;
>
> len = 0;
> --
> 1.7.6.4
>
> --
Ack.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] AVRCP: return empty string instead of rejecting
2011-09-28 22:35 [PATCH 1/2] AVRCP: return empty string instead of rejecting Lucas De Marchi
2011-09-28 22:35 ` [PATCH 2/2] AVRCP: fix case when only one setting is provided Lucas De Marchi
@ 2011-09-30 14:54 ` Luiz Augusto von Dentz
2011-10-03 8:54 ` Johan Hedberg
2 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2011-09-30 14:54 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: linux-bluetooth
Hi Lucas,
On Thu, Sep 29, 2011 at 1:35 AM, Lucas De Marchi
<lucas.demarchi@profusion.mobi> wrote:
> If media attribute is not available for a certain media file, return an
> empty string instead of rejecting the request. The spec is not so clear
> if only the title should be handled as an empty string when not present,
> but this is the only alternative to rejecting the request.
>
> IOP tests showed that some CT devices don't like reject messages: they
> never ask for an attribute again if they previously received a REJECTED
> message for that attribute. They consider REJECTED as "TG doesn't
> implement it these optional attributes" as opposed to what we had
> before, "this attribute is currently not available".
> ---
> audio/avrcp.c | 41 +++++++++++++++++++----------------------
> 1 files changed, 19 insertions(+), 22 deletions(-)
>
> diff --git a/audio/avrcp.c b/audio/avrcp.c
> index ba04a12..e7b0d1b 100644
> --- a/audio/avrcp.c
> +++ b/audio/avrcp.c
> @@ -678,46 +678,43 @@ static int mp_get_media_attribute(struct media_player *mp,
> valp = mi->title;
> break;
> case MEDIA_INFO_ARTIST:
> - if (mi->artist == NULL)
> - return -ENOENT;
> -
> valp = mi->artist;
> break;
> case MEDIA_INFO_ALBUM:
> - if (mi->album == NULL)
> - return -ENOENT;
> -
> valp = mi->album;
> break;
> case MEDIA_INFO_GENRE:
> - if (mi->genre == NULL)
> - return -ENOENT;
> -
> valp = mi->genre;
> break;
> case MEDIA_INFO_TRACK:
> - if (!mi->track)
> - return -ENOENT;
> + if (mi->track) {
> + snprintf(valstr, 20, "%u", mi->track);
> + valp = valstr;
> + } else {
> + valp = NULL;
> + }
>
> - snprintf(valstr, 20, "%u", mi->track);
> - valp = valstr;
> break;
> case MEDIA_INFO_N_TRACKS:
> - if (!mi->ntracks)
> - return -ENOENT;
> + if (mi->ntracks) {
> + snprintf(valstr, 20, "%u", mi->ntracks);
> + valp = valstr;
> + } else {
> + valp = NULL;
> + }
>
> - snprintf(valstr, 20, "%u", mi->ntracks);
> - valp = valstr;
> break;
> case MEDIA_INFO_PLAYING_TIME:
> - if (mi->track_len == 0xFFFFFFFF)
> - return -ENOENT;
> + if (mi->track_len == 0xFFFFFFFF) {
> + snprintf(valstr, 20, "%u", mi->track_len);
> + valp = valstr;
> + } else {
> + valp = NULL;
> + }
>
> - snprintf(valstr, 20, "%u", mi->track_len);
> - valp = valstr;
> break;
> default:
> - return -EINVAL;
> + return -ENOENT;
> }
>
> if (valp) {
> --
> 1.7.6.4
Ack.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 1/2] AVRCP: return empty string instead of rejecting
2011-09-28 22:35 [PATCH 1/2] AVRCP: return empty string instead of rejecting Lucas De Marchi
2011-09-28 22:35 ` [PATCH 2/2] AVRCP: fix case when only one setting is provided Lucas De Marchi
2011-09-30 14:54 ` [PATCH 1/2] AVRCP: return empty string instead of rejecting Luiz Augusto von Dentz
@ 2011-10-03 8:54 ` Johan Hedberg
2 siblings, 0 replies; 5+ messages in thread
From: Johan Hedberg @ 2011-10-03 8:54 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: linux-bluetooth
Hi Lucas,
On Wed, Sep 28, 2011, Lucas De Marchi wrote:
> If media attribute is not available for a certain media file, return an
> empty string instead of rejecting the request. The spec is not so clear
> if only the title should be handled as an empty string when not present,
> but this is the only alternative to rejecting the request.
>
> IOP tests showed that some CT devices don't like reject messages: they
> never ask for an attribute again if they previously received a REJECTED
> message for that attribute. They consider REJECTED as "TG doesn't
> implement it these optional attributes" as opposed to what we had
> before, "this attribute is currently not available".
> ---
> audio/avrcp.c | 41 +++++++++++++++++++----------------------
> 1 files changed, 19 insertions(+), 22 deletions(-)
Both patches applied. Thanks.
Johan
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-10-03 8:54 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-28 22:35 [PATCH 1/2] AVRCP: return empty string instead of rejecting Lucas De Marchi
2011-09-28 22:35 ` [PATCH 2/2] AVRCP: fix case when only one setting is provided Lucas De Marchi
2011-09-30 14:55 ` Luiz Augusto von Dentz
2011-09-30 14:54 ` [PATCH 1/2] AVRCP: return empty string instead of rejecting Luiz Augusto von Dentz
2011-10-03 8:54 ` Johan Hedberg
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.