* [PATCH 01/12] Add mpris-player to .gitignore
@ 2011-10-12 15:11 Lucas De Marchi
2011-10-12 15:11 ` [PATCH 02/12] Fix typo on doc Lucas De Marchi
` (10 more replies)
0 siblings, 11 replies; 15+ messages in thread
From: Lucas De Marchi @ 2011-10-12 15:11 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Lucas De Marchi
---
.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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 02/12] Fix typo on doc
2011-10-12 15:11 [PATCH 01/12] Add mpris-player to .gitignore Lucas De Marchi
@ 2011-10-12 15:11 ` Lucas De Marchi
2011-10-12 15:11 ` [PATCH 03/12] Don't overwrite metadata when changing track Lucas De Marchi
` (9 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Lucas De Marchi @ 2011-10-12 15:11 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Lucas De Marchi
---
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 03/12] Don't overwrite metadata when changing track
2011-10-12 15:11 [PATCH 01/12] Add mpris-player to .gitignore Lucas De Marchi
2011-10-12 15:11 ` [PATCH 02/12] Fix typo on doc Lucas De Marchi
@ 2011-10-12 15:11 ` Lucas De Marchi
2011-10-13 7:42 ` Johan Hedberg
2011-10-12 15:11 ` [PATCH 04/12] Make title always available in metadata Lucas De Marchi
` (8 subsequent siblings)
10 siblings, 1 reply; 15+ messages in thread
From: Lucas De Marchi @ 2011-10-12 15:11 UTC (permalink / raw)
To: linux-bluetooth; +Cc: 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
---
audio/media.c | 41 ++++++++++++++++++++++++++---------------
doc/media-api.txt | 6 ++++++
2 files changed, 32 insertions(+), 15 deletions(-)
diff --git a/audio/media.c b/audio/media.c
index ef595b3..c9fe0f0 100644
--- a/audio/media.c
+++ b/audio/media.c
@@ -1353,6 +1353,7 @@ static gboolean parse_player_metadata(struct media_player *mp,
{
DBusMessageIter dict;
DBusMessageIter var;
+ GHashTable *track;
int ctype;
gboolean title = FALSE;
@@ -1362,6 +1363,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;
@@ -1370,21 +1374,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);
@@ -1399,7 +1403,7 @@ static gboolean parse_player_metadata(struct media_player *mp,
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);
@@ -1409,13 +1413,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) {
@@ -1427,24 +1431,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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 04/12] Make title always available in metadata
2011-10-12 15:11 [PATCH 01/12] Add mpris-player to .gitignore Lucas De Marchi
2011-10-12 15:11 ` [PATCH 02/12] Fix typo on doc Lucas De Marchi
2011-10-12 15:11 ` [PATCH 03/12] Don't overwrite metadata when changing track Lucas De Marchi
@ 2011-10-12 15:11 ` Lucas De Marchi
2011-10-13 7:44 ` Johan Hedberg
2011-10-12 15:11 ` [PATCH 05/12] Move debug messages to their correspondent getters Lucas De Marchi
` (7 subsequent siblings)
10 siblings, 1 reply; 15+ messages in thread
From: Lucas De Marchi @ 2011-10-12 15:11 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Lucas De Marchi
---
audio/media.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/audio/media.c b/audio/media.c
index c9fe0f0..56913d5 100644
--- a/audio/media.c
+++ b/audio/media.c
@@ -1438,6 +1438,11 @@ static gboolean parse_player_metadata(struct media_player *mp,
if (g_hash_table_size(track) == 0) {
g_hash_table_unref(track);
track = NULL;
+ } else if (title == FALSE) {
+ struct metadata_value *value = g_new(struct metadata_value, 1);
+
+ value->type = DBUS_TYPE_STRING;
+ value->value.str = g_strdup("");
}
if (mp->track != NULL)
--
1.7.7
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 05/12] Move debug messages to their correspondent getters
2011-10-12 15:11 [PATCH 01/12] Add mpris-player to .gitignore Lucas De Marchi
` (2 preceding siblings ...)
2011-10-12 15:11 ` [PATCH 04/12] Make title always available in metadata Lucas De Marchi
@ 2011-10-12 15:11 ` Lucas De Marchi
2011-10-12 15:11 ` [PATCH 06/12] AVRCP: Do not list values for unsupported attributes Lucas De Marchi
` (6 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Lucas De Marchi @ 2011-10-12 15:11 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Lucas De Marchi
---
audio/avrcp.c | 19 ++++++++++---------
1 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/audio/avrcp.c b/audio/avrcp.c
index c36af5d..6bddf5b 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;
@@ -681,11 +685,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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 06/12] AVRCP: Do not list values for unsupported attributes
2011-10-12 15:11 [PATCH 01/12] Add mpris-player to .gitignore Lucas De Marchi
` (3 preceding siblings ...)
2011-10-12 15:11 ` [PATCH 05/12] Move debug messages to their correspondent getters Lucas De Marchi
@ 2011-10-12 15:11 ` Lucas De Marchi
2011-10-12 15:11 ` [PATCH 07/12] Add callback to list available metadata IDs Lucas De Marchi
` (5 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Lucas De Marchi @ 2011-10-12 15:11 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Lucas De Marchi
---
audio/avrcp.c | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/audio/avrcp.c b/audio/avrcp.c
index 6bddf5b..a7987fb 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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 07/12] Add callback to list available metadata IDs
2011-10-12 15:11 [PATCH 01/12] Add mpris-player to .gitignore Lucas De Marchi
` (4 preceding siblings ...)
2011-10-12 15:11 ` [PATCH 06/12] AVRCP: Do not list values for unsupported attributes Lucas De Marchi
@ 2011-10-12 15:11 ` Lucas De Marchi
2011-10-12 15:11 ` [PATCH 08/12] AVRCP: Return error for invalid " Lucas De Marchi
` (4 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Lucas De Marchi @ 2011-10-12 15:11 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Lucas De Marchi
---
audio/avrcp.h | 1 +
audio/media.c | 13 +++++++++++++
2 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/audio/avrcp.h b/audio/avrcp.h
index 66d09bc..360a80a 100644
--- a/audio/avrcp.h
+++ b/audio/avrcp.h
@@ -76,6 +76,7 @@ struct avrcp_player_cb {
int (*get_setting) (uint8_t attr, void *user_data);
int (*set_setting) (uint8_t attr, uint8_t value, void *user_data);
void *(*get_metadata) (uint32_t id, void *user_data);
+ GList *(*list_metadata) (void *user_data);
uint8_t (*get_status) (void *user_data);
uint32_t (*get_position) (void *user_data);
};
diff --git a/audio/media.c b/audio/media.c
index 56913d5..5a74fec 100644
--- a/audio/media.c
+++ b/audio/media.c
@@ -1156,6 +1156,18 @@ static int set_setting(uint8_t attr, uint8_t val, void *user_data)
return 0;
}
+static GList *list_metadata(void *user_data)
+{
+ struct media_player *mp = user_data;
+
+ DBG("");
+
+ if (mp->track == NULL)
+ return NULL;
+
+ return g_hash_table_get_keys(mp->track);
+}
+
static void *get_metadata(uint32_t id, void *user_data)
{
struct media_player *mp = user_data;
@@ -1207,6 +1219,7 @@ static uint32_t get_position(void *user_data)
static struct avrcp_player_cb player_cb = {
.get_setting = get_setting,
.set_setting = set_setting,
+ .list_metadata = list_metadata,
.get_metadata = get_metadata,
.get_position = get_position,
.get_status = get_status
--
1.7.7
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 08/12] AVRCP: Return error for invalid metadata IDs
2011-10-12 15:11 [PATCH 01/12] Add mpris-player to .gitignore Lucas De Marchi
` (5 preceding siblings ...)
2011-10-12 15:11 ` [PATCH 07/12] Add callback to list available metadata IDs Lucas De Marchi
@ 2011-10-12 15:11 ` Lucas De Marchi
2011-10-12 15:11 ` [PATCH 09/12] AVRCP: Check if len matches number of IDs Lucas De Marchi
` (3 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Lucas De Marchi @ 2011-10-12 15:11 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Lucas De Marchi
---
audio/avrcp.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/audio/avrcp.c b/audio/avrcp.c
index a7987fb..0ca91a5 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -427,6 +427,10 @@ static int player_get_media_attribute(struct avrcp_player *player,
DBG("Get media attribute: %u", id);
+ if (id == AVRCP_MEDIA_ATTRIBUTE_ILLEGAL ||
+ id > AVRCP_MEDIA_ATTRIBUTE_LAST)
+ return -ENOENT;
+
value = player->cb->get_metadata(id, player->user_data);
if (value == NULL) {
len = 0;
@@ -452,8 +456,6 @@ static int player_get_media_attribute(struct avrcp_player *player,
return -ENOBUFS;
memcpy(elem->val, valstr, len);
break;
- default:
- return -ENOENT;
}
done:
--
1.7.7
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 09/12] AVRCP: Check if len matches number of IDs
2011-10-12 15:11 [PATCH 01/12] Add mpris-player to .gitignore Lucas De Marchi
` (6 preceding siblings ...)
2011-10-12 15:11 ` [PATCH 08/12] AVRCP: Return error for invalid " Lucas De Marchi
@ 2011-10-12 15:11 ` Lucas De Marchi
2011-10-12 15:11 ` [PATCH 10/12] AVRCP: Only return available metadata attributes Lucas De Marchi
` (2 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Lucas De Marchi @ 2011-10-12 15:11 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Lucas De Marchi
If number of attributes remote side provided is larger than the length
we read, we would read garbage from stack memory.
---
audio/avrcp.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/audio/avrcp.c b/audio/avrcp.c
index 0ca91a5..8b46650 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -597,12 +597,16 @@ static uint8_t avrcp_handle_get_element_attributes(struct avrcp_player *player,
int size;
unsigned int i;
- if (len < 8 || *identifier != 0)
+ if (len < 9 || *identifier != 0)
+ goto err;
+
+ nattr = pdu->params[8];
+
+ if (len < nattr * sizeof(uint32_t) + 1)
goto err;
len = 0;
pos = 1; /* Keep track of current position in reponse */
- nattr = pdu->params[8];
if (!nattr) {
/*
--
1.7.7
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 10/12] AVRCP: Only return available metadata attributes
2011-10-12 15:11 [PATCH 01/12] Add mpris-player to .gitignore Lucas De Marchi
` (7 preceding siblings ...)
2011-10-12 15:11 ` [PATCH 09/12] AVRCP: Check if len matches number of IDs Lucas De Marchi
@ 2011-10-12 15:11 ` Lucas De Marchi
2011-10-12 15:11 ` [PATCH 11/12] Refactor to share code Lucas De Marchi
2011-10-12 15:11 ` [PATCH 12/12] Improve test/simple-player to allow user interaction Lucas De Marchi
10 siblings, 0 replies; 15+ messages in thread
From: Lucas De Marchi @ 2011-10-12 15:11 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Lucas De Marchi
When remote side requests all available metadata (i.e. it gives number
attributes equals to 0) do not return zero-length strings for
unavailable items. The only exception is title, that must be always present.
---
audio/avrcp.c | 12 +++++++++---
1 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/audio/avrcp.c b/audio/avrcp.c
index 8b46650..4e3ef97 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -611,10 +611,14 @@ static uint8_t avrcp_handle_get_element_attributes(struct avrcp_player *player,
if (!nattr) {
/*
* Return all available information, at least
- * title must be returned.
+ * title must be returned if there's a track selected.
*/
- for (i = 1; i < AVRCP_MEDIA_ATTRIBUTE_LAST; i++) {
- size = player_get_media_attribute(player, i,
+ GList *l, *attr_ids = player->cb->list_metadata(player->user_data);
+
+ for (l = attr_ids; l != NULL; l = l->next) {
+ uint32_t attr = GPOINTER_TO_UINT(l->data);
+
+ size = player_get_media_attribute(player, attr,
&pdu->params[pos],
AVRCP_PDU_MTU - pos);
@@ -623,6 +627,8 @@ static uint8_t avrcp_handle_get_element_attributes(struct avrcp_player *player,
pos += size;
}
}
+
+ g_list_free(attr_ids);
} else {
uint32_t *attr_ids;
--
1.7.7
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 11/12] Refactor to share code
2011-10-12 15:11 [PATCH 01/12] Add mpris-player to .gitignore Lucas De Marchi
` (8 preceding siblings ...)
2011-10-12 15:11 ` [PATCH 10/12] AVRCP: Only return available metadata attributes Lucas De Marchi
@ 2011-10-12 15:11 ` Lucas De Marchi
2011-10-12 15:11 ` [PATCH 12/12] Improve test/simple-player to allow user interaction Lucas De Marchi
10 siblings, 0 replies; 15+ messages in thread
From: Lucas De Marchi @ 2011-10-12 15:11 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Lucas De Marchi
Refactor code when we are 'listing selected attributes' to share code
with 'listing all attributes'. This way we always keep the list of
attributes in a GList, and call player_get_media_attribute() in only one
place.
---
audio/avrcp.c | 55 ++++++++++++++++++++++---------------------------------
1 files changed, 22 insertions(+), 33 deletions(-)
diff --git a/audio/avrcp.c b/audio/avrcp.c
index 4e3ef97..ac7c8d4 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -591,11 +591,11 @@ static uint8_t avrcp_handle_get_element_attributes(struct avrcp_player *player,
uint8_t transaction)
{
uint16_t len = ntohs(pdu->params_len);
- uint64_t *identifier = (void *) &pdu->params[0];
+ uint64_t *identifier = (uint64_t *) &pdu->params[0];
uint16_t pos;
uint8_t nattr;
int size;
- unsigned int i;
+ GList *l, *attr_ids;
if (len < 9 || *identifier != 0)
goto err;
@@ -605,53 +605,42 @@ static uint8_t avrcp_handle_get_element_attributes(struct avrcp_player *player,
if (len < nattr * sizeof(uint32_t) + 1)
goto err;
- len = 0;
- pos = 1; /* Keep track of current position in reponse */
-
if (!nattr) {
/*
* Return all available information, at least
* title must be returned if there's a track selected.
*/
- GList *l, *attr_ids = player->cb->list_metadata(player->user_data);
-
- for (l = attr_ids; l != NULL; l = l->next) {
- uint32_t attr = GPOINTER_TO_UINT(l->data);
-
- size = player_get_media_attribute(player, attr,
- &pdu->params[pos],
- AVRCP_PDU_MTU - pos);
+ attr_ids = player->cb->list_metadata(player->user_data);
+ } else {
+ unsigned int i;
+ uint32_t *attr = (uint32_t *) &pdu->params[9];
- if (size > 0) {
- len++;
- pos += size;
- }
+ for (i = 0, attr_ids = NULL; i < nattr; i++, attr++) {
+ uint32_t id = ntohl(bt_get_unaligned(attr));
+ attr_ids = g_list_prepend(attr_ids,
+ GUINT_TO_POINTER(id));
}
- g_list_free(attr_ids);
- } else {
- uint32_t *attr_ids;
-
- attr_ids = g_memdup(&pdu->params[9], sizeof(uint32_t) * nattr);
+ attr_ids = g_list_reverse(attr_ids);
+ }
- for (i = 0; i < nattr; i++) {
- uint32_t attr = ntohl(attr_ids[i]);
+ for (l = attr_ids, len = 0, pos = 1; l != NULL; l = l->next) {
+ uint32_t attr = GPOINTER_TO_UINT(l->data);
- size = player_get_media_attribute(player, attr,
+ size = player_get_media_attribute(player, attr,
&pdu->params[pos],
AVRCP_PDU_MTU - pos);
- if (size > 0) {
- len++;
- pos += size;
- }
+ if (size >= 0) {
+ len++;
+ pos += size;
}
+ }
- g_free(attr_ids);
+ g_list_free(attr_ids);
- if (!len)
- goto err;
- }
+ if (!len)
+ goto err;
pdu->params[0] = len;
pdu->params_len = htons(pos);
--
1.7.7
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 12/12] Improve test/simple-player to allow user interaction
2011-10-12 15:11 [PATCH 01/12] Add mpris-player to .gitignore Lucas De Marchi
` (9 preceding siblings ...)
2011-10-12 15:11 ` [PATCH 11/12] Refactor to share code Lucas De Marchi
@ 2011-10-12 15:11 ` Lucas De Marchi
2011-10-13 10:12 ` Johan Hedberg
10 siblings, 1 reply; 15+ messages in thread
From: Lucas De Marchi @ 2011-10-12 15:11 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Lucas De Marchi
Allow user to trigger TrackChanged and PropertyChanged. Calls made by
remote side (coming from D-Bus) are printed to stderr while the ones to
interact with user go to stdout.
---
test/simple-player | 71 +++++++++++++++++++++++++++++++++++++++++++++++++---
1 files changed, 67 insertions(+), 4 deletions(-)
diff --git a/test/simple-player b/test/simple-player
index f483897..209fcdc 100755
--- a/test/simple-player
+++ b/test/simple-player
@@ -1,5 +1,7 @@
#!/usr/bin/python
+from __future__ import print_function
+import os
import sys
import dbus
import dbus.service
@@ -10,9 +12,68 @@ class Player(dbus.service.Object):
@dbus.service.method("org.bluez.MediaPlayer",
in_signature="sv", out_signature="")
def SetProperty(self, key, value):
- print "SetProperty (%s, %s)" % (key, value)
+ print("SetProperty (%s, %s)" % (key, value), file=sys.stderr)
return
+ @dbus.service.signal("org.bluez.MediaPlayer", signature="sv")
+ def PropertyChanged(self, setting, value):
+ """PropertyChanged(setting, value)
+
+ Send a PropertyChanged signal. 'setting' and 'value' are
+ string parameters as specified in doc/media-api.txt.
+ """
+ pass
+
+ @dbus.service.signal("org.bluez.MediaPlayer", signature="a{sv}")
+ def TrackChanged(self, metadata):
+ """TrackChanged(metadata)
+
+ Send a TrackChanged signal. 'metadata' parameter is a dictionary,
+ with values as defined in doc/media-api.txt.
+ """
+ pass
+
+ def help(self, func):
+ help(self.__class__.__dict__[func])
+
+class InputHandler:
+ commands = { 'TrackChanged': '(metadata)',
+ 'PropertyChanged': '(key, value)',
+ 'help': '(cmd)' }
+ def __init__(self, player):
+ self.player = player
+ print('\n\nAvailable commands:')
+ for cmd in self.commands:
+ print('\t', cmd, self.commands[cmd], sep='')
+
+ print("\nUse python syntax to pass arguments to available methods.\n" \
+ "E.g.: TrackChanged({'Title': 'My title', 'Album': 'my album' })")
+ self.prompt()
+
+ def prompt(self):
+ print('\n>>> ', end='')
+ sys.stdout.flush()
+
+ def handle(self, fd, condition):
+ s = os.read(fd.fileno(), 1024).strip()
+ try:
+ cmd = s[:s.find('(')]
+ if not cmd in self.commands:
+ print("Unknown command ", cmd)
+ except ValueError:
+ print("Malformed command")
+ return True
+
+ try:
+ exec "self.player.%s" % s
+ except Exception as e:
+ print(e)
+ pass
+ self.prompt()
+ return True
+
+
+
if __name__ == '__main__':
dbus.mainloop.glib.DBusGMainLoop(set_as_default=True)
@@ -39,8 +100,6 @@ if __name__ == '__main__':
"Status" : "playing",
"Position" : dbus.UInt32(0) })
- print properties
-
metadata = dbus.Dictionary({ "Title" : "Title",
"Artist" : "Artist",
"Album" : "Album",
@@ -49,7 +108,11 @@ if __name__ == '__main__':
"Number" : dbus.UInt32(1),
"Duration" : dbus.UInt32(10000) })
- print metadata
+ print('Register media player with:\n\tProperties: %s\n\tMetadata: %s' \
+ % (properties, metadata))
+
+ handler = InputHandler(player)
+ gobject.io_add_watch(sys.stdin, gobject.IO_IN, handler.handle)
media.RegisterPlayer(path, properties, metadata)
--
1.7.7
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 03/12] Don't overwrite metadata when changing track
2011-10-12 15:11 ` [PATCH 03/12] Don't overwrite metadata when changing track Lucas De Marchi
@ 2011-10-13 7:42 ` Johan Hedberg
0 siblings, 0 replies; 15+ messages in thread
From: Johan Hedberg @ 2011-10-13 7:42 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: linux-bluetooth
Hi Lucas,
On Wed, Oct 12, 2011, Lucas De Marchi 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 | 41 ++++++++++++++++++++++++++---------------
> doc/media-api.txt | 6 ++++++
> 2 files changed, 32 insertions(+), 15 deletions(-)
The first two patches have been applied but this one doesn't compile:
audio/media.c: In function ‘parse_player_metadata’:
audio/media.c:1358:11: error: variable ‘title’ set but not used [-Werror=unused-but-set-variable]
Johan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 04/12] Make title always available in metadata
2011-10-12 15:11 ` [PATCH 04/12] Make title always available in metadata Lucas De Marchi
@ 2011-10-13 7:44 ` Johan Hedberg
0 siblings, 0 replies; 15+ messages in thread
From: Johan Hedberg @ 2011-10-13 7:44 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: linux-bluetooth
Hi Lucas,
On Wed, Oct 12, 2011, Lucas De Marchi wrote:
> ---
> audio/media.c | 5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/audio/media.c b/audio/media.c
> index c9fe0f0..56913d5 100644
> --- a/audio/media.c
> +++ b/audio/media.c
> @@ -1438,6 +1438,11 @@ static gboolean parse_player_metadata(struct media_player *mp,
> if (g_hash_table_size(track) == 0) {
> g_hash_table_unref(track);
> track = NULL;
> + } else if (title == FALSE) {
> + struct metadata_value *value = g_new(struct metadata_value, 1);
> +
> + value->type = DBUS_TYPE_STRING;
> + value->value.str = g_strdup("");
> }
>
> if (mp->track != NULL)
This doesn't make any sense. You're allocating a new variable which is
only available inside the "else if" scope, and then doing nothing with
it. Additionally this memory is leaked once you exit the scope.
Johan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 12/12] Improve test/simple-player to allow user interaction
2011-10-12 15:11 ` [PATCH 12/12] Improve test/simple-player to allow user interaction Lucas De Marchi
@ 2011-10-13 10:12 ` Johan Hedberg
0 siblings, 0 replies; 15+ messages in thread
From: Johan Hedberg @ 2011-10-13 10:12 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: linux-bluetooth
Hi Lucas,
On Wed, Oct 12, 2011, Lucas De Marchi wrote:
> Allow user to trigger TrackChanged and PropertyChanged. Calls made by
> remote side (coming from D-Bus) are printed to stderr while the ones to
> interact with user go to stdout.
> ---
> test/simple-player | 71 +++++++++++++++++++++++++++++++++++++++++++++++++---
> 1 files changed, 67 insertions(+), 4 deletions(-)
Patches 5-12 have been applied also. Thanks.
Johan
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2011-10-13 10:12 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-12 15:11 [PATCH 01/12] Add mpris-player to .gitignore Lucas De Marchi
2011-10-12 15:11 ` [PATCH 02/12] Fix typo on doc Lucas De Marchi
2011-10-12 15:11 ` [PATCH 03/12] Don't overwrite metadata when changing track Lucas De Marchi
2011-10-13 7:42 ` Johan Hedberg
2011-10-12 15:11 ` [PATCH 04/12] Make title always available in metadata Lucas De Marchi
2011-10-13 7:44 ` Johan Hedberg
2011-10-12 15:11 ` [PATCH 05/12] Move debug messages to their correspondent getters Lucas De Marchi
2011-10-12 15:11 ` [PATCH 06/12] AVRCP: Do not list values for unsupported attributes Lucas De Marchi
2011-10-12 15:11 ` [PATCH 07/12] Add callback to list available metadata IDs Lucas De Marchi
2011-10-12 15:11 ` [PATCH 08/12] AVRCP: Return error for invalid " Lucas De Marchi
2011-10-12 15:11 ` [PATCH 09/12] AVRCP: Check if len matches number of IDs Lucas De Marchi
2011-10-12 15:11 ` [PATCH 10/12] AVRCP: Only return available metadata attributes Lucas De Marchi
2011-10-12 15:11 ` [PATCH 11/12] Refactor to share code Lucas De Marchi
2011-10-12 15:11 ` [PATCH 12/12] Improve test/simple-player to allow user interaction Lucas De Marchi
2011-10-13 10:12 ` Johan Hedberg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox