---
.gitignore | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/.gitignore b/.gitignore
index 2ce99e7..c2fa9e9 100644
--- a/.gitignore
+++ b/.gitignore
@@ -80,6 +80,7 @@ test/ipctest
test/btiotest
test/test-textfile
test/uuidtest
+test/mpris-player
compat/dund
compat/hidd
compat/pand
--
1.7.7
Hi Luiz,
On Tue, Oct 11, 2011 at 6:47 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Lucas,
>
> On Mon, Oct 10, 2011 at 3:37 PM, Lucas De Marchi
> <[email protected]> wrote:
>> Since "138f831 AVRCP: return empty string instead of rejecting" we
>> return empty strings for all the attributes when there isn't any value
>> set.
>> ---
>> ?audio/avrcp.c | ? ?4 ----
>> ?1 files changed, 0 insertions(+), 4 deletions(-)
>>
>> diff --git a/audio/avrcp.c b/audio/avrcp.c
>> index c36af5d..058fa83 100644
>> --- a/audio/avrcp.c
>> +++ b/audio/avrcp.c
>> @@ -600,10 +600,6 @@ static uint8_t avrcp_handle_get_element_attributes(struct avrcp_player *player,
>> ? ? ? ?nattr = pdu->params[8];
>>
>> ? ? ? ?if (!nattr) {
>> - ? ? ? ? ? ? ? /*
>> - ? ? ? ? ? ? ? ?* Return all available information, at least
>> - ? ? ? ? ? ? ? ?* title must be returned.
>> - ? ? ? ? ? ? ? ?*/
>> ? ? ? ? ? ? ? ?for (i = 1; i < AVRCP_MEDIA_ATTRIBUTE_LAST; i++) {
>> ? ? ? ? ? ? ? ? ? ? ? ?size = player_get_media_attribute(player, i,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&pdu->params[pos],
>> --
>> 1.7.7
>
> Ack.
Following your suggestion for patch 7/7, this comment must remain
since we will need to treat title as a special case in
player_get_media_attribute().
Lucas De Marchi
If we use the same hash table to set the new metadata, we have 2
undesired behaviors:
1) New track may contain fields from previous track if it didn't set all
the fields
2) If we fail on parsing the signal, we will still change some of the
fields
---
v2: update documentation
audio/media.c | 43 ++++++++++++++++++++++++++-----------------
doc/media-api.txt | 6 ++++++
2 files changed, 32 insertions(+), 17 deletions(-)
diff --git a/audio/media.c b/audio/media.c
index 32dab86..13a0c78 100644
--- a/audio/media.c
+++ b/audio/media.c
@@ -1351,8 +1351,8 @@ static gboolean parse_player_metadata(struct media_player *mp,
{
DBusMessageIter dict;
DBusMessageIter var;
+ GHashTable *track;
int ctype;
- gboolean title = FALSE;
ctype = dbus_message_iter_get_arg_type(iter);
if (ctype != DBUS_TYPE_ARRAY)
@@ -1360,6 +1360,9 @@ static gboolean parse_player_metadata(struct media_player *mp,
dbus_message_iter_recurse(iter, &dict);
+ track = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL,
+ metadata_value_free);
+
while ((ctype = dbus_message_iter_get_arg_type(&dict)) !=
DBUS_TYPE_INVALID) {
DBusMessageIter entry;
@@ -1368,21 +1371,21 @@ static gboolean parse_player_metadata(struct media_player *mp,
int id;
if (ctype != DBUS_TYPE_DICT_ENTRY)
- return FALSE;
+ goto parse_error;
dbus_message_iter_recurse(&dict, &entry);
if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_STRING)
- return FALSE;
+ goto parse_error;
dbus_message_iter_get_basic(&entry, &key);
dbus_message_iter_next(&entry);
id = metadata_to_val(key);
if (id < 0)
- return FALSE;
+ goto parse_error;
if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_VARIANT)
- return FALSE;
+ goto parse_error;
dbus_message_iter_recurse(&entry, &var);
@@ -1391,13 +1394,12 @@ static gboolean parse_player_metadata(struct media_player *mp,
switch (id) {
case AVRCP_MEDIA_ATTRIBUTE_TITLE:
- title = TRUE;
case AVRCP_MEDIA_ATTRIBUTE_ARTIST:
case AVRCP_MEDIA_ATTRIBUTE_ALBUM:
case AVRCP_MEDIA_ATTRIBUTE_GENRE:
if (value->type != DBUS_TYPE_STRING) {
g_free(value);
- return FALSE;
+ goto parse_error;
}
dbus_message_iter_get_basic(&var, &value->value.str);
@@ -1407,13 +1409,13 @@ static gboolean parse_player_metadata(struct media_player *mp,
case AVRCP_MEDIA_ATTRIBUTE_DURATION:
if (value->type != DBUS_TYPE_UINT32) {
g_free(value);
- return FALSE;
+ goto parse_error;
}
dbus_message_iter_get_basic(&var, &value->value.num);
break;
default:
- return FALSE;
+ goto parse_error;
}
switch (value->type) {
@@ -1425,24 +1427,31 @@ static gboolean parse_player_metadata(struct media_player *mp,
DBG("%s=%u", key, value->value.num);
}
- if (!mp->track)
- mp->track = g_hash_table_new_full(g_direct_hash,
- g_direct_equal, NULL,
- metadata_value_free);
-
- g_hash_table_replace(mp->track, GUINT_TO_POINTER(id), value);
+ g_hash_table_replace(track, GUINT_TO_POINTER(id), value);
dbus_message_iter_next(&dict);
}
- if (title == FALSE)
- return TRUE;
+ if (g_hash_table_size(track) == 0) {
+ g_hash_table_unref(track);
+ track = NULL;
+ }
+ if (mp->track != NULL)
+ g_hash_table_unref(mp->track);
+
+ mp->track = track;
mp->position = 0;
g_timer_start(mp->timer);
avrcp_player_event(mp->player, AVRCP_EVENT_TRACK_CHANGED, NULL);
return TRUE;
+
+parse_error:
+ if (track)
+ g_hash_table_unref(track);
+
+ return FALSE;
}
static gboolean track_changed(DBusConnection *connection, DBusMessage *msg,
diff --git a/doc/media-api.txt b/doc/media-api.txt
index 7dc7661..b8dcdbd 100644
--- a/doc/media-api.txt
+++ b/doc/media-api.txt
@@ -144,6 +144,12 @@ Signals PropertyChanged(string setting, variant value)
TrackChanged(dict metadata)
+ This signal indicates that current track has changed.
+ All available metadata for the new track shall be set
+ at once in the metadata argument. Metadata cannot be
+ updated in parts, otherwise it will be interpreted as
+ multiple track changes.
+
Possible values:
string Title:
--
1.7.7
Hi Lucas,
On Mon, Oct 10, 2011 at 3:37 PM, Lucas De Marchi
<[email protected]> wrote:
> ---
> ?audio/avrcp.c | ? ?2 +-
> ?1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/audio/avrcp.c b/audio/avrcp.c
> index 0d9b6d0..3576008 100644
> --- a/audio/avrcp.c
> +++ b/audio/avrcp.c
> @@ -603,7 +603,7 @@ static uint8_t avrcp_handle_get_element_attributes(struct avrcp_player *player,
> ? ? ? ?nattr = pdu->params[8];
>
> ? ? ? ?if (!nattr) {
> - ? ? ? ? ? ? ? for (i = 1; i < AVRCP_MEDIA_ATTRIBUTE_LAST; i++) {
> + ? ? ? ? ? ? ? for (i = 1; i <= AVRCP_MEDIA_ATTRIBUTE_LAST; i++) {
> ? ? ? ? ? ? ? ? ? ? ? ?size = player_get_media_attribute(player, i,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&pdu->params[pos],
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?AVRCP_PDU_MTU - pos);
> --
> 1.7.7
What if we have a new callback to list the available metadata in the
media player and then iterate in that list, this way we don't have to
lookup for everything instead we use g_hash_table_get_keys and just
lookup for existing ones.
--
Luiz Augusto von Dentz
Hi Lucas,
On Mon, Oct 10, 2011 at 3:37 PM, Lucas De Marchi
<[email protected]> wrote:
> ---
> ?audio/avrcp.c | ? ?7 +++----
> ?1 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/audio/avrcp.c b/audio/avrcp.c
> index ea175e3..0d9b6d0 100644
> --- a/audio/avrcp.c
> +++ b/audio/avrcp.c
> @@ -565,11 +565,10 @@ static uint8_t avrcp_handle_list_player_values(struct avrcp_player *player,
> ? ? ? ?if (len != 1 || !player)
> ? ? ? ? ? ? ? ?goto err;
>
> - ? ? ? len = attr_get_max_val(pdu->params[0]);
> - ? ? ? if (!len) {
> - ? ? ? ? ? ? ? error("Attribute is invalid: %u", pdu->params[0]);
> + ? ? ? if (player_get_attribute(player, pdu->params[0]) < 0)
> ? ? ? ? ? ? ? ?goto err;
> - ? ? ? }
> +
> + ? ? ? len = attr_get_max_val(pdu->params[0]);
>
> ? ? ? ?for (i = 1; i <= len; i++)
> ? ? ? ? ? ? ? ?pdu->params[i] = i;
> --
> 1.7.7
Ack.
--
Luiz Augusto von Dentz
Hi Lucas,
On Mon, Oct 10, 2011 at 3:37 PM, Lucas De Marchi
<[email protected]> wrote:
> ---
> ?audio/avrcp.c | ? 19 ++++++++++---------
> ?1 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/audio/avrcp.c b/audio/avrcp.c
> index 058fa83..ea175e3 100644
> --- a/audio/avrcp.c
> +++ b/audio/avrcp.c
> @@ -474,9 +474,15 @@ static int player_set_attribute(struct avrcp_player *player,
>
> ?static int player_get_attribute(struct avrcp_player *player, uint8_t attr)
> ?{
> - ? ? ? DBG("Get attribute: %u", attr);
> + ? ? ? int value;
>
> - ? ? ? return player->cb->get_setting(attr, player->user_data);
> + ? ? ? DBG("attr %u", attr);
> +
> + ? ? ? value = player->cb->get_setting(attr, player->user_data);
> + ? ? ? if (value < 0)
> + ? ? ? ? ? ? ? DBG("attr %u not supported by player", attr);
> +
> + ? ? ? return value;
> ?}
>
> ?static uint8_t avrcp_handle_get_capabilities(struct avrcp_player *player,
> @@ -535,10 +541,8 @@ static uint8_t avrcp_handle_list_player_attributes(struct avrcp_player *player,
> ? ? ? ? ? ? ? ?goto done;
>
> ? ? ? ?for (i = 1; i <= AVRCP_ATTRIBUTE_SCAN; i++) {
> - ? ? ? ? ? ? ? if (player_get_attribute(player, i) < 0) {
> - ? ? ? ? ? ? ? ? ? ? ? DBG("Ignoring setting %u: not supported by player", i);
> + ? ? ? ? ? ? ? if (player_get_attribute(player, i) < 0)
> ? ? ? ? ? ? ? ? ? ? ? ?continue;
> - ? ? ? ? ? ? ? }
>
> ? ? ? ? ? ? ? ?len++;
> ? ? ? ? ? ? ? ?pdu->params[len] = i;
> @@ -677,11 +681,8 @@ static uint8_t avrcp_handle_get_current_player_value(struct avrcp_player *player
> ? ? ? ? ? ? ? ?}
>
> ? ? ? ? ? ? ? ?val = player_get_attribute(player, settings[i]);
> - ? ? ? ? ? ? ? if (val < 0) {
> - ? ? ? ? ? ? ? ? ? ? ? DBG("Ignoring %u: not supported by player",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? settings[i]);
> + ? ? ? ? ? ? ? if (val < 0)
> ? ? ? ? ? ? ? ? ? ? ? ?continue;
> - ? ? ? ? ? ? ? }
>
> ? ? ? ? ? ? ? ?pdu->params[++len] = settings[i];
> ? ? ? ? ? ? ? ?pdu->params[++len] = val;
> --
> 1.7.7
Ack.
--
Luiz Augusto von Dentz
Hi Lucas,
On Mon, Oct 10, 2011 at 3:37 PM, Lucas De Marchi
<[email protected]> wrote:
> If we use the same hash table to set the new metadata, we have 2
> undesired behaviors:
>
> 1) New track may contain fields from previous track if it didn't set all
> the fields
> 2) If we fail on parsing the signal, we will still change some of the
> fields
> ---
> ?audio/media.c | ? 43 ++++++++++++++++++++++++++-----------------
> ?1 files changed, 26 insertions(+), 17 deletions(-)
>
> diff --git a/audio/media.c b/audio/media.c
> index 32dab86..13a0c78 100644
> --- a/audio/media.c
> +++ b/audio/media.c
> @@ -1351,8 +1351,8 @@ static gboolean parse_player_metadata(struct media_player *mp,
> ?{
> ? ? ? ?DBusMessageIter dict;
> ? ? ? ?DBusMessageIter var;
> + ? ? ? GHashTable *track;
> ? ? ? ?int ctype;
> - ? ? ? gboolean title = FALSE;
>
> ? ? ? ?ctype = dbus_message_iter_get_arg_type(iter);
> ? ? ? ?if (ctype != DBUS_TYPE_ARRAY)
> @@ -1360,6 +1360,9 @@ static gboolean parse_player_metadata(struct media_player *mp,
>
> ? ? ? ?dbus_message_iter_recurse(iter, &dict);
>
> + ? ? ? track = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? metadata_value_free);
> +
> ? ? ? ?while ((ctype = dbus_message_iter_get_arg_type(&dict)) !=
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID) {
> ? ? ? ? ? ? ? ?DBusMessageIter entry;
> @@ -1368,21 +1371,21 @@ static gboolean parse_player_metadata(struct media_player *mp,
> ? ? ? ? ? ? ? ?int id;
>
> ? ? ? ? ? ? ? ?if (ctype != DBUS_TYPE_DICT_ENTRY)
> - ? ? ? ? ? ? ? ? ? ? ? return FALSE;
> + ? ? ? ? ? ? ? ? ? ? ? goto parse_error;
>
> ? ? ? ? ? ? ? ?dbus_message_iter_recurse(&dict, &entry);
> ? ? ? ? ? ? ? ?if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_STRING)
> - ? ? ? ? ? ? ? ? ? ? ? return FALSE;
> + ? ? ? ? ? ? ? ? ? ? ? goto parse_error;
>
> ? ? ? ? ? ? ? ?dbus_message_iter_get_basic(&entry, &key);
> ? ? ? ? ? ? ? ?dbus_message_iter_next(&entry);
>
> ? ? ? ? ? ? ? ?id = metadata_to_val(key);
> ? ? ? ? ? ? ? ?if (id < 0)
> - ? ? ? ? ? ? ? ? ? ? ? return FALSE;
> + ? ? ? ? ? ? ? ? ? ? ? goto parse_error;
>
> ? ? ? ? ? ? ? ?if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_VARIANT)
> - ? ? ? ? ? ? ? ? ? ? ? return FALSE;
> + ? ? ? ? ? ? ? ? ? ? ? goto parse_error;
>
> ? ? ? ? ? ? ? ?dbus_message_iter_recurse(&entry, &var);
>
> @@ -1391,13 +1394,12 @@ static gboolean parse_player_metadata(struct media_player *mp,
>
> ? ? ? ? ? ? ? ?switch (id) {
> ? ? ? ? ? ? ? ?case AVRCP_MEDIA_ATTRIBUTE_TITLE:
> - ? ? ? ? ? ? ? ? ? ? ? title = TRUE;
> ? ? ? ? ? ? ? ?case AVRCP_MEDIA_ATTRIBUTE_ARTIST:
> ? ? ? ? ? ? ? ?case AVRCP_MEDIA_ATTRIBUTE_ALBUM:
> ? ? ? ? ? ? ? ?case AVRCP_MEDIA_ATTRIBUTE_GENRE:
> ? ? ? ? ? ? ? ? ? ? ? ?if (value->type != DBUS_TYPE_STRING) {
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?g_free(value);
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? return FALSE;
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? goto parse_error;
> ? ? ? ? ? ? ? ? ? ? ? ?}
>
> ? ? ? ? ? ? ? ? ? ? ? ?dbus_message_iter_get_basic(&var, &value->value.str);
> @@ -1407,13 +1409,13 @@ static gboolean parse_player_metadata(struct media_player *mp,
> ? ? ? ? ? ? ? ?case AVRCP_MEDIA_ATTRIBUTE_DURATION:
> ? ? ? ? ? ? ? ? ? ? ? ?if (value->type != DBUS_TYPE_UINT32) {
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?g_free(value);
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? return FALSE;
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? goto parse_error;
> ? ? ? ? ? ? ? ? ? ? ? ?}
>
> ? ? ? ? ? ? ? ? ? ? ? ?dbus_message_iter_get_basic(&var, &value->value.num);
> ? ? ? ? ? ? ? ? ? ? ? ?break;
> ? ? ? ? ? ? ? ?default:
> - ? ? ? ? ? ? ? ? ? ? ? return FALSE;
> + ? ? ? ? ? ? ? ? ? ? ? goto parse_error;
> ? ? ? ? ? ? ? ?}
>
> ? ? ? ? ? ? ? ?switch (value->type) {
> @@ -1425,24 +1427,31 @@ static gboolean parse_player_metadata(struct media_player *mp,
> ? ? ? ? ? ? ? ? ? ? ? ?DBG("%s=%u", key, value->value.num);
> ? ? ? ? ? ? ? ?}
>
> - ? ? ? ? ? ? ? if (!mp->track)
> - ? ? ? ? ? ? ? ? ? ? ? mp->track = g_hash_table_new_full(g_direct_hash,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? g_direct_equal, NULL,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? metadata_value_free);
> -
> - ? ? ? ? ? ? ? g_hash_table_replace(mp->track, GUINT_TO_POINTER(id), value);
> + ? ? ? ? ? ? ? g_hash_table_replace(track, GUINT_TO_POINTER(id), value);
> ? ? ? ? ? ? ? ?dbus_message_iter_next(&dict);
> ? ? ? ?}
>
> - ? ? ? if (title == FALSE)
> - ? ? ? ? ? ? ? return TRUE;
> + ? ? ? if (g_hash_table_size(track) == 0) {
> + ? ? ? ? ? ? ? g_hash_table_unref(track);
> + ? ? ? ? ? ? ? track = NULL;
> + ? ? ? }
>
> + ? ? ? if (mp->track != NULL)
> + ? ? ? ? ? ? ? g_hash_table_unref(mp->track);
> +
> + ? ? ? mp->track = track;
> ? ? ? ?mp->position = 0;
> ? ? ? ?g_timer_start(mp->timer);
>
> ? ? ? ?avrcp_player_event(mp->player, AVRCP_EVENT_TRACK_CHANGED, NULL);
>
> ? ? ? ?return TRUE;
> +
> +parse_error:
> + ? ? ? if (track)
> + ? ? ? ? ? ? ? g_hash_table_unref(track);
> +
> + ? ? ? return FALSE;
> ?}
>
> ?static gboolean track_changed(DBusConnection *connection, DBusMessage *msg,
> --
> 1.7.7
>
It might be useful to add to the documentation that the metadata need
to be set all at once so it cannot be updated in parts.
--
Luiz Augusto von Dentz
Hi Lucas,
On Mon, Oct 10, 2011 at 3:37 PM, Lucas De Marchi
<[email protected]> wrote:
> Since "138f831 AVRCP: return empty string instead of rejecting" we
> return empty strings for all the attributes when there isn't any value
> set.
> ---
> ?audio/avrcp.c | ? ?4 ----
> ?1 files changed, 0 insertions(+), 4 deletions(-)
>
> diff --git a/audio/avrcp.c b/audio/avrcp.c
> index c36af5d..058fa83 100644
> --- a/audio/avrcp.c
> +++ b/audio/avrcp.c
> @@ -600,10 +600,6 @@ static uint8_t avrcp_handle_get_element_attributes(struct avrcp_player *player,
> ? ? ? ?nattr = pdu->params[8];
>
> ? ? ? ?if (!nattr) {
> - ? ? ? ? ? ? ? /*
> - ? ? ? ? ? ? ? ?* Return all available information, at least
> - ? ? ? ? ? ? ? ?* title must be returned.
> - ? ? ? ? ? ? ? ?*/
> ? ? ? ? ? ? ? ?for (i = 1; i < AVRCP_MEDIA_ATTRIBUTE_LAST; i++) {
> ? ? ? ? ? ? ? ? ? ? ? ?size = player_get_media_attribute(player, i,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&pdu->params[pos],
> --
> 1.7.7
Ack.
--
Luiz Augusto von Dentz
Hi Lucas,
On Mon, Oct 10, 2011 at 3:37 PM, Lucas De Marchi
<[email protected]> wrote:
> ---
> ?doc/media-api.txt | ? ?2 +-
> ?1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/doc/media-api.txt b/doc/media-api.txt
> index af4cfa0..7dc7661 100644
> --- a/doc/media-api.txt
> +++ b/doc/media-api.txt
> @@ -133,7 +133,7 @@ Object path freely definable
> ?Methods ? ? ? ? ? ? ? ?void SetProperty(string property, variant value)
>
> ? ? ? ? ? ? ? ? ? ? ? ?Changes the value of the specified property. Only
> - ? ? ? ? ? ? ? ? ? ? ? properties that are listed a read-write can be changed.
> + ? ? ? ? ? ? ? ? ? ? ? properties that are listed as read-write can be changed.
>
> ? ? ? ? ? ? ? ? ? ? ? ?On success this will emit a PropertyChanged signal.
>
> --
> 1.7.7
>
Ack.
--
Luiz Augusto von Dentz
Hi Lucas,
On Mon, Oct 10, 2011 at 3:37 PM, Lucas De Marchi
<[email protected]> wrote:
> ---
> ?.gitignore | ? ?1 +
> ?1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/.gitignore b/.gitignore
> index 2ce99e7..c2fa9e9 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -80,6 +80,7 @@ test/ipctest
> ?test/btiotest
> ?test/test-textfile
> ?test/uuidtest
> +test/mpris-player
> ?compat/dund
> ?compat/hidd
> ?compat/pand
> --
> 1.7.7
Ack.
--
Luiz Augusto von Dentz
---
audio/avrcp.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/audio/avrcp.c b/audio/avrcp.c
index 0d9b6d0..3576008 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -603,7 +603,7 @@ static uint8_t avrcp_handle_get_element_attributes(struct avrcp_player *player,
nattr = pdu->params[8];
if (!nattr) {
- for (i = 1; i < AVRCP_MEDIA_ATTRIBUTE_LAST; i++) {
+ for (i = 1; i <= AVRCP_MEDIA_ATTRIBUTE_LAST; i++) {
size = player_get_media_attribute(player, i,
&pdu->params[pos],
AVRCP_PDU_MTU - pos);
--
1.7.7
---
audio/avrcp.c | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/audio/avrcp.c b/audio/avrcp.c
index ea175e3..0d9b6d0 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -565,11 +565,10 @@ static uint8_t avrcp_handle_list_player_values(struct avrcp_player *player,
if (len != 1 || !player)
goto err;
- len = attr_get_max_val(pdu->params[0]);
- if (!len) {
- error("Attribute is invalid: %u", pdu->params[0]);
+ if (player_get_attribute(player, pdu->params[0]) < 0)
goto err;
- }
+
+ len = attr_get_max_val(pdu->params[0]);
for (i = 1; i <= len; i++)
pdu->params[i] = i;
--
1.7.7
---
audio/avrcp.c | 19 ++++++++++---------
1 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/audio/avrcp.c b/audio/avrcp.c
index 058fa83..ea175e3 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -474,9 +474,15 @@ static int player_set_attribute(struct avrcp_player *player,
static int player_get_attribute(struct avrcp_player *player, uint8_t attr)
{
- DBG("Get attribute: %u", attr);
+ int value;
- return player->cb->get_setting(attr, player->user_data);
+ DBG("attr %u", attr);
+
+ value = player->cb->get_setting(attr, player->user_data);
+ if (value < 0)
+ DBG("attr %u not supported by player", attr);
+
+ return value;
}
static uint8_t avrcp_handle_get_capabilities(struct avrcp_player *player,
@@ -535,10 +541,8 @@ static uint8_t avrcp_handle_list_player_attributes(struct avrcp_player *player,
goto done;
for (i = 1; i <= AVRCP_ATTRIBUTE_SCAN; i++) {
- if (player_get_attribute(player, i) < 0) {
- DBG("Ignoring setting %u: not supported by player", i);
+ if (player_get_attribute(player, i) < 0)
continue;
- }
len++;
pdu->params[len] = i;
@@ -677,11 +681,8 @@ static uint8_t avrcp_handle_get_current_player_value(struct avrcp_player *player
}
val = player_get_attribute(player, settings[i]);
- if (val < 0) {
- DBG("Ignoring %u: not supported by player",
- settings[i]);
+ if (val < 0)
continue;
- }
pdu->params[++len] = settings[i];
pdu->params[++len] = val;
--
1.7.7
If we use the same hash table to set the new metadata, we have 2
undesired behaviors:
1) New track may contain fields from previous track if it didn't set all
the fields
2) If we fail on parsing the signal, we will still change some of the
fields
---
audio/media.c | 43 ++++++++++++++++++++++++++-----------------
1 files changed, 26 insertions(+), 17 deletions(-)
diff --git a/audio/media.c b/audio/media.c
index 32dab86..13a0c78 100644
--- a/audio/media.c
+++ b/audio/media.c
@@ -1351,8 +1351,8 @@ static gboolean parse_player_metadata(struct media_player *mp,
{
DBusMessageIter dict;
DBusMessageIter var;
+ GHashTable *track;
int ctype;
- gboolean title = FALSE;
ctype = dbus_message_iter_get_arg_type(iter);
if (ctype != DBUS_TYPE_ARRAY)
@@ -1360,6 +1360,9 @@ static gboolean parse_player_metadata(struct media_player *mp,
dbus_message_iter_recurse(iter, &dict);
+ track = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL,
+ metadata_value_free);
+
while ((ctype = dbus_message_iter_get_arg_type(&dict)) !=
DBUS_TYPE_INVALID) {
DBusMessageIter entry;
@@ -1368,21 +1371,21 @@ static gboolean parse_player_metadata(struct media_player *mp,
int id;
if (ctype != DBUS_TYPE_DICT_ENTRY)
- return FALSE;
+ goto parse_error;
dbus_message_iter_recurse(&dict, &entry);
if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_STRING)
- return FALSE;
+ goto parse_error;
dbus_message_iter_get_basic(&entry, &key);
dbus_message_iter_next(&entry);
id = metadata_to_val(key);
if (id < 0)
- return FALSE;
+ goto parse_error;
if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_VARIANT)
- return FALSE;
+ goto parse_error;
dbus_message_iter_recurse(&entry, &var);
@@ -1391,13 +1394,12 @@ static gboolean parse_player_metadata(struct media_player *mp,
switch (id) {
case AVRCP_MEDIA_ATTRIBUTE_TITLE:
- title = TRUE;
case AVRCP_MEDIA_ATTRIBUTE_ARTIST:
case AVRCP_MEDIA_ATTRIBUTE_ALBUM:
case AVRCP_MEDIA_ATTRIBUTE_GENRE:
if (value->type != DBUS_TYPE_STRING) {
g_free(value);
- return FALSE;
+ goto parse_error;
}
dbus_message_iter_get_basic(&var, &value->value.str);
@@ -1407,13 +1409,13 @@ static gboolean parse_player_metadata(struct media_player *mp,
case AVRCP_MEDIA_ATTRIBUTE_DURATION:
if (value->type != DBUS_TYPE_UINT32) {
g_free(value);
- return FALSE;
+ goto parse_error;
}
dbus_message_iter_get_basic(&var, &value->value.num);
break;
default:
- return FALSE;
+ goto parse_error;
}
switch (value->type) {
@@ -1425,24 +1427,31 @@ static gboolean parse_player_metadata(struct media_player *mp,
DBG("%s=%u", key, value->value.num);
}
- if (!mp->track)
- mp->track = g_hash_table_new_full(g_direct_hash,
- g_direct_equal, NULL,
- metadata_value_free);
-
- g_hash_table_replace(mp->track, GUINT_TO_POINTER(id), value);
+ g_hash_table_replace(track, GUINT_TO_POINTER(id), value);
dbus_message_iter_next(&dict);
}
- if (title == FALSE)
- return TRUE;
+ if (g_hash_table_size(track) == 0) {
+ g_hash_table_unref(track);
+ track = NULL;
+ }
+ if (mp->track != NULL)
+ g_hash_table_unref(mp->track);
+
+ mp->track = track;
mp->position = 0;
g_timer_start(mp->timer);
avrcp_player_event(mp->player, AVRCP_EVENT_TRACK_CHANGED, NULL);
return TRUE;
+
+parse_error:
+ if (track)
+ g_hash_table_unref(track);
+
+ return FALSE;
}
static gboolean track_changed(DBusConnection *connection, DBusMessage *msg,
--
1.7.7
Since "138f831 AVRCP: return empty string instead of rejecting" we
return empty strings for all the attributes when there isn't any value
set.
---
audio/avrcp.c | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/audio/avrcp.c b/audio/avrcp.c
index c36af5d..058fa83 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -600,10 +600,6 @@ static uint8_t avrcp_handle_get_element_attributes(struct avrcp_player *player,
nattr = pdu->params[8];
if (!nattr) {
- /*
- * Return all available information, at least
- * title must be returned.
- */
for (i = 1; i < AVRCP_MEDIA_ATTRIBUTE_LAST; i++) {
size = player_get_media_attribute(player, i,
&pdu->params[pos],
--
1.7.7
---
doc/media-api.txt | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/doc/media-api.txt b/doc/media-api.txt
index af4cfa0..7dc7661 100644
--- a/doc/media-api.txt
+++ b/doc/media-api.txt
@@ -133,7 +133,7 @@ Object path freely definable
Methods void SetProperty(string property, variant value)
Changes the value of the specified property. Only
- properties that are listed a read-write can be changed.
+ properties that are listed as read-write can be changed.
On success this will emit a PropertyChanged signal.
--
1.7.7