2011-10-21 17:58:24

by Lucas De Marchi

Subject: [PATCH 1/5] Fix handling of dbus signals

We're previously leaking a D-Bus message and always returning that
the signal was not handled.
---
test/mpris-player.c | 16 +++++++++++-----
1 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/test/mpris-player.c b/test/mpris-player.c
index 29bea46..4592e5d 100644
--- a/test/mpris-player.c
+++ b/test/mpris-player.c
@@ -549,7 +549,7 @@ static DBusHandlerResult properties_changed(DBusConnection *conn,
if (!signal) {
fprintf(stderr, "Unable to allocate new PropertyChanged"
" signal\n");
- goto done;
+ goto err;
}

dbus_message_iter_init_append(signal, &entry);
@@ -563,13 +563,19 @@ static DBusHandlerResult properties_changed(DBusConnection *conn,
dbus_message_iter_next(&iter);

if (parse_metadata(&iter, &metadata) < 0)
- goto done;
+ goto err;

dbus_message_iter_close_container(&entry, &metadata);

dbus_connection_send(sys, signal, NULL);
+ dbus_message_unref(signal);
+ g_free(path);

-done:
+ return DBUS_HANDLER_RESULT_HANDLED;
+
+err:
+ if (signal)
+ dbus_message_unref(signal);
g_free(path);
return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
}
@@ -607,7 +613,7 @@ static DBusHandlerResult session_filter(DBusConnection *conn,
add_player(conn, name, new);
}

- return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+ return DBUS_HANDLER_RESULT_HANDLED;
}

static DBusHandlerResult system_filter(DBusConnection *conn,
@@ -633,7 +639,7 @@ static DBusHandlerResult system_filter(DBusConnection *conn,
__io_terminated = 1;
}

- return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+ return DBUS_HANDLER_RESULT_HANDLED;
}

static char *get_default_adapter(DBusConnection *conn)
--
1.7.7

2011-10-22 10:21:32

by Johan Hedberg

Subject: Re: [PATCH 4/5] Unregister players' events when disconnected

Hi Lucas,

On Fri, Oct 21, 2011, Lucas De Marchi wrote:
> ---
> audio/avrcp.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)

Patches 1-4 have been applied. Thanks.

The volume stuff (patch 5) needs more discussion.

Johan

2011-10-22 10:19:00

by Luiz Augusto von Dentz

Subject: Re: [PATCH 5/5] AVRCP: handle volume up/down keys as TG

Hi Lucas,

On Fri, Oct 21, 2011 at 8:58 PM, Lucas De Marchi
<[email protected]> wrote:
> ---
> ?audio/avctp.c | ? ?2 ++
> ?1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/audio/avctp.c b/audio/avctp.c
> index df3b2b8..fdd8a9e 100644
> --- a/audio/avctp.c
> +++ b/audio/avctp.c
> @@ -155,6 +155,8 @@ static struct {
> ? ? ? ?{ "BACKWARD", ? ? ? ? ? BACKWARD_OP, ? ? ? ? ? ?KEY_PREVIOUSSONG },
> ? ? ? ?{ "REWIND", ? ? ? ? ? ? REWIND_OP, ? ? ? ? ? ? ?KEY_REWIND },
> ? ? ? ?{ "FAST FORWARD", ? ? ? FAST_FORWARD_OP, ? ? ? ?KEY_FASTFORWARD },
> + ? ? ? { "VOLUME UP", ? ? ? ? ?VOL_UP_OP, ? ? ? ? ? ? ?KEY_VOLUMEUP },
> + ? ? ? { "VOLUME DOWN", ? ? ? ?VOL_DOWN_OP, ? ? ? ? ? ?KEY_VOLUMEDOWN },
> ? ? ? ?{ NULL }
> ?};
>
> --
> 1.7.7
>

I remember discussing this before, this is not as simple as it seems
because we need to check what A2DP role we are doing so I would like
to do absolute volume control instead which solves the volume
synchronization problem and don't have any recommendation against
using it.

--
Luiz Augusto von Dentz

2011-10-22 10:10:50

by Luiz Augusto von Dentz

Subject: Re: [PATCH 4/5] Unregister players' events when disconnected

Hi Lucas,

On Fri, Oct 21, 2011 at 8:58 PM, Lucas De Marchi
<[email protected]> wrote:
> ---
> ?audio/avrcp.c | ? ?1 +
> ?1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/audio/avrcp.c b/audio/avrcp.c
> index e908bf4..f06afbf 100644
> --- a/audio/avrcp.c
> +++ b/audio/avrcp.c
> @@ -1119,6 +1119,7 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
> ? ? ? ?switch (new_state) {
> ? ? ? ?case AVCTP_STATE_DISCONNECTED:
> ? ? ? ? ? ? ? ?player->session = NULL;
> + ? ? ? ? ? ? ? player->registered_events = 0;
>
> ? ? ? ? ? ? ? ?if (player->handler) {
> ? ? ? ? ? ? ? ? ? ? ? ?avctp_unregister_pdu_handler(player->handler);
> --
> 1.7.7
>

Ack.

--
Luiz Augusto von Dentz

2011-10-22 10:10:21

by Luiz Augusto von Dentz

Subject: Re: [PATCH 3/5] Fix leak of GTimer

Hi Lucas,

On Fri, Oct 21, 2011 at 8:58 PM, Lucas De Marchi
<[email protected]> wrote:
> ---
> ?audio/media.c | ? ?1 +
> ?1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/audio/media.c b/audio/media.c
> index ade2a4b..5528ada 100644
> --- a/audio/media.c
> +++ b/audio/media.c
> @@ -885,6 +885,7 @@ static void media_player_free(gpointer data)
> ? ? ? ?if (mp->settings)
> ? ? ? ? ? ? ? ?g_hash_table_unref(mp->settings);
>
> + ? ? ? g_timer_destroy(mp->timer);
> ? ? ? ?g_free(mp->sender);
> ? ? ? ?g_free(mp->path);
> ? ? ? ?g_free(mp);
> --
> 1.7.7
>

Ack.

--
Luiz Augusto von Dentz

2011-10-22 10:09:52

by Luiz Augusto von Dentz

Subject: Re: [PATCH 2/5] Add missing TRACK-REACHED-START event

Hi Lucas,

On Fri, Oct 21, 2011 at 8:58 PM, Lucas De Marchi
<[email protected]> wrote:
> When track is changed, we automatically start to play it, therefore we
> need to notify CT regarding this event.
> ---
> ?audio/media.c | ? ?2 ++
> ?1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/audio/media.c b/audio/media.c
> index a7647ea..ade2a4b 100644
> --- a/audio/media.c
> +++ b/audio/media.c
> @@ -1488,6 +1488,8 @@ static gboolean parse_player_metadata(struct media_player *mp,
> ? ? ? ?uid = get_uid(mp);
>
> ? ? ? ?avrcp_player_event(mp->player, AVRCP_EVENT_TRACK_CHANGED, &uid);
> + ? ? ? avrcp_player_event(mp->player, AVRCP_EVENT_TRACK_REACHED_START,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? NULL);
>
> ? ? ? ?return TRUE;
>
> --
> 1.7.7

Ack.

--
Luiz Augusto von Dentz

2011-10-22 10:09:16

by Luiz Augusto von Dentz

Subject: Re: [PATCH 1/5] Fix handling of dbus signals

Hi Lucas,

On Fri, Oct 21, 2011 at 8:58 PM, Lucas De Marchi
<[email protected]> wrote:
> We're previously leaking a D-Bus message and always returning that
> the signal was not handled.
> ---
> ?test/mpris-player.c | ? 16 +++++++++++-----
> ?1 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/test/mpris-player.c b/test/mpris-player.c
> index 29bea46..4592e5d 100644
> --- a/test/mpris-player.c
> +++ b/test/mpris-player.c
> @@ -549,7 +549,7 @@ static DBusHandlerResult properties_changed(DBusConnection *conn,
> ? ? ? ?if (!signal) {
> ? ? ? ? ? ? ? ?fprintf(stderr, "Unable to allocate new PropertyChanged"
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?" signal\n");
> - ? ? ? ? ? ? ? goto done;
> + ? ? ? ? ? ? ? goto err;
> ? ? ? ?}
>
> ? ? ? ?dbus_message_iter_init_append(signal, &entry);
> @@ -563,13 +563,19 @@ static DBusHandlerResult properties_changed(DBusConnection *conn,
> ? ? ? ?dbus_message_iter_next(&iter);
>
> ? ? ? ?if (parse_metadata(&iter, &metadata) < 0)
> - ? ? ? ? ? ? ? goto done;
> + ? ? ? ? ? ? ? goto err;
>
> ? ? ? ?dbus_message_iter_close_container(&entry, &metadata);
>
> ? ? ? ?dbus_connection_send(sys, signal, NULL);
> + ? ? ? dbus_message_unref(signal);
> + ? ? ? g_free(path);
>
> -done:
> + ? ? ? return DBUS_HANDLER_RESULT_HANDLED;
> +
> +err:
> + ? ? ? if (signal)
> + ? ? ? ? ? ? ? dbus_message_unref(signal);
> ? ? ? ?g_free(path);
> ? ? ? ?return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
> ?}
> @@ -607,7 +613,7 @@ static DBusHandlerResult session_filter(DBusConnection *conn,
> ? ? ? ? ? ? ? ?add_player(conn, name, new);
> ? ? ? ?}
>
> - ? ? ? return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
> + ? ? ? return DBUS_HANDLER_RESULT_HANDLED;
> ?}
>
> ?static DBusHandlerResult system_filter(DBusConnection *conn,
> @@ -633,7 +639,7 @@ static DBusHandlerResult system_filter(DBusConnection *conn,
> ? ? ? ? ? ? ? ?__io_terminated = 1;
> ? ? ? ?}
>
> - ? ? ? return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
> + ? ? ? return DBUS_HANDLER_RESULT_HANDLED;
> ?}
>
> ?static char *get_default_adapter(DBusConnection *conn)
> --
> 1.7.7
>
> --

Ack.


--
Luiz Augusto von Dentz

2011-10-21 17:58:28

by Lucas De Marchi

Subject: [PATCH 5/5] AVRCP: handle volume up/down keys as TG

---
audio/avctp.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/audio/avctp.c b/audio/avctp.c
index df3b2b8..fdd8a9e 100644
--- a/audio/avctp.c
+++ b/audio/avctp.c
@@ -155,6 +155,8 @@ static struct {
{ "BACKWARD", BACKWARD_OP, KEY_PREVIOUSSONG },
{ "REWIND", REWIND_OP, KEY_REWIND },
{ "FAST FORWARD", FAST_FORWARD_OP, KEY_FASTFORWARD },
+ { "VOLUME UP", VOL_UP_OP, KEY_VOLUMEUP },
+ { "VOLUME DOWN", VOL_DOWN_OP, KEY_VOLUMEDOWN },
{ NULL }
};

--
1.7.7

2011-10-21 17:58:27

by Lucas De Marchi

Subject: [PATCH 4/5] Unregister players' events when disconnected

---
audio/avrcp.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/audio/avrcp.c b/audio/avrcp.c
index e908bf4..f06afbf 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -1119,6 +1119,7 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
switch (new_state) {
case AVCTP_STATE_DISCONNECTED:
player->session = NULL;
+ player->registered_events = 0;

if (player->handler) {
avctp_unregister_pdu_handler(player->handler);
--
1.7.7

2011-10-21 17:58:26

by Lucas De Marchi

Subject: [PATCH 3/5] Fix leak of GTimer

---
audio/media.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/audio/media.c b/audio/media.c
index ade2a4b..5528ada 100644
--- a/audio/media.c
+++ b/audio/media.c
@@ -885,6 +885,7 @@ static void media_player_free(gpointer data)
if (mp->settings)
g_hash_table_unref(mp->settings);

+ g_timer_destroy(mp->timer);
g_free(mp->sender);
g_free(mp->path);
g_free(mp);
--
1.7.7

2011-10-21 17:58:25

by Lucas De Marchi

Subject: [PATCH 2/5] Add missing TRACK-REACHED-START event

When track is changed, we automatically start to play it, therefore we
need to notify CT regarding this event.
---
audio/media.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/audio/media.c b/audio/media.c
index a7647ea..ade2a4b 100644
--- a/audio/media.c
+++ b/audio/media.c
@@ -1488,6 +1488,8 @@ static gboolean parse_player_metadata(struct media_player *mp,
uid = get_uid(mp);

avrcp_player_event(mp->player, AVRCP_EVENT_TRACK_CHANGED, &uid);
+ avrcp_player_event(mp->player, AVRCP_EVENT_TRACK_REACHED_START,
+ NULL);

return TRUE;

--
1.7.7