[PATCH 1/2] Fix MediaPlayer documentation

[PATCH 1/2] Fix MediaPlayer documentation

[Lucas De Marchi]

---
 doc/media-api.txt |   10 ++--------
 1 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/doc/media-api.txt b/doc/media-api.txt
index b8dcdbd..e061f38 100644
--- a/doc/media-api.txt
+++ b/doc/media-api.txt
@@ -180,12 +180,6 @@ Signals		PropertyChanged(string setting, variant value)
 
 					Track duration in milliseconds
 
-		StatusChanged(string status, uint32 position)
-
-			Possible status: "playing", "stopped", "paused",
-					"forward-seek", "reverse-seek" or
-					"error"
-
 Properties	string Equalizer [readwrite]
 
 			Possible values: "off" or "on"
@@ -203,13 +197,13 @@ Properties	string Equalizer [readwrite]
 
 			Possible values: "off", "alltracks" or "group"
 
-		string Status [readonly]
+		string Status [readwrite]
 
 			Possible status: "playing", "stopped", "paused",
 					"forward-seek", "reverse-seek" or
 					"error"
 
-		uint32 Position [readonly]
+		uint32 Position [readwrite]
 
 			Playback position in milliseconds
 
-- 
1.7.7

[PATCH 2/2] Fix leak of dbus message

[Lucas De Marchi]

---
 test/mpris-player.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/test/mpris-player.c b/test/mpris-player.c
index 9e63b4b..29bea46 100644
--- a/test/mpris-player.c
+++ b/test/mpris-player.c
@@ -122,6 +122,7 @@ static dbus_bool_t emit_property_changed(DBusConnection *conn,
 {
 	DBusMessage *signal;
 	DBusMessageIter iter;
+	dbus_bool_t result;
 
 	signal = dbus_message_new_signal(path, interface, "PropertyChanged");
 
@@ -137,7 +138,10 @@ static dbus_bool_t emit_property_changed(DBusConnection *conn,
 
 	append_variant(&iter, type, value);
 
-	return dbus_connection_send(conn, signal, NULL);
+	result = dbus_connection_send(conn, signal, NULL);
+	dbus_message_unref(signal);
+
+	return result;
 }
 
 static int parse_property(DBusConnection *conn, const char *path,
-- 
1.7.7

Re: [PATCH 1/2] Fix MediaPlayer documentation

[Luiz Augusto von Dentz]

Hi Lucas,

On Wed, Oct 19, 2011 at 2:24 AM, Lucas De Marchi
<lucas.demarchi@profusion.mobi> wrote:
> ---
>  doc/media-api.txt |   10 ++--------
>  1 files changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/doc/media-api.txt b/doc/media-api.txt
> index b8dcdbd..e061f38 100644
> --- a/doc/media-api.txt
> +++ b/doc/media-api.txt
> @@ -180,12 +180,6 @@ Signals            PropertyChanged(string setting, variant value)
>
>                                        Track duration in milliseconds
>
> -               StatusChanged(string status, uint32 position)
> -
> -                       Possible status: "playing", "stopped", "paused",
> -                                       "forward-seek", "reverse-seek" or
> -                                       "error"
> -

Nice catch.

>  Properties     string Equalizer [readwrite]
>
>                        Possible values: "off" or "on"
> @@ -203,13 +197,13 @@ Properties        string Equalizer [readwrite]
>
>                        Possible values: "off", "alltracks" or "group"
>
> -               string Status [readonly]
> +               string Status [readwrite]
>
>                        Possible status: "playing", "stopped", "paused",
>                                        "forward-seek", "reverse-seek" or
>                                        "error"
>
> -               uint32 Position [readonly]
> +               uint32 Position [readwrite]

Im not sure about making those properties readwrite, iirc there is no
way to set those values directly via AVRCP commands instead there is
the key presses to control the playback, so for now I would leave them
as readonly so the player don't have to handle this in
org.bluez.MediaPlayer.SetProperty.


-- 
Luiz Augusto von Dentz

Re: [PATCH 2/2] Fix leak of dbus message

[Luiz Augusto von Dentz]

Hi Lucas,

On Wed, Oct 19, 2011 at 2:24 AM, Lucas De Marchi
<lucas.demarchi@profusion.mobi> wrote:
> ---
>  test/mpris-player.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/test/mpris-player.c b/test/mpris-player.c
> index 9e63b4b..29bea46 100644
> --- a/test/mpris-player.c
> +++ b/test/mpris-player.c
> @@ -122,6 +122,7 @@ static dbus_bool_t emit_property_changed(DBusConnection *conn,
>  {
>        DBusMessage *signal;
>        DBusMessageIter iter;
> +       dbus_bool_t result;
>
>        signal = dbus_message_new_signal(path, interface, "PropertyChanged");
>
> @@ -137,7 +138,10 @@ static dbus_bool_t emit_property_changed(DBusConnection *conn,
>
>        append_variant(&iter, type, value);
>
> -       return dbus_connection_send(conn, signal, NULL);
> +       result = dbus_connection_send(conn, signal, NULL);
> +       dbus_message_unref(signal);
> +
> +       return result;
>  }
>
>  static int parse_property(DBusConnection *conn, const char *path,
> --
> 1.7.7
>
> --

Ack.

-- 
Luiz Augusto von Dentz

Re: [PATCH 1/2] Fix MediaPlayer documentation

[Lucas De Marchi]

Hi Luiz,

On Wed, Oct 19, 2011 at 7:31 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Lucas,
>
> On Wed, Oct 19, 2011 at 2:24 AM, Lucas De Marchi
> <lucas.demarchi@profusion.mobi> wrote:
>> ---
>>  doc/media-api.txt |   10 ++--------
>>  1 files changed, 2 insertions(+), 8 deletions(-)
>>
>> diff --git a/doc/media-api.txt b/doc/media-api.txt
>> index b8dcdbd..e061f38 100644
>> --- a/doc/media-api.txt
>> +++ b/doc/media-api.txt
>> @@ -180,12 +180,6 @@ Signals            PropertyChanged(string setting, variant value)
>>
>>                                        Track duration in milliseconds
>>
>> -               StatusChanged(string status, uint32 position)
>> -
>> -                       Possible status: "playing", "stopped", "paused",
>> -                                       "forward-seek", "reverse-seek" or
>> -                                       "error"
>> -
>
> Nice catch.
>
>>  Properties     string Equalizer [readwrite]
>>
>>                        Possible values: "off" or "on"
>> @@ -203,13 +197,13 @@ Properties        string Equalizer [readwrite]
>>
>>                        Possible values: "off", "alltracks" or "group"
>>
>> -               string Status [readonly]
>> +               string Status [readwrite]
>>
>>                        Possible status: "playing", "stopped", "paused",
>>                                        "forward-seek", "reverse-seek" or
>>                                        "error"
>>
>> -               uint32 Position [readonly]
>> +               uint32 Position [readwrite]
>
> Im not sure about making those properties readwrite, iirc there is no
> way to set those values directly via AVRCP commands instead there is
> the key presses to control the playback, so for now I would leave them
> as readonly so the player don't have to handle this in
> org.bluez.MediaPlayer.SetProperty.

/me confused.... I'm just documenting what's implemented right now.
Otherwise, if we don't have the StatusChanged method above, how does
user set position and status?

Looking at the impl. of mpris-player, e.g.:

parse_property() {
...
			emit_property_changed(sys, path,
					"org.bluez.MediaPlayer", "Status",
					DBUS_TYPE_STRING, &value);
...
			emit_property_changed(sys, path,
					"org.bluez.MediaPlayer", "Position",
					DBUS_TYPE_UINT32, &msec);
}

regards,
Lucas De Marchi

Re: [PATCH 1/2] Fix MediaPlayer documentation

[Lucas De Marchi]

Hi Luiz,

On Wed, Oct 19, 2011 at 9:45 AM, Lucas De Marchi
<lucas.demarchi@profusion.mobi> wrote:
> Hi Luiz,
>
> On Wed, Oct 19, 2011 at 7:31 AM, Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
>> Hi Lucas,
>>
>> On Wed, Oct 19, 2011 at 2:24 AM, Lucas De Marchi
>> <lucas.demarchi@profusion.mobi> wrote:
>>> ---
>>>  doc/media-api.txt |   10 ++--------
>>>  1 files changed, 2 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/doc/media-api.txt b/doc/media-api.txt
>>> index b8dcdbd..e061f38 100644
>>> --- a/doc/media-api.txt
>>> +++ b/doc/media-api.txt
>>> @@ -180,12 +180,6 @@ Signals            PropertyChanged(string setting, variant value)
>>>
>>>                                        Track duration in milliseconds
>>>
>>> -               StatusChanged(string status, uint32 position)
>>> -
>>> -                       Possible status: "playing", "stopped", "paused",
>>> -                                       "forward-seek", "reverse-seek" or
>>> -                                       "error"
>>> -
>>
>> Nice catch.
>>
>>>  Properties     string Equalizer [readwrite]
>>>
>>>                        Possible values: "off" or "on"
>>> @@ -203,13 +197,13 @@ Properties        string Equalizer [readwrite]
>>>
>>>                        Possible values: "off", "alltracks" or "group"
>>>
>>> -               string Status [readonly]
>>> +               string Status [readwrite]
>>>
>>>                        Possible status: "playing", "stopped", "paused",
>>>                                        "forward-seek", "reverse-seek" or
>>>                                        "error"
>>>
>>> -               uint32 Position [readonly]
>>> +               uint32 Position [readwrite]
>>
>> Im not sure about making those properties readwrite, iirc there is no
>> way to set those values directly via AVRCP commands instead there is
>> the key presses to control the playback, so for now I would leave them
>> as readonly so the player don't have to handle this in
>> org.bluez.MediaPlayer.SetProperty.
>
> /me confused.... I'm just documenting what's implemented right now.
> Otherwise, if we don't have the StatusChanged method above, how does
> user set position and status?
>
> Looking at the impl. of mpris-player, e.g.:
>
> parse_property() {
> ...
>                        emit_property_changed(sys, path,
>                                        "org.bluez.MediaPlayer", "Status",
>                                        DBUS_TYPE_STRING, &value);
> ...
>                        emit_property_changed(sys, path,
>                                        "org.bluez.MediaPlayer", "Position",
>                                        DBUS_TYPE_UINT32, &msec);
> }
>

Humn... readwrite there is about properties that can be set through
SetProperty, not the PropertyChanged signal. I'll remove this change
and re-submit.

regards,
Lucas De Marchi

[PATCH] Fix MediaPlayer documentation

[Lucas De Marchi]

---
 doc/media-api.txt |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/doc/media-api.txt b/doc/media-api.txt
index b8dcdbd..c748e50 100644
--- a/doc/media-api.txt
+++ b/doc/media-api.txt
@@ -180,12 +180,6 @@ Signals		PropertyChanged(string setting, variant value)
 
 					Track duration in milliseconds
 
-		StatusChanged(string status, uint32 position)
-
-			Possible status: "playing", "stopped", "paused",
-					"forward-seek", "reverse-seek" or
-					"error"
-
 Properties	string Equalizer [readwrite]
 
 			Possible values: "off" or "on"
-- 
1.7.7

Re: [PATCH] Fix MediaPlayer documentation

[Johan Hedberg]

Hi Lucas,

On Wed, Oct 19, 2011, Lucas De Marchi wrote:
> ---
>  doc/media-api.txt |    6 ------
>  1 files changed, 0 insertions(+), 6 deletions(-)
> 
> diff --git a/doc/media-api.txt b/doc/media-api.txt
> index b8dcdbd..c748e50 100644
> --- a/doc/media-api.txt
> +++ b/doc/media-api.txt
> @@ -180,12 +180,6 @@ Signals		PropertyChanged(string setting, variant value)
>  
>  					Track duration in milliseconds
>  
> -		StatusChanged(string status, uint32 position)
> -
> -			Possible status: "playing", "stopped", "paused",
> -					"forward-seek", "reverse-seek" or
> -					"error"
> -
>  Properties	string Equalizer [readwrite]
>  
>  			Possible values: "off" or "on"

Applied. Thanks.

Johan

Re: [PATCH 2/2] Fix leak of dbus message

[Johan Hedberg]

Hi Lucas,

On Tue, Oct 18, 2011, Lucas De Marchi wrote:
> ---
>  test/mpris-player.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)

Applied. Thanks.

Johan