This is the first patch in a series which removes the shared memory,
and moves all the playerData objects into the normal libc heap.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7304 09075e82-0dd4-0310-85a5-a0d7c8717e4f
Now that we do proper locking and signalling instead of continuous
polling for IPC, a deadlock was found: at the end of a song, the
player thread waits until the main thread sets pc->queueState from
PLAYER_QUEUE_DECODE to PLAYER_QUEUE_PLAY. He is never woken up, since
syncPlaylistWithQueue() does not activate the notification. I added
wakeup_player_nb() to setQueueState(), since the player must be
signalled whenever pc->queueState changes.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7303 09075e82-0dd4-0310-85a5-a0d7c8717e4f
Buffer sizes should be size_t. This is safe here, at least not
unsafer than without the patch. I have no idea why audioBufferSize
and audioBufferPos were explicitly declared as signed integer.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7296 09075e82-0dd4-0310-85a5-a0d7c8717e4f
When growing the audioOutput->convBuffer, we can use free()+malloc()
instead of realloc(), which saves a memcpy().
git-svn-id: https://svn.musicpd.org/mpd/trunk@7295 09075e82-0dd4-0310-85a5-a0d7c8717e4f
The audio output plugins should get a constant pointer, because they
must not modify the buffer. Since the size is a non-negative buffer
size in bytes, we should change its type to size_t.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7293 09075e82-0dd4-0310-85a5-a0d7c8717e4f
In my previous patch set, I forgot to change the
pcm_sizeOfConvBuffer() invocation in convertAudioFormat() to also use
size_t.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7292 09075e82-0dd4-0310-85a5-a0d7c8717e4f
calculateCrossFadeChunks() still returns int, although the caller uses
it as an unsigned value. Since the function body checks for negative
values, it is safe to cast to unsigned.
crossFade() takes signed parameters, although it callers pass unsigned
integers. Change declaration to unsigned.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7291 09075e82-0dd4-0310-85a5-a0d7c8717e4f
There were some const pointers missing in the previous const-cleanup
patch.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7290 09075e82-0dd4-0310-85a5-a0d7c8717e4f
libfaad wants uint32_t pointers. Passing a long pointer is bugged on
amd64.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7289 09075e82-0dd4-0310-85a5-a0d7c8717e4f
The patch "Start using song pointers in core data structures" removed
dc->utf8url, and the adaption for wavpack_plugin.c was missing.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7288 09075e82-0dd4-0310-85a5-a0d7c8717e4f
The new function successor() can be used to simplify a lot of code
lines and saves a lot of "i+>=buffered_chunks" checks.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7285 09075e82-0dd4-0310-85a5-a0d7c8717e4f
currentChunk is a global variable, which renders the whole output
buffer code non-reentrant. Although this is not a real problem since
there is only one global output buffer currently, we should move it to
the OutputBuffer struct.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7284 09075e82-0dd4-0310-85a5-a0d7c8717e4f
This patch rewrites notify.c to use the pthread API, namely
pthread_mutex and pthread_cond. This is a lot cheaper and easier than
the pipe() hack.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7280 09075e82-0dd4-0310-85a5-a0d7c8717e4f
The code paths which return from the functions all have to call
quitDecode(). If we simply break instead of calling quitDecode()
explicitly, this function gets called in the last line of this
function anyway.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7278 09075e82-0dd4-0310-85a5-a0d7c8717e4f
The multi-line expression which calculates sizeToTime is hard to read,
partly because "cb->audioFormat." is too long. Create a separate
inline function in audio.h for that.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7277 09075e82-0dd4-0310-85a5-a0d7c8717e4f
Moved the decision whether to cross-fade the current song to
calculateCrossFadeChunks(). This simplifies the function
decoderParent() and eliminates one layer of indentation.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7276 09075e82-0dd4-0310-85a5-a0d7c8717e4f
Similar to the crossFade() patch: pass chunk objects to playChunk(),
simplify decodeParent() by removing clutter.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7275 09075e82-0dd4-0310-85a5-a0d7c8717e4f
To unify the decoderParent() main loop some more, use it to wait for
the decoder to change the song. Only one single processDecodeInput()
caller left after this patch.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7273 09075e82-0dd4-0310-85a5-a0d7c8717e4f
When there are not enough decode cross-fade chunks in the buffer yet,
the current code does busy-wait, which will delay the decoder even
more. sleep instead, expecting the decoder to wake us up.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7272 09075e82-0dd4-0310-85a5-a0d7c8717e4f
Calling crossFade() with the chunk objects is easier than unrolling
all the chunk properties manually, and decodeParent() can really use
more of these simplifications.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7271 09075e82-0dd4-0310-85a5-a0d7c8717e4f
sizeof() is the more "natural" or "direct" access to the buffer size,
instead of a macro happening to be used to the buffer declaration.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7270 09075e82-0dd4-0310-85a5-a0d7c8717e4f
To make access to OutputBuffer easier, move everything which belongs
to a chunk into its own structure, namely OutputBufferChunk.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7269 09075e82-0dd4-0310-85a5-a0d7c8717e4f
The chunk size should be in outputBuffer.h since the output buffer
code is its primary user.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7268 09075e82-0dd4-0310-85a5-a0d7c8717e4f
Hiding OutputBuffer internals, again. We get an extra assertion in
return.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7267 09075e82-0dd4-0310-85a5-a0d7c8717e4f
The variable "quit" can be removed, since its only setter can use
"break" instead, just like the other code paths.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7266 09075e82-0dd4-0310-85a5-a0d7c8717e4f
The variables "nextChunk" and "crossFadeChunks" are only used when
doCrossFade==1. This means that we do not have to reset these as long
as doCrossFade!=1.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7265 09075e82-0dd4-0310-85a5-a0d7c8717e4f
The cross-fade check is still very complicated whenever it uses
OutputBuffer internals. Greatly simplify another check by introducing
outputBufferRelative().
git-svn-id: https://svn.musicpd.org/mpd/trunk@7264 09075e82-0dd4-0310-85a5-a0d7c8717e4f
Another "don't use OutputBuffer internals" patch. This ignores the
copied "end" value, but I do not think that has ever been a real
issue.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7263 09075e82-0dd4-0310-85a5-a0d7c8717e4f
decoderParent() uses a lot of OutputBuffer internals to see whether
cross-fading should be started. Move these checks to outputBuffer.c,
which also simplifies decoderParent().
git-svn-id: https://svn.musicpd.org/mpd/trunk@7262 09075e82-0dd4-0310-85a5-a0d7c8717e4f
Declare the variables "test" and "fadePosition" in the scope where
they are really used. This removes some of the clutter in the
function decodeParent().
git-svn-id: https://svn.musicpd.org/mpd/trunk@7261 09075e82-0dd4-0310-85a5-a0d7c8717e4f
Eliminating some duplicated and. This also decreases the number of
lines calling processDecodeInput().
git-svn-id: https://svn.musicpd.org/mpd/trunk@7260 09075e82-0dd4-0310-85a5-a0d7c8717e4f
I have spent some time to understand decodeParent(), which does a lot
of obfuscated magic... I find it useful to help others to also
understand it, so I wrote a few comments.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7259 09075e82-0dd4-0310-85a5-a0d7c8717e4f
Another patch indended to improve the CPP macro hell. This enlarges
the function decodeParent(), but it cannot be converted into a
standalone function easily, because it references so many local
variables.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7258 09075e82-0dd4-0310-85a5-a0d7c8717e4f
clearOutputBuffer() also resets currentChunk; this might resolve a
theoretical bug.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7257 09075e82-0dd4-0310-85a5-a0d7c8717e4f
dc->start cannot be true after the loop, because it was the loop
condition. dc->start could have been set by another thread between
the while loop and the if, but I suspect this is not the case the
author intended, so we just remove the dc->start check.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7256 09075e82-0dd4-0310-85a5-a0d7c8717e4f
In my opinion, the code becomes more readable when we explicitly check
"==NOERROR" instead of an implicit 0 check.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7255 09075e82-0dd4-0310-85a5-a0d7c8717e4f
Macros are ugly, and multi-line macros are even more ugly. This patch
converts processDecodeInput() to a C function. The disadvantage may
be that the function does not have access to the caller's local
variables, which might be regarded as an advantage on the other hand.
For this reason, we have to pass variable references. This costs a
tiny bit of performance, but it's worth eliminating this monster
macro, and further patches will optimize this cost down.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7254 09075e82-0dd4-0310-85a5-a0d7c8717e4f
since clearPlayerQueue() is always called within
lockPlaylistInteraction() / unlockPlaylistInteraction(), it simplifies
the code to add another function which calls these three functions.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7253 09075e82-0dd4-0310-85a5-a0d7c8717e4f
The block after "if" breaks out of the loop. To make the code a
little bit more readable, don't write the rest in an "else" block,
since this code path does not break.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7251 09075e82-0dd4-0310-85a5-a0d7c8717e4f
The method availableOutputBuffer() calculates how many chunks are in
use. This simplifies code which needs this information, and it can
run without knowing OutputBuffer internals. The function knows how to
calculate this when begin>end; this might have been a bug in
decodeParent(), which does not.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7250 09075e82-0dd4-0310-85a5-a0d7c8717e4f
After the previous patch, it is clear that the loop in
advanceOutputBufferTo() can be replaced with a simple assignment.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7249 09075e82-0dd4-0310-85a5-a0d7c8717e4f
The parameter "currentChunkSent" is not used and can be dropped.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7248 09075e82-0dd4-0310-85a5-a0d7c8717e4f
This patch removes some clutter from decodeParent() by moving some
code out.
git-svn-id: https://svn.musicpd.org/mpd/trunk@7247 09075e82-0dd4-0310-85a5-a0d7c8717e4f
realloc() has to copy data to the new buffer. Since convBuffer
contains temporary data only, we can safely use free() plus a new
malloc(), which saves the mempy().
git-svn-id: https://svn.musicpd.org/mpd/trunk@7246 09075e82-0dd4-0310-85a5-a0d7c8717e4f
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
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
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
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
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
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
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
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
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
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
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
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
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
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
* 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
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
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
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
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
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
* 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
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
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
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
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
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
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
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
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
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
