[PATCH 1/5] Fix handling of dbus signals

[PATCH 1/5] Fix handling of dbus signals

[Lucas De Marchi]

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

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

[Lucas De Marchi]

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

[PATCH 3/5] Fix leak of GTimer

[Lucas De Marchi]

---
 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

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

[Lucas De Marchi]

---
 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

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

[Lucas De Marchi]

---
 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

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

[Luiz Augusto von Dentz]

Hi Lucas,

On Fri, Oct 21, 2011 at 8:58 PM, Lucas De Marchi
<lucas.demarchi@profusion.mobi> 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

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

[Luiz Augusto von Dentz]

Hi Lucas,

On Fri, Oct 21, 2011 at 8:58 PM, Lucas De Marchi
<lucas.demarchi@profusion.mobi> 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

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

[Luiz Augusto von Dentz]

Hi Lucas,

On Fri, Oct 21, 2011 at 8:58 PM, Lucas De Marchi
<lucas.demarchi@profusion.mobi> 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

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

[Luiz Augusto von Dentz]

Hi Lucas,

On Fri, Oct 21, 2011 at 8:58 PM, Lucas De Marchi
<lucas.demarchi@profusion.mobi> 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

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

[Luiz Augusto von Dentz]

Hi Lucas,

On Fri, Oct 21, 2011 at 8:58 PM, Lucas De Marchi
<lucas.demarchi@profusion.mobi> 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

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

[Johan Hedberg]

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