Re: [Musicpd-dev-team] Empty fields
Hi, This behavior has been introduced in MPD 0.14: When MPD returns an empty value, it doesn't mean that the tag is empty but that the tag is unknown (not set in the music file). As a client developer I find this behavior very useful for example if you want to build a tree like this: The Doors L.A. Woman Strange Days Unknown Album It's also useful if some of your music files are badly tagged. Marc 2009/3/6 Jerome Quelin jque...@gmail.com On 09/03/06 10:30 +0100, Max Kellermann wrote: On 2009/03/06 10:24, Jerome Quelin jque...@gmail.com wrote: definitely sure: i had to update the test-suite of the perl modules audio::mpd and poe::component::client::mpd to take this into account. i'm not saying it's right or wrong, but it has definitely changed Ok, so let's talk about whether it's right or wrong. Does it make any sense to have an empty string in a tag value? Should MPD discard the whole tag item in this case? My opinion: kill empty values. that's also my opinion, but then i can understand people wanting to have empty values. indeed, they would not be able to say whether all songs have an album, or if some don't have an album. in the end, i don't really care. just pick a behaviour and stick with it. i would hate to have to update the test suite every mpd version! :-) jérôme -- jque...@gmail.com -- Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA -OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise -Strategies to boost innovation and cut costs with open source participation -Receive a $600 discount off the registration fee with the source code: SFAD http://p.sf.net/sfu/XcvMzF8H ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team -- Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA -OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise -Strategies to boost innovation and cut costs with open source participation -Receive a $600 discount off the registration fee with the source code: SFAD http://p.sf.net/sfu/XcvMzF8H___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] The libmpdclient v2.0 project
11 other patches available on my repository: Add unit tests to libmpclient Fix glib include Change include guards Convert MPD_INFO_ENTITY_ to enum Fix currentsong test Cosmetic Improve calls to mpd_executeCommand Small performance improvements Split libmpdclient: entity Bug fix Split connection into connection ipc What do you mean by ''libmpdclient should be stateless regarding its caller program'? Do you plan to remove the getNext* commands and to make the send* commands return data? In this case, how do you want to implement CommandList'? Marc - This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100url=/___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] The libmpdclient v2.0 project
- I saw that your idle patch introduced inconsistent indentation (e.g. mpd_glibStartIdle); please configure your editor to indent with tabs. I don't want to start a 'religion war' on this point but don't you think it would be better to use spaces everywhere instead of tabs? This discussion has already been done thousands of times by other people so I won't say more but you can find a lot of projects using spaces instead of tabs because tabs are always a nightmare for projects involving different developers and for developers contributing to several projects. (spaces indent makes you sure that the files will appear identically in any text editor). - A commit should include a brief one-line description, followed by an empty line, and a longer description following. This way, stuff like git shortlog works properly. Don't write another description in the email - a well-documented patch doesn't need further explanation. Ok. It's my first time using git but I will remember it. - set local includes first, to test if the include file has specified its own dependencies properly, i.e. include idle.h before stdio.h and string.h in idle.c. In return_element.c you did this very well: return_element.h before str_pool.h (str_pool.h was already checked by str_pool.c). Ok. - This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100url=/___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] The libmpdclient v2.0 project
Hi, There are some commits on my repository that you may find interesting to integrate. Marc Pavot: Use tabs instead of spaces Split libmpdclient: status Set local includes first for idle.c Split libmpdclient: stats Merge branch 'master' of git://git.musicpd.org/cirrus/libmpdclient Merge branch 'master' of git://git.musicpd.org/cirrus/libmpdclient Remove useless malloc/free changed typedef mpd_Status to struct changed typedef mpd_Stats and mpd_SearchStats to struct New disconnect event W32 support of idle mode Marc - This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100url=/___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] The libmpdclient v2.0 project
- portability - even Windows? There's no good C99 compiler on Windows.. mingw32? Just a quick remark about this point: I compile Ario and libmpdclient on windows with mingw32 and it works fine. (I distribute .exe files for Ario). - This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100url=/___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] The libmpdclient v2.0 project
Some of you already promised to help me with that. From now on, I will accept patches (git pull requests preferred). Please keep your git repository rebased on mine. I have created a git repository to help you on this project: http://repo.or.cz/w/libmpdclient/marc.git It is based on your repository and I have done one commit to continue the split of libmpdclient. It creates three new files: connection, idle and return_element. The separation is not perfect (for example some structures and methods in connection will have to become private in the future) but it's a first brick. Marc - This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100url=/___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] [PATCH] idle command enhancement
I attach a new patch that integrates some fixes and ideas proposed by Qball.
This patch replaces the previous one.
I have added the support of idle command in Ario (
http://ario-player.sourceforge.net/) and I have found something that looks
like a bug in MPD:
- When a song is finished, the next one is played and the 'player' event is
emitted.
- When the client sends the status command just after this event, the songid
is the new one but the 'elapsed' time is not reseted to 0.
This is problem because I have implemented the solution using a timer on
client side to compute the elapsed time but with this bug the elapsed time
continues to be incremented on a new song.
I propose a patch to MPD to fix this issue but I don't really know this part
of MPD so it may not be the best solution.
Marc
2008/11/23 Marc Pavot [EMAIL PROTECTED]
Hi,
Thanks, I have merged your patch. Please write a patch which adds
documentation for that feature to doc/protocol.xml.
Done.
I also attach a patch to libmpdclient that can be used as a basis for the
discussion on 'idle' implementation on client side.
Main points about this implementation:
- It is designed to be integrated into client main loop. This solution has
the advantage to avoid multithreading problems and to be well integrated
into client applications. The disadvantage is that we have to implement
something specific for each kind of loop. My patch comes with a generic
callback mechanism to allow different implementations and with an
implementation for GLib main loop.
- The client application starts by initializing the main loop integration
(mpd_glibInit)
- It can then start or stop the idle mode (mpd_startIdle and mpd_stopIdle).
In mpd_startIdle, the client specifies a callback to be called if a
notification is received.
- If the client application tries to send a command while it is in idle
mode, 'noidle' is sent and the callback is called (if needed) before the
real command.
Hope this helps.
Marc
diff --git a/libmpdclient.c b/libmpdclient.c
index 1c19dd8..2071af6 100644
--- a/libmpdclient.c
+++ b/libmpdclient.c
@@ -81,6 +81,17 @@
# define WSACleanup() do { /* nothing */ } while (0)
#endif
+static const char *const idle_names[] = {
+ database,
+ stored_playlist,
+ playlist,
+ player,
+ mixer,
+ output,
+ options,
+ NULL
+};
+
#ifdef WIN32
static int winsock_dll_error(mpd_Connection *connection)
{
@@ -367,6 +378,13 @@ mpd_Connection * mpd_newConnection(const char * host, int port, float timeout) {
connection-doneListOk = 0;
connection-returnElement = NULL;
connection-request = NULL;
+#ifdef MPD_GLIB
+ connection-source_id = 0;
+#endif
+ connection-idle = 0;
+ connection-startIdle = NULL;
+ connection-stopIdle = NULL;
+ connection-notify_cb = NULL;
if (winsock_dll_error(connection))
return connection;
@@ -446,6 +464,9 @@ static void mpd_executeCommand(mpd_Connection * connection, char * command) {
char * commandPtr = command;
int commandLen = strlen(command);
+ if (connection-idle)
+ mpd_stopIdle(connection);
+
if(!connection-doneProcessing !connection-commandList) {
strcpy(connection-errorStr,not done processing current command);
connection-error = 1;
@@ -1965,3 +1986,105 @@ void mpd_sendPlaylistDeleteCommand(mpd_Connection *connection,
free(sPlaylist);
free(string);
}
+
+static void mpd_readChanges(mpd_Connection *connection)
+{
+ unsigned i;
+ unsigned flags = 0;
+ mpd_ReturnElement *re;
+
+ if (!connection-returnElement) mpd_getNextReturnElement(connection);
+
+ while (connection-returnElement) {
+ re = connection-returnElement;
+ if (re-name !strncmp (re-name, changed, strlen (changed))) {
+ for (i = 0; idle_names[i]; ++i) {
+if (!strcmp (re-value, idle_names[i])) {
+ flags |= (1 i);
+}
+ }
+ }
+ mpd_getNextReturnElement(connection);
+ }
+
+ /* Notifiy application */
+ if (connection-notify_cb flags)
+ connection-notify_cb (connection, flags, connection-userdata);
+}
+
+void mpd_startIdle(mpd_Connection *connection, mpd_NotificationCb notify_cb, void *userdata)
+{
+if (connection-idle)
+return;
+
+ if (connection-startIdle)
+ connection-startIdle(connection);
+
+ mpd_executeCommand(connection, idle\n);
+ connection-idle = 1;
+ connection-notify_cb = notify_cb;
+ connection-userdata = userdata;
+}
+
+void mpd_stopIdle(mpd_Connection *connection)
+{
+ if (connection-stopIdle)
+ connection-stopIdle(connection);
+
+ connection-idle = 0;
+ connection-notify_cb = NULL;
+ connection-doneProcessing = 1;
+ mpd_executeCommand(connection, noidle\n);
+ mpd_readChanges(connection);
+}
+
+#ifdef MPD_GLIB
+static gboolean mpd_glibReadCb (GIOChannel *iochan, GIOCondition cond, gpointer data)
+{
+ mpd_Connection *connection = data;
+
+ if (!connection-idle) {
+ connection-source_id = 0;
+ return FALSE;
+ }
+
+ if ((cond G_IO_IN)) {
+ connection-idle = 0;
+ if (connection-source_id) {
+ g_source_remove (connection-source_id
Re: [Musicpd-dev-team] [PATCH] idle command enhancement
Hi,
Thanks, I have merged your patch. Please write a patch which adds
documentation for that feature to doc/protocol.xml.
Done.
I also attach a patch to libmpdclient that can be used as a basis for the
discussion on 'idle' implementation on client side.
Main points about this implementation:
- It is designed to be integrated into client main loop. This solution has
the advantage to avoid multithreading problems and to be well integrated
into client applications. The disadvantage is that we have to implement
something specific for each kind of loop. My patch comes with a generic
callback mechanism to allow different implementations and with an
implementation for GLib main loop.
- The client application starts by initializing the main loop integration
(mpd_glibInit)
- It can then start or stop the idle mode (mpd_startIdle and mpd_stopIdle).
In mpd_startIdle, the client specifies a callback to be called if a
notification is received.
- If the client application tries to send a command while it is in idle
mode, 'noidle' is sent and the callback is called (if needed) before the
real command.
Hope this helps.
Marc
diff --git a/doc/protocol.xml b/doc/protocol.xml
index 49f6d97..304b492 100644
--- a/doc/protocol.xml
+++ b/doc/protocol.xml
@@ -118,6 +118,7 @@
term
cmdsynopsis
commandidle/command
+ arg choice=opt rep=repeatreplaceableSUBSYSTEMS/replaceable/arg
/cmdsynopsis
/term
listitem
@@ -184,6 +185,11 @@
commandidle/command mode and print results
immediately; might be empty at this time.
/para
+para
+ If the optional varnameSUBSYSTEMS/varname argument is used,
+ MPD will only send notifications when something changed in
+ one of the specified subsytems.
+/para
/listitem
/varlistentry
varlistentry id=command_status
diff --git a/libmpdclient.c b/libmpdclient.c
index 1c19dd8..32ca719 100644
--- a/libmpdclient.c
+++ b/libmpdclient.c
@@ -81,6 +81,17 @@
# define WSACleanup() do { /* nothing */ } while (0)
#endif
+static const char *const idle_names[] = {
+ database,
+ stored_playlist,
+ playlist,
+ player,
+ mixer,
+ output,
+ options,
+ NULL
+};
+
#ifdef WIN32
static int winsock_dll_error(mpd_Connection *connection)
{
@@ -367,6 +378,13 @@ mpd_Connection * mpd_newConnection(const char * host, int port, float timeout) {
connection-doneListOk = 0;
connection-returnElement = NULL;
connection-request = NULL;
+#ifdef MPD_GLIB
+ connection-source_id = 0;
+#endif
+ connection-idle = 0;
+ connection-startIdle = NULL;
+ connection-stopIdle = NULL;
+ connection-notify_cb = NULL;
if (winsock_dll_error(connection))
return connection;
@@ -446,6 +464,9 @@ static void mpd_executeCommand(mpd_Connection * connection, char * command) {
char * commandPtr = command;
int commandLen = strlen(command);
+ if (connection-idle)
+ mpd_stopIdle(connection);
+
if(!connection-doneProcessing !connection-commandList) {
strcpy(connection-errorStr,not done processing current command);
connection-error = 1;
@@ -1965,3 +1986,100 @@ void mpd_sendPlaylistDeleteCommand(mpd_Connection *connection,
free(sPlaylist);
free(string);
}
+
+static void mpd_readChanges(mpd_Connection *connection)
+{
+ unsigned i;
+ unsigned flags = 0;
+ mpd_ReturnElement *re;
+
+ if (!connection-returnElement) mpd_getNextReturnElement(connection);
+
+ while (connection-returnElement) {
+ re = connection-returnElement;
+ if (re-name !strncmp (re-name, changed, strlen (changed))) {
+ for (i = 0; idle_names[i]; ++i) {
+if (!strcmp (re-value, idle_names[i])) {
+ flags |= (1 i);
+}
+ }
+ }
+ mpd_getNextReturnElement(connection);
+ }
+
+ /* Notifiy application */
+ if (connection-notify_cb flags)
+ connection-notify_cb (connection, flags);
+}
+
+void mpd_startIdle(mpd_Connection *connection, mpd_NotificationCb notify_cb)
+{
+ if (connection-startIdle)
+ connection-startIdle(connection);
+
+ mpd_executeCommand(connection, idle\n);
+ connection-idle = 1;
+ connection-notify_cb = notify_cb;
+}
+
+void mpd_stopIdle(mpd_Connection *connection)
+{
+ if (connection-stopIdle)
+ connection-stopIdle(connection);
+
+ connection-idle = 0;
+ connection-notify_cb = NULL;
+ connection-doneProcessing = 1;
+ mpd_executeCommand(connection, noidle\n);
+ mpd_readChanges(connection);
+}
+
+#ifdef MPD_GLIB
+static gboolean mpd_glibReadCb (GIOChannel *iochan, GIOCondition cond, gpointer data)
+{
+ mpd_Connection *connection = data;
+
+ if (!connection-idle) {
+ connection-source_id = 0;
+ return FALSE;
+ }
+
+ if ((cond G_IO_IN)) {
+ connection-idle = 0;
+ if (connection-source_id) {
+ g_source_remove (connection-source_id);
+ connection-source_id = 0;
+ }
+ mpd_readChanges(connection);
+ }
+
+ return TRUE;
+}
+
+static void mpd_glibStartIdle(mpd_Connection *connection)
+{
Re: [Musicpd-dev-team] [PATCH] idle command enhancement
Hi,
- It adds the possibility to choose which notification you want to
receive.
The idle command can now take no argument (same behavior as before) or
several arguments.
For example if a client uses command 'idle mixer elapsed', it will only
receive notifications when the volume changed or when the elapsed time
changed.
Good idea.
Someone proposed that idle 10 would make MPD respond after 10
seconds, disregarding whether an event has really happened, and
probably returns with an empty event set.
Can you make a separate patch of that? And maybe implement the
timeout thing if you have time...
I attach a separate patch. What would be the advantage of this timeout
thing? I don't see in which use case it can be useful.
- And last, with this patch MPD doesn't leave idle mode anymore when a
notification has been done. It means that idle mode will continue until
'noidle' command is sent. I think this will easy client implementation of
idle command and reduce a lot the amount of useless traffic. Please tell
me
if you see a major drawback with this behavior.
No, I think the disadvantages of this solution will prevail here.
Events aren't that often (unless we really implement your 'elapsed'
event), and after most events, the client has to query the server
anyway.
Could you list these disadvantages? In fact I don't see any
flags = ~0;
Thanks, it's much better like this ;-)
Marc
diff --git a/src/client.c b/src/client.c
index ee98464..fd55b31 100644
--- a/src/client.c
+++ b/src/client.c
@@ -91,6 +91,9 @@ struct client {
/** idle flags pending on this client, to be sent as soon as
the client enters idle */
unsigned idle_flags;
+
+ /** idle flags that the client wants to receive */
+ unsigned idle_subscriptions;
};
static LIST_HEAD(clients);
@@ -766,16 +769,6 @@ mpd_fprintf void client_printf(struct client *client, const char *fmt, ...)
va_end(args);
}
-static const char *const idle_names[] = {
- database,
- stored_playlist,
- playlist,
- player,
- mixer,
- output,
- options,
-};
-
/**
* Send idle response to this client.
*/
@@ -783,6 +776,7 @@ static void
client_idle_notify(struct client *client)
{
unsigned flags, i;
+ const char *const* idle_names;
assert(client-idle_waiting);
assert(client-idle_flags != 0);
@@ -791,10 +785,9 @@ client_idle_notify(struct client *client)
client-idle_flags = 0;
client-idle_waiting = false;
- for (i = 0; i sizeof(idle_names) / sizeof(idle_names[0]); ++i) {
- assert(idle_names[i] != NULL);
-
- if (flags (1 i))
+ idle_names = idle_get_names();
+ for (i = 0; idle_names[i]; ++i) {
+ if (flags (1 i) client-idle_subscriptions)
client_printf(client, changed: %s\n,
idle_names[i]);
}
@@ -814,20 +807,22 @@ void client_manager_idle_add(unsigned flags)
continue;
client-idle_flags |= flags;
- if (client-idle_waiting) {
+ if (client-idle_waiting
+ (client-idle_flags client-idle_subscriptions)) {
client_idle_notify(client);
client_write_output(client);
}
}
}
-bool client_idle_wait(struct client *client)
+bool client_idle_wait(struct client *client, unsigned flags)
{
assert(!client-idle_waiting);
client-idle_waiting = true;
+ client-idle_subscriptions = flags;
- if (client-idle_flags != 0) {
+ if (client-idle_flags client-idle_subscriptions) {
client_idle_notify(client);
return true;
} else
diff --git a/src/client.h b/src/client.h
index e6661e2..9c70318 100644
--- a/src/client.h
+++ b/src/client.h
@@ -78,6 +78,6 @@ void client_manager_idle_add(unsigned flags);
* sent immediately and true is returned. If no, it puts the
* client into waiting mode and returns false.
*/
-bool client_idle_wait(struct client *client);
+bool client_idle_wait(struct client *client, unsigned flags);
#endif
diff --git a/src/command.c b/src/command.c
index dc71f1d..6a9489d 100644
--- a/src/command.c
+++ b/src/command.c
@@ -39,6 +39,7 @@
#include tag_print.h
#include path.h
#include os_compat.h
+#include idle.h
#define COMMAND_STATUS_VOLUME volume
#define COMMAND_STATUS_STATEstate
@@ -1253,8 +1254,28 @@ static enum command_return
handle_idle(struct client *client,
mpd_unused int argc, mpd_unused char *argv[])
{
+unsigned flags = 0, j;
+int i;
+const char *const* idle_names;
+
+idle_names = idle_get_names();
+for (i = 1; i argc; ++i) {
+if (!argv[i])
+continue;
+
+for (j = 0; idle_names[j]; ++j) {
+if (!strcasecmp(argv[i], idle_names[j])) {
+flags |= (1 j);
+}
+}
+}
+
+/* No argument means that the client wants to receive everything */
+if (flags == 0)
+flags = ~0;
+
/* enable idle mode on this client */
- client_idle_wait(client);
+ client_idle_wait(client, flags);
/* return value
Re: [Musicpd-dev-team] [PATCH] idle command enhancement
Hi,
This new patch replaces the previous one. It adds the possibility to send
the new value in a notification.
For example, if the volume changes to 85, MPD sends 'changed: mixer 85'.
Marc
2008/11/16 Marc Pavot [EMAIL PROTECTED]
Hi,
I propose you a patch that improves different aspects of idle command:
- It adds a new 'elapsed' event to notify clients when the elapsed time has
changed (in absolute value in second)
- It adds the possibility to choose which notification you want to receive.
The idle command can now take no argument (same behavior as before) or
several arguments.
For example if a client uses command 'idle mixer elapsed', it will only
receive notifications when the volume changed or when the elapsed time
changed.
- And last, with this patch MPD doesn't leave idle mode anymore when a
notification has been done. It means that idle mode will continue until
'noidle' command is sent. I think this will easy client implementation of
idle command and reduce a lot the amount of useless traffic. Please tell me
if you see a major drawback with this behavior.
Marc
diff --git a/src/client.c b/src/client.c
index ee98464..34610eb 100644
--- a/src/client.c
+++ b/src/client.c
@@ -91,6 +91,9 @@ struct client {
/** idle flags pending on this client, to be sent as soon as
the client enters idle */
unsigned idle_flags;
+
+ /** idle flags that the client wants to receive */
+ unsigned idle_subscriptions;
};
static LIST_HEAD(clients);
@@ -766,16 +769,6 @@ mpd_fprintf void client_printf(struct client *client, const char *fmt, ...)
va_end(args);
}
-static const char *const idle_names[] = {
- database,
- stored_playlist,
- playlist,
- player,
- mixer,
- output,
- options,
-};
-
/**
* Send idle response to this client.
*/
@@ -783,20 +776,26 @@ static void
client_idle_notify(struct client *client)
{
unsigned flags, i;
+const char *const* idle_names;
+char ** idle_values;
assert(client-idle_waiting);
assert(client-idle_flags != 0);
flags = client-idle_flags;
client-idle_flags = 0;
- client-idle_waiting = false;
-
- for (i = 0; i sizeof(idle_names) / sizeof(idle_names[0]); ++i) {
- assert(idle_names[i] != NULL);
- if (flags (1 i))
- client_printf(client, changed: %s\n,
- idle_names[i]);
+idle_names = idle_get_names();
+idle_values = idle_get_values();
+ for (i = 0; idle_names[i]; ++i) {
+ if (flags (1 i) client-idle_subscriptions) {
+if (idle_values[i])
+ client_printf(client, changed: %s %s\n,
+ idle_names[i], idle_values[i]);
+else
+ client_printf(client, changed: %s\n,
+ idle_names[i]);
+}
}
client_puts(client, OK\n);
@@ -821,11 +820,13 @@ void client_manager_idle_add(unsigned flags)
}
}
-bool client_idle_wait(struct client *client)
+bool client_idle_wait(struct client *client,
+ unsigned flags)
{
assert(!client-idle_waiting);
client-idle_waiting = true;
+ client-idle_subscriptions = flags;
if (client-idle_flags != 0) {
client_idle_notify(client);
diff --git a/src/client.h b/src/client.h
index e6661e2..9c70318 100644
--- a/src/client.h
+++ b/src/client.h
@@ -78,6 +78,6 @@ void client_manager_idle_add(unsigned flags);
* sent immediately and true is returned. If no, it puts the
* client into waiting mode and returns false.
*/
-bool client_idle_wait(struct client *client);
+bool client_idle_wait(struct client *client, unsigned flags);
#endif
diff --git a/src/command.c b/src/command.c
index dc71f1d..7a7cdfb 100644
--- a/src/command.c
+++ b/src/command.c
@@ -39,6 +39,7 @@
#include tag_print.h
#include path.h
#include os_compat.h
+#include idle.h
#define COMMAND_STATUS_VOLUME volume
#define COMMAND_STATUS_STATEstate
@@ -1253,8 +1254,31 @@ static enum command_return
handle_idle(struct client *client,
mpd_unused int argc, mpd_unused char *argv[])
{
+unsigned flags = 0, j;
+int i;
+const char *const* idle_names;
+
+idle_names = idle_get_names();
+for (i = 1; i argc; ++i) {
+if (!argv[i])
+continue;
+
+for (j = 0; idle_names[j]; ++j) {
+if (!strcasecmp(argv[i], idle_names[j])) {
+flags |= (1 j);
+}
+}
+}
+
+/* No argument means that the client wants to receive everything */
+if (flags == 0) {
+for (j = 0; idle_names[j]; ++j) {
+flags |= (1 j);
+}
+}
+
/* enable idle mode on this client */
- client_idle_wait(client);
+ client_idle_wait(client, flags);
/* return value is 1 so the caller won't print OK */
return 1;
@@ -1280,7 +1304,7 @@ static const struct command commands[] = {
{ disableoutput, PERMISSION_ADMIN, 1, 1
[Musicpd-dev-team] [PATCH] idle command enhancement
Hi,
I propose you a patch that improves different aspects of idle command:
- It adds a new 'elapsed' event to notify clients when the elapsed time has
changed (in absolute value in second)
- It adds the possibility to choose which notification you want to receive.
The idle command can now take no argument (same behavior as before) or
several arguments.
For example if a client uses command 'idle mixer elapsed', it will only
receive notifications when the volume changed or when the elapsed time
changed.
- And last, with this patch MPD doesn't leave idle mode anymore when a
notification has been done. It means that idle mode will continue until
'noidle' command is sent. I think this will easy client implementation of
idle command and reduce a lot the amount of useless traffic. Please tell me
if you see a major drawback with this behavior.
Marc
diff --git a/src/client.c b/src/client.c
index ee98464..224dfff 100644
--- a/src/client.c
+++ b/src/client.c
@@ -91,6 +91,9 @@ struct client {
/** idle flags pending on this client, to be sent as soon as
the client enters idle */
unsigned idle_flags;
+
+ /** idle flags that the client wants to receive */
+ unsigned idle_subscriptions;
};
static LIST_HEAD(clients);
@@ -766,16 +769,6 @@ mpd_fprintf void client_printf(struct client *client, const char *fmt, ...)
va_end(args);
}
-static const char *const idle_names[] = {
- database,
- stored_playlist,
- playlist,
- player,
- mixer,
- output,
- options,
-};
-
/**
* Send idle response to this client.
*/
@@ -783,23 +776,21 @@ static void
client_idle_notify(struct client *client)
{
unsigned flags, i;
+const char *const* idle_names;
assert(client-idle_waiting);
assert(client-idle_flags != 0);
flags = client-idle_flags;
client-idle_flags = 0;
- client-idle_waiting = false;
-
- for (i = 0; i sizeof(idle_names) / sizeof(idle_names[0]); ++i) {
- assert(idle_names[i] != NULL);
- if (flags (1 i))
+idle_names = idle_get_names();
+ for (i = 0; idle_names[i]; ++i) {
+ if (flags (1 i) client-idle_subscriptions)
client_printf(client, changed: %s\n,
idle_names[i]);
}
- client_puts(client, OK\n);
client-lastTime = time(NULL);
}
@@ -821,11 +812,13 @@ void client_manager_idle_add(unsigned flags)
}
}
-bool client_idle_wait(struct client *client)
+bool client_idle_wait(struct client *client,
+ unsigned flags)
{
assert(!client-idle_waiting);
client-idle_waiting = true;
+ client-idle_subscriptions = flags;
if (client-idle_flags != 0) {
client_idle_notify(client);
diff --git a/src/client.h b/src/client.h
index e6661e2..9c70318 100644
--- a/src/client.h
+++ b/src/client.h
@@ -78,6 +78,6 @@ void client_manager_idle_add(unsigned flags);
* sent immediately and true is returned. If no, it puts the
* client into waiting mode and returns false.
*/
-bool client_idle_wait(struct client *client);
+bool client_idle_wait(struct client *client, unsigned flags);
#endif
diff --git a/src/command.c b/src/command.c
index dc71f1d..7a7cdfb 100644
--- a/src/command.c
+++ b/src/command.c
@@ -39,6 +39,7 @@
#include tag_print.h
#include path.h
#include os_compat.h
+#include idle.h
#define COMMAND_STATUS_VOLUME volume
#define COMMAND_STATUS_STATEstate
@@ -1253,8 +1254,31 @@ static enum command_return
handle_idle(struct client *client,
mpd_unused int argc, mpd_unused char *argv[])
{
+unsigned flags = 0, j;
+int i;
+const char *const* idle_names;
+
+idle_names = idle_get_names();
+for (i = 1; i argc; ++i) {
+if (!argv[i])
+continue;
+
+for (j = 0; idle_names[j]; ++j) {
+if (!strcasecmp(argv[i], idle_names[j])) {
+flags |= (1 j);
+}
+}
+}
+
+/* No argument means that the client wants to receive everything */
+if (flags == 0) {
+for (j = 0; idle_names[j]; ++j) {
+flags |= (1 j);
+}
+}
+
/* enable idle mode on this client */
- client_idle_wait(client);
+ client_idle_wait(client, flags);
/* return value is 1 so the caller won't print OK */
return 1;
@@ -1280,7 +1304,7 @@ static const struct command commands[] = {
{ disableoutput, PERMISSION_ADMIN, 1, 1, handle_disableoutput },
{ enableoutput, PERMISSION_ADMIN, 1, 1, handle_enableoutput },
{ find, PERMISSION_READ, 2, -1, handle_find },
- { idle, PERMISSION_READ, 0, 0, handle_idle },
+ { idle, PERMISSION_READ, 0, -1, handle_idle },
{ kill, PERMISSION_ADMIN, -1, -1, handle_kill },
{ list, PERMISSION_READ, 1, -1, handle_list },
{ listall, PERMISSION_READ, 0, 1, handle_listall },
diff --git a/src/idle.c b/src/idle.c
index c779d0a..477a676 100644
--- a/src/idle.c
+++ b/src/idle.c
@@ -30,6 +30,24 @@
static unsigned idle_flags;
static pthread_mutex_t
Re: [Musicpd-dev-team] idle command
The two sockets solution is easy to implement and interesting but as Max sayed it soncumes precious ressources. We may have another solution (used for example by XMMS2): - Each query has an identifier - Each reply has the identifier corresponding to the query - Each subscription to an event notification also has an identifier - Each notification has the identifier corresponding to the subscription This means some important changes in MPD protocol but it has several advantages: - It uses only one socket - It is easy to implement on client side (you just have to keep a list of identifier to understand what you receive) - It enables you to implement synchronous and asynchronous calls to MPD on client side which is not an easy thing with current protocol. - It will be easy to implement a publish-subscribe solution for event notifications Marc 2008/11/14 Max Kellermann [EMAIL PROTECTED] On 2008/11/14 11:15, Roeland Douma [EMAIL PROTECTED] wrote: Other than that I think an important event is missing. The event that notifies the user of the place the song is. Since If I start a client I want to know that. This can be very small messages. But they can be very use full. You mean which song is currently being played? That's the event player. Documentation incomplete again. This however would be better done with publish subscribe. Since some clients might or might not be interested. With the current implementation publish subscribe should just be which events you would like to receive and which not. Which is ore or less the general concept of publish subscribe of course. I have an example for you. Lets say I have a mobile MPD client on my phone. I want to display the current song, the next song. And some controls. Then I a not interested in any other events. See my previous response to Marc in this thread: On 2008/11/14 09:01, Max Kellermann [EMAIL PROTECTED] wrote: On 2008/11/13 21:45, Marc Pavot [EMAIL PROTECTED] wrote: - you cannot subscribe to receive only some of the notifications Sounds like a reasonable feature request. What about idle mixer options to only receive mixer and options events? Other ideas? Max - This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100url=/ ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team - This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100url=/___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] idle command
Hi Why not? The client can let the connection block until something happens, or it can abort idle as soon as he wishes to use the connection for something else. That is simple, but powerful. As already proposed by someone else, a publish-subscribe protocol would be much more appropriate because with the 'idle' approach: - you have to send 'noidle' command each time you want to send another command - you have to send 'idle' again after this command - you have to send 'idle' again each time a notification has been done - you cannot subscribe to receive only some of the notifications I could really need a helping hand on libmpdclient. Are you familiar with asynchronous I/O? I am not an expert of asynchronous I/O but it may be time to become one :-). Marc - This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100url=/___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
