2011-10-18 23:24:38

by Lucas De Marchi

[permalink] [raw]
Subject: [PATCH 1/2] Fix MediaPlayer documentation

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



2011-10-20 07:37:10

by Johan Hedberg

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

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

2011-10-20 07:36:47

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH] Fix MediaPlayer documentation

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

2011-10-19 12:05:00

by Lucas De Marchi

[permalink] [raw]
Subject: [PATCH] Fix MediaPlayer documentation

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


2011-10-19 12:00:49

by Lucas De Marchi

[permalink] [raw]
Subject: Re: [PATCH 1/2] Fix MediaPlayer documentation

Hi Luiz,

On Wed, Oct 19, 2011 at 9:45 AM, Lucas De Marchi
<[email protected]> wrote:
> Hi Luiz,
>
> On Wed, Oct 19, 2011 at 7:31 AM, Luiz Augusto von Dentz
> <[email protected]> wrote:
>> Hi Lucas,
>>
>> On Wed, Oct 19, 2011 at 2:24 AM, Lucas De Marchi
>> <[email protected]> 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

2011-10-19 11:45:56

by Lucas De Marchi

[permalink] [raw]
Subject: Re: [PATCH 1/2] Fix MediaPlayer documentation

Hi Luiz,

On Wed, Oct 19, 2011 at 7:31 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Lucas,
>
> On Wed, Oct 19, 2011 at 2:24 AM, Lucas De Marchi
> <[email protected]> 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

2011-10-19 09:32:21

by Luiz Augusto von Dentz

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

Hi Lucas,

On Wed, Oct 19, 2011 at 2:24 AM, Lucas De Marchi
<[email protected]> 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

2011-10-19 09:31:58

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 1/2] Fix MediaPlayer documentation

Hi Lucas,

On Wed, Oct 19, 2011 at 2:24 AM, Lucas De Marchi
<[email protected]> 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

2011-10-18 23:24:39

by Lucas De Marchi

[permalink] [raw]
Subject: [PATCH 2/2] Fix leak of dbus message

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