- 12 Apr, 2008 17 commits
-
-
Eric Wong authored
It is way more complicated than it should be; and locking it for thread-safety is too difficult. [merged r7183 from branches/ew] git-svn-id: https://svn.musicpd.org/mpd/trunk@7241 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Eric Wong authored
I initially started to do a heavy rewrite that changed the way processes communicated, but that was too much to do at once. So this change only focuses on replacing the player and decode processes with threads and using condition variables instead of polling in loops; so the changeset itself is quiet small. * The shared output buffer variables will still need locking to guard against race conditions. So in this effect, we're probably just as buggy as before. The reduced context-switching overhead of using threads instead of processes may even make bugs show up more or less often... * Basic functionality appears to be working for playing local (and NFS) audio, including: play, pause, stop, seek, previous, next, and main playlist editing * I haven't tested HTTP streams yet, they should work. * I've only tested ALSA and Icecast. ALSA works fine, Icecast metadata seems to get screwy at times and breaks song advancement in the playlist at times. * state file loading works, too (after some last-minute hacks with non-blocking wakeup functions) * The non-blocking (*_nb) variants of the task management functions are probably overused. They're more lenient and easier to use because much of our code is still based on our previous polling-based system. * It currently segfaults on exit. I haven't paid much attention to the exit/signal-handling routines other than ensuring it compiles. At least the state file seems to work. We don't do any cleanups of the threads on exit, yet. * Update is still done in a child process and not in a thread. To do this in a thread, we'll need to ensure it does proper locking and communication with the main thread; but should require less memory in the end because we'll be updating the database "in-place" rather than updating a copy and then bulk-loading when done. * We're more sensitive to bugs in 3rd party libraries now. My plan is to eventually use a master process which forks() and restarts the child when it dies: locking and communication with the main thread; but should require less memory in the end because we'll be updating the database "in-place" rather than updating a copy and then bulk-loading when done. * We're more sensitive to bugs in 3rd party libraries now. My plan is to eventually use a master process which forks() and restarts the child when it dies: master - just does waitpid() + fork() in a loop \- main thread \- decoder thread \- player thread At the beginning of every song, the main thread will set a dirty flag and update the state file. This way, if we encounter a song that triggers a segfault killing the main thread, the master will start the replacement main on the next song. * The main thread still wakes up every second on select() to check for signals; which affects power management. [merged r7138 from branches/ew] git-svn-id: https://svn.musicpd.org/mpd/trunk@7240 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Max Kellermann authored
Clean up some space indentations, replace with tabs. git-svn-id: https://svn.musicpd.org/mpd/trunk@7239 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Max Kellermann authored
git-svn-id: https://svn.musicpd.org/mpd/trunk@7238 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Max Kellermann authored
[ew: rewrote commit message as it was confusing] git-svn-id: https://svn.musicpd.org/mpd/trunk@7237 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Max Kellermann authored
autoconf flags for enabling and disabling TCP and unix domain socket support. Embedded machines without a TCP stack may be better off without TCP support. git-svn-id: https://svn.musicpd.org/mpd/trunk@7236 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Max Kellermann authored
This trivial patch addresses bug 1639. When a bind_to_address argument starts with a slash, assume that it is the address of a Unix domain socket. git-svn-id: https://svn.musicpd.org/mpd/trunk@7235 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Max Kellermann authored
It is a good practice to constify pointers when their dereferenced data is not modified within the functions or its descendants. git-svn-id: https://svn.musicpd.org/mpd/trunk@7234 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Max Kellermann authored
The parameter "port" is not actually used by establishListen(), and can be removed. This also allows establishListen() to be used for socket addresses which have no port. git-svn-id: https://svn.musicpd.org/mpd/trunk@7233 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Max Kellermann authored
The number of buffered chunks can obviously not become negative. The "buffered_before_play<0" therefore cannot be useful, so let's remove it, too. git-svn-id: https://svn.musicpd.org/mpd/trunk@7232 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Max Kellermann authored
The chunk index cannot be negative. git-svn-id: https://svn.musicpd.org/mpd/trunk@7231 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Max Kellermann authored
Do explicit casts before comparing signed with unsigned. The one in log.c actually fixes another warning: in the expanded macro, there may be a check "logLevel>=0", which is always true. git-svn-id: https://svn.musicpd.org/mpd/trunk@7230 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Max Kellermann authored
Unfortunately, the function iconv() wants a non-const input buffer. In this context, we only have a const pointer, which emits a correct gcc warning. Work around this ugliness with an union-deconst hack. This is optimized away in the binary. git-svn-id: https://svn.musicpd.org/mpd/trunk@7229 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Max Kellermann authored
The counter variables c_samp and c_chan begin at zero and can never be negative. git-svn-id: https://svn.musicpd.org/mpd/trunk@7228 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Max Kellermann authored
The local variable d_samp is initialized, but never actually used. git-svn-id: https://svn.musicpd.org/mpd/trunk@7227 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Max Kellermann authored
Use unsigned integers in decoderParent() for chunk numbers which cannot be negative. git-svn-id: https://svn.musicpd.org/mpd/trunk@7226 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Max Kellermann authored
enable -Wextra (excluding -Wno-unused-parameter, -Wno-deprecated-declarations). git-svn-id: https://svn.musicpd.org/mpd/trunk@7225 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
- 30 Mar, 2008 1 commit
-
-
Eric Wong authored
Thanks to Jérome Perrin git-svn-id: https://svn.musicpd.org/mpd/trunk@7224 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
- 27 Mar, 2008 1 commit
-
-
Eric Wong authored
We don't really care what that variable is, so it might as well be uninitialized, but valgrind does... git-svn-id: https://svn.musicpd.org/mpd/trunk@7220 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
- 26 Mar, 2008 21 commits
-
-
Eric Wong authored
Don't bother initializing the junk buffer, we really don't care. The array was also unnecessary and ugly. Also, avoid returning the byte count we read/wrote since it unnecessarily exposes internal details of the implementation to the callers. git-svn-id: https://svn.musicpd.org/mpd/trunk@7219 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Eric Wong authored
git-svn-id: https://svn.musicpd.org/mpd/trunk@7218 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Eric Wong authored
* move set_nonblock{,ing}() into utils.c since we use it elsewhere, too * add proper error checking to set_nonblocking() * use os_compat.h instead of individually #includ-ing system headers git-svn-id: https://svn.musicpd.org/mpd/trunk@7217 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Max Kellermann authored
When the decoder receives SIGCONT during waitNotify(), the kernel restarts the read() system call. This lets the decoder process block indefinitely, while the player process waits for it to react. This should probably be solved with a proper signal handler which aborts the read() system call, but for now, we just write to the pipe to make it wake up. git-svn-id: https://svn.musicpd.org/mpd/trunk@7216 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Max Kellermann authored
When the decoder process is faster than the player process, all decodedd buffers are full at some point in time. The decoder has to wait for buffers to become free (finished playing). It used to do this by polling the buffer status 100 times a second. This generates a lot of unnecessary CPU wakeups. This patch adds a way for the player process to notify the decoder process that it may continue its work. We could use pthread_cond for that, unfortunately inter-process mutexes/conds are not supported by some kernels (Linux), so we cannot use this light-weight method until mpd moves to using threads instead of processes. The other method would be semaphores, which historically are global resources with a unique name; this historic API is cumbersome, and I wanted to avoid it. I came up with a quite naive solution for now: I create an anonymous pipe with pipe(), and the decoder process reads on that pipe. Until the player process sends data on it as a signal, the decoder process blocks. This can be optimized in a number of ways: - if the decoder process is still working (instead of waiting for buffers), we could save the write() system call, since there is nobody waiting for the notification. [ew: I tried this using a counter in shared memory, didn't help] - the pipe buffer will be full at some point, when the decoder thread is too slow. For this reason, the writer side of the pipe is non-blocking, and mpd can ignore the resulting EWOULDBLOCK. - since we have shared memory, we could check whether somebody is actually waiting without a context switch, and we could just not write the notification byte. [ew: tried same method/result as first point above] - if there is already a notification in the pipe, we could also not write another one. [ew: tried same method/result as first/third points above] - the decoder will only consume 64 bytes at a time. If the pipe buffer is full, this will result in a lot of read() invocations. This does not hurt badly, but on a heavily loaded system, this might add a little bit more load. The preceding optimizations however are able eliminate the this. - finally, we should use another method for inter process notifications - maybe kill() or just make mpd use threads, finally. In spite of all these possibilities to optimize this code further, this pipe notification trick is faster than the 100 Hz poll. On my machine, it reduced the number of wakeups to less than 30%. git-svn-id: https://svn.musicpd.org/mpd/trunk@7215 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Max Kellermann authored
Use unsigned variables for storing the count of items or for iteration variables. Since there can never be a negative number of items, it makes sense to use an unsigned data type here. This change is safe because the unsigned values are only used for adddressing array items. git-svn-id: https://svn.musicpd.org/mpd/trunk@7214 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Max Kellermann authored
The interfaces main loop repeats the select() (non-blocking) after an event was handled. I do not see any reason for that, since all events should be handled after the first select(). This double select() does nothing than consume more CPU cycles. git-svn-id: https://svn.musicpd.org/mpd/trunk@7213 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Max Kellermann authored
mpd sets a 1s select() timeout for no reason. This makes mpd wake up the CPU, consume some cycles just to see there is nothing to do. We can save that by specifying NULL instead of a timeout. git-svn-id: https://svn.musicpd.org/mpd/trunk@7212 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Eric Wong authored
* malloc() => xmalloc() for error checking * strncpy() replaced with memcpy(), memcpy appears perfectly safe here and mpd does not ever use strncpy() (see r4491) git-svn-id: https://svn.musicpd.org/mpd/trunk@7211 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Laszlo Ashin authored
This patch does the following: -enables WVC support for streams as well, -improves MPD inputStream <=> WavPack stream connector, -fixes two compile warnings (which were caused by MPD API change). Mantis #1660 <http://musicpd.org/mantis/view.php?id=1660> git-svn-id: https://svn.musicpd.org/mpd/trunk@7210 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Eric Wong authored
Basically, I don't trust myself nor Max to not have bugs in our code when switching over to unsigned types, so I've added more assertions which will hopefully trip and force us to fix these bugs before somebody can exploit them :) Some cleanups for parameter parsing using strtol and error reporting to the user. Also, fix some completely garbled indentation in inputStream_http.c git-svn-id: https://svn.musicpd.org/mpd/trunk@7209 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Eric Wong authored
git-svn-id: https://svn.musicpd.org/mpd/trunk@7208 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Eric Wong authored
Terminals are 80 columns and that's a hard limit, no exceptions git-svn-id: https://svn.musicpd.org/mpd/trunk@7207 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Max Kellermann authored
This patch moves code which initializes the OutputBuffer struct to outputBuffer.c. Although this is generally a good idea, it prepares the following patch. git-svn-id: https://svn.musicpd.org/mpd/trunk@7206 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Max Kellermann authored
When dealing with in-memory lengths, the standard type "size_t" should be used. Missing one can be quite dangerous, because an attacker could provoke an integer under-/overflow, which may provide an attack vector. git-svn-id: https://svn.musicpd.org/mpd/trunk@7205 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Max Kellermann authored
Just one more assertion. There should be more of that! git-svn-id: https://svn.musicpd.org/mpd/trunk@7204 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Max Kellermann authored
we do not save anything by limiting a variable to an unsigned char, since the compiler aligns it at machine word size anyway. however by using the full machine word, we save one instruction, and we remove the useless artificial limitation to 255. git-svn-id: https://svn.musicpd.org/mpd/trunk@7203 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Max Kellermann authored
The local variable eof can actually be replaced with a simple "break". With a negative ret, the value of chunkpos can be invalidated, I am not sure if this might have been a bug. [ew: no, a negative ret will correspond to ret == OV_HOLE and ret will be reset to zero leaving chunkpos untouched (code cleaned up to make this more obvious] git-svn-id: https://svn.musicpd.org/mpd/trunk@7202 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Max Kellermann authored
The database parser does not check whether the song object has been initialized yet, which may lead to a NULL pointer dereference. Add this check. git-svn-id: https://svn.musicpd.org/mpd/trunk@7201 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Max Kellermann authored
strtok() may return NULL if the input is an empty string. The playlist parser did not check for that. git-svn-id: https://svn.musicpd.org/mpd/trunk@7200 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-
Max Kellermann authored
Local variables which are never read before the first assignment don't need initialization. Saves a few bytes of text. Also don't reset variables which are never read until function return. git-svn-id: https://svn.musicpd.org/mpd/trunk@7199 09075e82-0dd4-0310-85a5-a0d7c8717e4f
-