When we reset pc.next_song if there is no song queued, this might
cause a race condition: the next song to be played is cleared, while
pc.command was already set. Clear the "next_song" only if there is a
song queued for the current do_play() invocation.
If a new song is queued before calling playerSeek(), then the player
and the playlist enter an inconsistent state, because the player
discards the playlist's "queued" song in favor of the seeked song.
Call playlist_update_queued_song() after playerSeek().
After a player command (successful or not), reset pc.next_song,
because the queue is supposed to be empty then. Otherwise,
playlist.queued and pc.next_song may disagree, which triggers an
assertion failure.
Commit f78cddb4 introduced a regression: after a song was moved, the
order array was not updated (in random mode). This caused MPD to
think the "current" song has changed when you moved something to the
position of the current song.
Always assume the buffer is empty before calling the encoder. Always
flush the buffer immediately after there has been added something.
This reduces the risk of buffer overruns, because there will never be
a "rest" in the current buffer.
The non-blocking mode of libshout is sparsely documented, and MPD's
implementation had several bugs. Also removed connect throttling
code, that is done by the MPD core since 0.14.
After the state file has been loaded, the playlist version is still
"1", and "plchanges 1" returns the whole playlist. Fix this by
increasing the playlist version after the state file has been loaded.
Don't call syncPlaylistWithQueue() in nextSongInPlaylist() and
previousSongInPlaylist(). This is a relic from the time when there
was no event, and was a workaround to the timing problem.
Export the "g_playlist" variable, and pass it to all playlist
functions. This way, we can split playlist.c easier into separate
parts. The code which initializes the singleton variable is moved to
playlist_global.c.
Before every operation which modifies the playlist, remember a pointer
to the song struct. After the modification, determine the "next song"
again, and if it differs, dequeue and queue the new song.
This removes a lot of complexity from the playlist update code, and
makes it more robust.
The "current" variable is used for calculating the seek destination,
and was declared as "int". With very long song files, the 32 bit
integer can overflow. ffmpeg expects an int64_t, which is very
unlikely to overflow. Switch to int64_t.
If avcodec_decode_audio2() returns no output for an AVPacket,
libavcodec may buffer some data, and return a larger chunk of output
later. This patch disables a lot of bogus warnings.
Output the name of the codec as a debug message. During my tests,
ffmpeg never filled this struct member, but it may do so in the past,
and this debug message might become helpful.
The shout_mp3 encoder had two bugs: when no song was ever played, MPD
segfaulted during cleanup. Second bug: memory leak, each time the
shout device was opened, lame_init() was called again, and
lame_close() is only called once during shutdown.
Fix this by shutting down LAME each time the clear_encoder() method is
called.
When the update thread is started before MPD has forked (for
daemonization), it is killed, because threads do not survive a fork().
This induces an inconsistent state where MPD won't start any update
thread at all, because it thinks the thread is already running.
Move the "while" loop which checks for commands to the caller
ao_pause(). This simplifies the pause() method, and lets us remove
audio_output_is_pending().
If no ports are configured, don't overwrite the (NULL) configuration
with the port names of the first JACK server. If the server changes
after a JACK reconnect, MPD won't attempt to auto-detect again.
Currently, the JACK plugin manipulates the audio_format struct which
was passed to the open() method. This is very likely to break,
because the plugin must not permanently store this pointer. After
this patch, MPD ignores sample rate changes. It looks like other
software is doing the same, and I guess this is a non-issue.
This patch converts the audio_format pointer within jack_data into a
static audio_format struct.
Hi -
independently of libmikmod's other problems - there seems
to be a problem in mpd's wrapper: MikMod_Exit() is called
after the first file is decoded, which frees some ressources
within the mikmod library. An attempt to play a second file
leads to a crash. The appended patch fixes this for me.
(I don't know what the "dup" entry is good for - someone
who knows should review that too.)
best regards
Matthias
[mk: removed 3 more MikMod_Exit() invocations]
When there are duplicate slashes in the song paths, eliminate them;
example:
/var/lib/mpd/music//foo.mp3
becomes:
/var/lib/mpd/music/foo.mp3
The slash is only detected at the border between the music_directory
and the local part.
When the user configures a music_directory with a trailing slash, it
may break playlist loading, because MPD expects a double slash. Chop
off the trailing slash.
ffmpeg_tag_internal() does not look for a few tags that mpd
supports. Most noteably:
comment -> TAG_ITEM_COMMENT -> Description
genre -> TAG_ITEM_GENRE -> WM/Genre (not WM/GenreID)
year -> TAG_ITEM_DATE -> WM/Year
I *think* that this is the last of the tags that AVFormatContext() in
ffmpeg supports that mpd also uses.
Make those two methods optional to implement, and let input_stream.c
provide fallbacks. The buffer() method will be removed one day, and
there is now only one implementation left (input_curl.c).
The open_stream() method opens the input_stream. This allows the
archive plugin to do its own initialization, and it also allows it to
use input_stream.data. We can remove input_stream.archive now, which
was unnatural to have in the first place.
Preparation for supporting other channel numbers than stereo: use
loops instead of duplicating code for the second channel. Most
likely, gcc will unroll these loops, so the binary won't be any
different.
This patch implements the MMS protocol, by using libmms. It is quite
experimental: it does not support seeking yet, and it is currently
using synchronous I/O, which causes MPD to hang while waiting for the
server.
When the playlist is cleared, pc.errored_song is also cleared. This
causes pc_errored_song_uri() to crash, because it assumes that
pc.errored_song is set. Reset pc.error to fix that assumption.
When waiting for free space in the ring buffer, the JACK plugin
sleeped 10ms until there is enough space. This delay was too large
for low-latency setups (<10ms), and created a lot of xruns. Work
around that by reducing the sleep time to 1ms.
A proper solution for this would be to use an event based approach,
and we will do it, just not now.
When the connection failed once, you had to restart MPD, because it
never cleared the jack_data.shutdown flag. Instead, it refused to
play anything "because there is no client thread" (which is wrong at
that point).
GIOChannel is more portable than raw read()/write() calls. We're
using GIOChannel anyway, because we need it for plugging the client
into the GLib main loop.
Configure the GIOChannel to the bare minimum: no character set, no
buffering.
On some platforms, g_free() must be used for memory allocated by
GLib. This patch intends to correct a lot of occurrences, but is
probably not complete.
Both methods are always called together. There is no point in having
them separate. This simplifies the code, because the old configure()
method could be called more than once, and had to free old
allocations.
Reimplemented the legacy mixer configuration: copy the deprecated
configuration values into the audio_output section. Don't configure
the mixers twice (once for the audio_output, and a second time for the
legacy values).
This requires volume_init() to be called before initAudioDriver().
Return the default value in the conf_get_block_*() functions when
param==NULL was passed.
This simplifies a lot of code, because all initialization can be done
in one code path, regardless whether configuration is present.
Two bugs here led to a large number of interrupts being generated on the
sound card when ALSA output is being used. Because we specify no default
period_time, the sound card gives us 3000 interrupts/sec rather than a more
sane 20 or 30. This completes the revert of dd7711 already started by
4ca24f.
The larger bug was in the change to config_get_block_unsigned() and using 0
as the default value for both 'buffer_time' and 'period_time'. This means
any pre-setting of these options in newAlsaData() gets wiped out. Add a new
default for period_time, and ensure default values for buffer_time and
period_time are used if none are provided by the user.
Signed-off-by: Dan McGee <dan@archlinux.org>
[mk: set defaults in newAlsaData() to fix auto-configuration; renamed
"_MS" back to "_US" because ALSA expects microseconds, not milliseconds]
Signed-off-by: Max Kellermann <max@duempel.org>
Added all important id tags from the MusicBrainz wiki:
http://musicbrainz.org/doc/MusicBrainzTag
This should automatically enable its suport in the vorbis and flac
decoder plugins.
The input_stream API sets size to -1 when the size of the resource is
not known. The modplug decoder checked for size==0, which would be an
empty file.
You are allowed to call decoder_read() with decoder==NULL. It is a
convenience function provided by the decoder API. Don't manually fall
back to input_stream_read().
When the playlist was loaded from the state file, the order numbers
were the same as the positions. In random mode, we need to shuffle
the queue order. To accomplish that, call setPlaylistRandomStatus()
at the end of readPlaylistState(), and do a fresh shuffle.
When MPD is not playing while in random mode, and the client issues
the "clear" command, MPD crashes in stopPlaylist(), or more exactly,
in queue_order_to_position(-1). Exit from stopPlaylist() if MPD isn't
playing.
PlaylistInfo() (notice the capital 'P') sends a stored playlist to the
client. Move it to a separate library, where all the code which glues
the playlist and the MPD protocol together will live.
The playlist.c source is currently quite hard to understand. I have
managed to wrap my head around it, and this patch attempts to explain
it to the next guy.
The function playPlaylistIfPlayerStopped() is only called when the
player thread is stopped. Converted that runtime check into an
assertion, and remove one indent level.
One of the previous patches removed the "random" mode check from
nextSongInPlaylist(), which caused a shuffle whenever MPD wrapped to
the first song in "repeat" mode. Re-add that "random" check.
In playPlaylist(), the second "song==-1 && playing" check can never be
reached, because at this point, the function has already returned
(after unpausing).
When a song is deleted, start playing the next song immediately,
within deleteFromPlaylist(). This allows us to remove the ugly
playlist_noGoToNext flag, and the currentSongInPlaylist() function.
By calling queue_next_order() before playlist.current is invalidated
(by the deletion of a song), we get more robust results, and the code
becomes a little bit easier. incrPlaylistCurrent() is unused now, and
can be removed.
The function shuffles the virtual order of songs, but does not move
them physically. This is used in random mode.
The new function replaces playlist.c's randomizeOrder() function,
which was aware of playlist.current and playlist.queued. The latter
is always -1 anyway, and the former as preserved by the caller, by
converting playlist.current to a position, and then back to an order
number.
Add a "changed" check to setPlaylistRepeatStatus(): when the new
repeat mode is the same as the old one, don't do anything at all. No
more checks, no "idle" event.
When the random mode is toggled, MPD did not clear the queue. Because
of this, MPD continued with the next (random or non-random) song
according to the previous mode. Clear the queued song to fix that.
The function moveSongInPlaylist() attempted to read the position of
the current song, even if it was -1. Check that first. The same bug
was in shufflePlaylist().
The null plugin synchronizes the playback so it will happen in real
time. This patch adds a configuration option which disables this: the
playback will then be as fast as possible. This can be useful to
profile MPD.
It is possible that playlist.current is reset before the TAG event
handler playlist_tag_event() is called. Convert the assertion into a
run-time check.
Break from the loop instead of returning the function. This calls
player_stop_decoder(), which in turn emits the PLAYLIST event. This
allows the playlist to re-start the player.
Don't attempt to restart the player if it was stopped, but there were
still songs left on the playlist. This looks like it has been a
workaround for a bug which has been fixed long time ago.
The player_thread loop requests the next song from the playlist as
soon as the decoder finishes the song which is currently being played.
This is superfluous, and can lead to synchronization errors and wrong
results. The playlist already knows when the player starts playing
the next song (player_wait_for_decoder() triggers the PLAYLIST event),
and will then trigger the scheduler to provide the next song.
The "TAG" event is emitted by the player thread when the current
song's tag has changed. Split this event from "PLAYLIST" and make it
a separate callback, which is more efficient.
The "sticker" command allows clients to query or manipulate the
sticker database. This patch implements the sub-commands "get" and
"set"; more will follow soon (enumeration), as well as extended
"lsinfo" / "playlistinfo" versions.
When a song is deleted from the database, remove its sticker, too.
What's still missing is some sort of garbage collector after a fresh
database create (--create-db).
"Stickers" are pieces of information attached to existing MPD objects
(e.g. song files, directories, albums). Clients can create arbitrary
name/value pairs. MPD itself does not assume any special meaning in
them.
If a song is not within the music directory ("file:///..."), it has no
"parent directory". The archive code nonetheless dereferences the
parent pointer, causing a segmentation fault. Check parent!=NULL.
One of the previous patches made MPD consume 100% CPU in a busy wait:
when the music_pipe was full, it did not wait (with notify_wait()) for
free chunks, because a variable has a different meaning now. Always
pass "true" as the "wait" parameter.
Some plugins used the APE or ID3 tag loader as a fallback when their
own methods of loading tags did not work. Move this code out of all
decoder plugins, into song_file_update().
This new API gives the caller a writable buffer to the music pipe
chunk. This may allow the caller to eliminate several buffer copies,
because it may manipulate the returned buffer, until it calls
music_pipe_expand().
When libvorbis knows that a song is seekable, it seeks around like
crazy in the file before starting to decode it. This is very
expensive on remote HTTP resources, and delays MPD for 10 or 20
seconds.
This patch disables seeking on remote songs, because the advantages of
quickly playing a song seem to weigh more than the theoretical ability
of seeking for most MPD users. If users feel this feature is needed,
we will make a configuration option for that.
getBoolConfigParam() returns an int. It is not possible to check for
CONF_BOOL_UNSET after it has been assigned to a bool; use a temporary
int value for that.
Calling input_curl_select() after EOF has been reached causes an
assertion failure. This can happen if the HTTP response is empty.
Check c->eof before calling input_curl_select().
Path names in the directory and song structs are always encoded in
UTF-8. Don't use strcmp(), it cannot handle UTF-8 characters
properly. Use GLib's UTF-8 aware g_utf8_collate() function for that.
I was having problems with shoutcast stream outputs before applying
the attached patch, which enlarges the shoutcast output
buffer. Ideally, this should be configurable, but this resolves the
issue for my needs.
vorbis_parse_comment() should be a function which converts one comment
to a tag item. It should do everything required to do the conversion,
including looping over all possible tag types.
mpd uses some additional files to work, such as pid_file, state_file,
db_file, etc. when running mpd as non-root user, it is often that those
files end in ~/.mpd
in that case, we end up with 2 entries in a user's home, .mpdconf and
.mpd - which clutters homedirs.
this patch allows ~/.mpd/mpd.conf as an alternative to ~/.mpdconf,
allowing for a cleaner homedir
If a tag value is an empty string, the space after the colon was
removed by g_strchomp(). Fix this by removing the space check and
using g_strchug() on the return value.
The matchesAnMpdTagItemKey() API becomes more powerful and flexible if
the return value is the value pointer instead of a boolean. It also
removes (invalid and dangerous) assumptions about the string from its
caller.
When a song file is deleted during database update, all pointers to it
must be removed from the playlist. The "for" loop in
deleteASongFromPlaylist() did not deal with multiple copies of the
deleted song properly, and left instances of the (to-be-invalidated)
pointer in. Fix this by reversing the loop.
Added TAG_ITEM_ALBUM_ARTIST.
With this patch, MPD should be able to read the (inofficial)
"ALBUMARTIST" Vorbis comment. Implementations in other decoder
plugins will follow soon.
matchesAnMpdTagItemKey() broke when two tag items had the same prefix,
because it did not check if the tag name ended after the prefix. Add
a check for the colon and the space after the tag name.
If http_proxy_{host, port, user, password} are provided in mpd.conf
they are not passed on to libcurl. As a result mpd cannot stream from
behind an http proxy.
The attached patch `http_proxy.patch` makes the relevant calls to
curl_easy_setopt(...) for all proxy configuration parameters, but is
only tested for host and port.
MPD's shuffling algorithm was not implemented well: it considers songs
which were already swapped, making it somewhat non-random.
Fix the Fisher-Yates shuffle algorithm by passing the proper bounds to
the PRNG.
When decoder_run_song() (decoder_thread.c) waits for the input stream
to become ready, it did that in a busy loop. Add a select() call to
input_curl_buffer() during connect/handshake (i.e. before the first
chunk of body data was received), to let the CPU relax.
MPD will (optionall) use sqlite databases in the future. Add a
configure option to enable that. There is no code yet to really use
sqlite, so the practical use of this patch is limited.
This patch tryes to introduce pluggable mixer (struct mixer_plugin) along with some basic infrastructure (mixer_* functions). Instance of mixer (struct mixer) is used in
alsa and oss output plugin
Loosely based on a patch provided by lesion in bug #1766. The playlistinfo
command can now retrieve ranges of the playlist. The new argument indicates
which entry is the last one that will be displayed. The number of displayed
entries may be smaller than expected if the end of the playlist is reached.
Previous usage:
playlistinfo [start]
New usage:
playlistinfo [start[:end]]
This library allocates temporary buffers for storing PCM conversion
results. It should replace all those "static" buffer variables which
are racy and never freed.
Handle the DELETE and UPDATE events in separate callbacks:
song_delete_event() safely deletes a song, and update_finished_event()
is called when database update is complete.
Don't call command_error() if loading a song from the playlist fails.
This may result in assertion failures, since command_error() may be
called more than once.
Determine the suffix manually, and use decoder_plugin_from_suffix()
and archive_plugin_from_suffix() instead.
This way, song_file_update_inarchive() can be optimized: it does not
have to translate its path.
For internal checks (i.e. not in command.c), we need to check whether
an URI is in the databse, in the local file system or a remote URI
with a scheme.
When the decoder of the new song is not fast enough, the player thread
has to wait for it for a moment. However the variable "nextChunk" was
reset to -1 during that, making the next loop iteration assume that
cross-fading has not begun yet. This patch overwrites it with "0"
while waiting.
This patch fixes a minor memory leak: when decoder_tag() attempted to
send a merged tag object (created by tag_add_stream_tags()), and was
interrupted by a decoder command, it did not free the temporary merged
tag object.
Don't use g_strescape(), because it escapes all non-ASCII characters.
Add a new function which clears all non-printable characters, not just
"newline".
Commit b3e2635a introduced a regression: when a stream tag was
changed, the playlist version had to be updated. This was done in
syncCurrentPlayerDecodeMetadata(), called by syncPlayerAndPlaylist().
After b3e2635a, this was not called anymore. Fix this by emitting
PIPE_EVENT_PLAYLIST.
JACK documentation states: "The caller is responsible for calling
free(3) any non-NULL returned value."
This does not seem to include the array elements. Duplicate them
after jack_get_ports(), and free only the array. Convert
JackData.output_ports to non-const.
There is only one location using PIPE_EVENT_SIGNAL: to synchronize
player_command() with player_command_finished(). Use the "notify"
library instead of the event_pipe here.
event_pipe_emit_fast() is aimed for use in signal handlers: it doesn't
lock the mutex, and doesn't log on error. That makes it potentially
lossy, but for its intended use, that does not matter.
Make the event_pipe (formerly main_notify) send/receive a set of
events, with a callback for each one.
The default event PIPE_EVENT_SIGNAL does not have a callback. It
is still there for waking up the main thread, when it is waiting for
the player thread.
We are going to migrate away from the concept of notifying the main
thread. There should be events sent to it instead. This patch starts
a series to implement that.
With the GLib main loop, the client manager can install its own event
in case a client is expired. No need for main.c to call
client_manager_expire() manually.
The SIGHUP handler was used by the update process to make the main
process re-read the database. We don't need this anymore, since the
update takes place in a thread now.
This is a rather huge patch, which unfortunately cannot be splitted.
Instead of using our custom ioops.h library, convert everything to use
the GLib main loop.
Currently, both sides of the pipe are blocking, although we do not
need blocking read(). Convert it back to blocking. Eliminate the
select() from wait_main_task().
To wake up the main thread, don't attempt to use a GCond/GMutex
(struct notify). This kind of mixed wakeup method has known race
conditions.
The idea behind this patch is: for wakeups which happen while the main
thread is sleeping, use only a pipe. For wakeups which happen while
the main thread is waiting for the player thread, we can later change
to GCond. For now, accept the overhead of using a pipe for the
latter.
In the long run, the main thread will never wait for the player
thread, but will do everything asynchronously.
The new WIN32 version of set_nonblocking() can only deal with sockets,
i.e. it will fail on main_notify.c. On WIN32, we have to reimplement
main_notify.c anyway, so this is not a big deal.
There are no unix sockets on WIN32, and therefore no authentication.
WIN32 might have similar capabilities, but until we implement them,
disable that MPD feature.
On WIN32, parsePath() now simply duplicates the input string. There
is currently nothing special we can do here. The old code was not
portable on WIN32.
I tried to search for a certain composer in my collection, but only
non-mp4 files showed up. The source code reveals that this tag is not
read. This can be fixed by reading the 'Writer' tag field, in
mp4_plugin.c, in function mp4_load_tag.
I actually tried this, and after compiling with those lines added,
also mp4 (.m4a) files showed up when searching for a composer.
The log file is duped to STDOUT_FILENO and STDERR_FILENO. No need to
keep another copy of it in out_fd all the time. We only need it once
once in setup_log_output().
The logging library currently has 3 constructor functions: initLog(),
open_log_files(), setup_log_output(), called in this order. Merged
the first two.
If the user wants the log files with a specific mode, he has to start
MPD with the correct umask. Don't hard-code that.
This fixes a bug: when log cycling failed, MPD would not restore the
old umask.
This patch adds RVA2 (relative volume adjustment) tag
support to mpd, as a fallback if no replaygain tags are
found. The code is almost directly from madplay (GPL).
RVA2 tags are generated for example by the "normalize" utility.
Updated by: Avuton Olrich <avuton@gmail.com>
The input_stream object should only be closed by the MPD core
(i.e. decoder_thread.c / decoder_run()). A decoder plugin which
attempts to close it will result in a segmentation fault.
When save_absolute_paths_in_playlists was enabled in mpd.conf, MPD
broke all playlists when manipulated using the "playlistdelete"
command. The reason was that map_directory_child_fs() was used, which
doesn't accept slashes in the file name. Use the new map_uri_fs()
function instead.
With a large maximum playlist length, the integer multiplication
"playlist_max_length * MPD_PATH_MAX" may overflow. Change that to a
division. This was not a dangerous bug, since it was only used for
a quick estimate.
It is illegal to pass an empty audio buffer around. pcm_resample()
sometimes seems to result in 0 samples, maybe related to
libsamplerate. To work around that problem, add special checks after
both pcm_convert() invocations. Removed the pcm_resample()==0 checks
from pcm_convert().
When a response is very long (e.g. a large playlist > 100k songs),
most of it will end up in the deferred buffers. Filling the deferred
queue is very expensive currently, because a new buffer is allocated
for every client_write() operation. This may lead to long delays, and
the client might give up and disconnect meanwhile. This patch makes
MPD attempt to flush the deferred queue as often as possible, to work
around this problem. Due to the MPD 0.14 code freeze, we should not
optimize the buffering code now.
Make "secure" a log level different from "default". "secure" should be
right between "default" and "verbose". Map "default" to Glib's
"MESSAGE" log level.
There have been bug reports on MPD regarding 24 bit output via
libao/esd. The "ao" plugin does not attempt fall back to 16 bit
currently, and thus fails to play 24 bit audio (i.e. all mp3 files).
Make it always use 16 bit samples for now, until more bits are
well-tested.
The OS X output does not seem to support 24 bit audio in the way MPD
implements it currently. Fall back to 16 bit for now, and schedule
24 bit support on OS X for MPD 0.15.
MPD 0.13 and older followed all symbolic links. Although this can be
a security problem (as it has always been), 0.14 should offer the same
default behaviour as 0.13.
libsamplerate produces cracks in the sound output when the destination
buffer is too small. This is the case when pcm_convert_size() rounds
down. Use ceil(x) instead of floor(0.5+x) there to prevent a buffer
overrun.
Commit dd7711d8 removed MPD's default ALSA buffer_time. The result
was a buffer size which was way too small for playing streams on some
sound hardware, and caused skips and distorted sound. Revert the
default to 500 ms.
"float (*lamebuf)[2] = g_malloc()" does NOT allocate two float*
buffers. The formula is even wrong: it should be applied to LAME's
output buffer, not its input buffer.
Converted "lamebuf" to the two variables "left" and "right", and
allocate them independently with the exact buffer size. Set
right=left if mono output is configured.
The configuration options "follow_outside_symlinks" and
"follow_inside_symlinks" let the user control whether MPD should
follow symbolic links in the music directory.
[mk: converted variables to "bool"; moved configuration to
update_global_init()]
Those two functions are called when MPD starts and exits. It allows
the update library to perform global initialization and
deinitialization. The implementations are currently empty.
In contrast to, getBoolConfigParam(), config_get_bool() properly
returns a "bool" value. In case of "unset", it returns the default
value provided by the caller.
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.
assert_static() will help us to find false asserts in compile time. Of
course it only works in case of expressions which can be evaluated
compile time. It cannot be used in global scope.
When MPD quits in a non-clean way, the state file isn't written, and
on the next start, MPD time warps to the previous clean shutdown.
Save the state file every 5 minutes; this will probably be
configurable at a later time.
Note that we don't set a wakeup timer for that: when there is no MPD
traffic, MPD won't wake up to save the state file. This minor bug is
tolerated, because usually there is no change in MPD's state when the
main thread is idle.
If the caller attempts to seek only a few bytes forward, chances are
good that the offset is already in the buffer. In this case, simply
fast-forward the buffer.
If someone calls seek() with an invalid (negative) offset, the curl
implementation of that method returned false, but left this invalid
offset in input_stream.offset. Move the calculation to a temporary
variable.
While waiting for the input stream to become ready, ignore all
commands except STOP. This fixes seeking errors with (remote) songs
which the decoder has already finished.
skip_symlinks() expects an UTF-8 encoded file name, but
updateDirectory() passed ent->d_name (in file system encoding) to it.
Convert it to UTF-8 first.
HTTP servers respond with "416 Requested Range Not Satisfiable" when a
client attempts to seek to the end of the file. Catch this special
case in input_curl_seek(). This fixes a glitch in the ogg vorbis
decoder plugin.
Since we are using curl_multi_info_read() / CURLMSG_DONE for detecting
end-of-response, we can remove all running_handles==0 checks. For
some reason, that has never worked correctly.
curl_multi_info_read() is the authoritative source of the
"end-of-response" information. Always set c->eof when a CURLMSG_DONE
message is received, and check the result (success/failure) after
that.
When a global audio format is configured (setting
"audio_output_format"), decoder_data() overwrote the "length"
parameter with the size of the output buffer (result of
pcm_convert_size()). Declare a separate variable for the output
buffer length.
neaacdec.h declares all arguments as "unsigned long", but internally
expects uint32_t pointers. This triggers gcc warnings on 64 bit
architectures. To avoid that, make configure.ac detect whether we're
using Debian's corrected headers or the original libfaad headers. In
any case, pass a pointer to an uint32_t, conditionally casted to
"unsigned long*".
alloca() is not a portable function. Don't use it. Using
strncasecmp() is much more efficient anyway, because no memory needs
to be allocated and copied.
Don't send a "next song" request to the main thread when the current
song hasn't started playing yet, i.e. there are already two different
songs in the music pipe. This would erase information about the song
boundary within the music pipe, and thus triggered an assertion
failure. The bug could occur when playing very short songs which fit
into the pipe as a whole.
Fix a deadlock: when the decoder waited for buffer space, the player
could enter a deadlock situation because it waits for more chunks for
crossfading chunks. Signal the decoder before entering notify_wait().
The wavpack open function gives us an option called OPEN_STREAMING. This
provides more robust and error tolerant playback, but it automatically
disables seeking. (More exactly the wavpack lib will not return the
length information.) So, if the stream is already not seekable we can
use this option safely.
Seeking was somewhat broken in some decoder plugins because they sent
empty chunks, and never got a command. Check the decoder command
before doing anything else in decoder_data().
According to the documentation, mpc_decoder_decode() returns an
mpc_uint32_t. Since the special return value (mpc_uint32_t)-1
translates to a very large long integer, this may cause segmentation
faults if not interpreted properly.
Don't split the buffer conversion loop. When libmpcdec returns a
chunk, convert and send the whole chunk at a time. This moves several
checks out of the loop, and greatly improves performance.
Parse ID3 tags, even when they are in the middle of the stream. Very
few streams provide embedded ID3 tags. Most of them send only
Shoutcast "icy" tags, which limits the practical usefulness of this
patch.
When a command is received, decode_next_frame_header() and
decodeNextFrame() return DECODE_BREAK. This is already checked by
both callers, which means that we can eliminate lots of
decoder_get_command() checks.
When a tag is updated, the old tag was freed before the new one was
created. Reverse the order to be sure that other threads always see a
valid pointer.
This still leaves a possible race condition, but it will be addressed
later.
The stream_decode() and file_decode() methods returned a boolean,
indicating whether they were able to decode the song. This is
redundant, since we already know that: if decoder_initialized() has
been called (and dc.state==DECODE), the plugin succeeded. Change both
methods to return void.
The currently replay_gain_apply() implementation duplicates code from
pcm_volume(), except that it uses a floating point scale. Eliminate
all duplicated code from and make it utilize the pcm_volume() library
function. This introduces replay gain support for 24 bit audio.
It may be desirable to change the range of integer volume levels
(e.g. to 1024, which may utilize shifts instead of expensive integer
divisions). Introduce the constant PCM_VOLUME_1 which describes the
integer value for "100% volume". This is currently 1000.
The function simplifies wavpack_replaygain(), because it already
contains the float parser, and it works with a fixed buffer instead of
doing expensive heap allocations.
The assertion on dc.state in decoder_read() was too strict: when a
decoder tried to call decoder_read() from tag_dup(), the decoder state
was NONE. Allow this special case.
The flac plugin wasn't initialized properly when an OGG file was being
decoded. For some reason, flac_process_metadata() was explicitly not
called for OGG files. Since that seems to fix the issue, make it
always call flac_process_metadata().
Since decoder_list.c does not include the libflac headers, it cannot
know whether to add the oggflac plugin to the decoder list. Solve
this by always enabling the oggflac sub-plugin, even with older
libflac versions. When the libflac API cannot support oggflac,
disable the plugin at runtime by returning "false" from its init()
method.
The "oggflac" plugin was enabled only if HAVE_FLAC_COMMON was
defined. HAVE_FLAC_COMMON however is only an automake variable, and
is never available in decoder_list.c. Make decoder_list.c depend on
HAVE_FLAC||HAVE_OGGFLAC instead.
The player did not care about the exact error value, it only checked
whether an error has occured. This could fit well into
decoder_control.state - introduce a new state "DECODE_STATE_ERROR".
At this moment the wavpack lib doesn't use the return value of the
push_back function, which has an equivalent meaning of the return
value of ungetc(). This is a lucky situation, because so far it
simply returned with 1 as a hard coded value. From now on the
function will return EOF on error. (This function makes exactly one
byte pushable back.)
There are some functions in the wavpack-mpd input streams wrapper
which had too commonly used names (especially can_seek). I prefixed
these with "wavpack_input_".
The listen.c module breaks the build because the variable name used
("sun") for the Unix domain socket part collides with something else
on an OpenSolaris system, likely Sun specific. Renaming it to _sun
(or something else of choice) fixes the build.
[mk: renamed to "s_un"]
I had this option enabled during development, but at some point, it
must have gotten lost. FAILONERROR makes the curl stream fail when
the server returns a status code 400 or higher. We are not interested
in the server's error document.
Initialize libc's locale functions. Currently, we are only interested
in LC_CTYPE (character classification), because this is what is used
by GLib's g_get_charset().
GLib provides the function g_get_filename_charsets() which determines
the file system character set. This changes MPD's fallback: GLib
prefers UTF-8 as a fallback. MPD used to fall back to ISO Latin 1.
libwavpack expects the read_bytes() stream method to fill the whole
buffer, and fails badly when we return a partial read (i.e. not enough
data available yet). This caused wavpack streams to break.
Re-implement the buffer filling loop.
Instead of manually waiting for the input stream to become ready (to
catch server errors), just read the first byte. Since the
wavpack_input has the capability to push back one byte, we can simply
re-feed it. Advantage is: decoder_read() handles everything for us,
i.e. waiting for the stream, polling for decoder commands and error
handling.
The API of mp4_load_tag() was strange: it always returned a tag
object, no matter if a tag was found in the file; the existence of a
tag was indicated with the tag_found integer reference. This flag is
superfluous, since we can simply check whether the tag is empty or
not.
Allocate the mp4ff_callback_t object on the stack. This is easier to
handle, since we don't have to free it. Incidentally, this fixes a
memory leak in mp4_load_tag().
The function decoder_read() already cares about the decoder command,
and loops until data is available. Reduced mpd_ffmpeg_read() to no
more than the decoder_read() call.
The variable "next_song" is already protected by a memory barrier.
"total_time" is not important for synchronization, and we don't need
"volatile" here.
If an input stream provides tags (e.g. from an icecast server), send
them in the decoder_data() and decoder_tag() methods. Removed the
according code from the mp3 and oggvorbis plugins - decoders shouldn't
have to care about stream tags.
This patch also adds the missing decoder_tag() invocation to the mp3
plugin.
MPD used to have a copy of the mp4ff library. Since that has been
removed, AAC suport was disabled when there was no libmp4ff. Separate
the libmp4ff test, and enable AAC support no matter if libmp4ff is
available.
The "mod" decoder plugin was being initialized lazily, but was
deinitialized unconditionally. That led to segmentation faults.
Convert mod_initMikMod() to be the global module initialization
method. The MPD core should care about lazy initialization.
Non-local songs used to have no tags. If the decoder sends us a tag,
we should incorporate it into the song struct. This way, clients can
always show the correct song name (if provided by the server).
The try_decode() method may have read some data from the stream, which
is now lost. To make this data available to other methods, get it
back by rewinding the input stream after each try_decode() invocation.
The ogg and wavpack plugins did this manually and inconsistently; this
code can now be removed.
If the source chunk has a tag, merge it into the destination chunk.
The source chunk gets deleted after that, and this is our last chance
to grab the tag.
Ogg and ffmpeg detection was disabled when the stream was not
seekable, because the detection was too expensive. Since the curl
input stream can now rewind the stream cheaply, we can re-enable
detection on streams.
During codec detection, the beginning of the stream is consumed. This
is a common operation, which takes a lot of time when handling remote
resources. To optimize this, remember the first 64 kB of a stream.
This way, we can rewind the stream without actually fetching the start
of the stream again.
Since the aac and mod plugins have told MPD that they cannot seek, MPD
will never send a SEEK command to them. Removed the SEEK comand
checks from both plugins.
Don't pass the "seekable" flag with every decoder_data() invocation.
Since that flag won't change within the file, it is enough to pass it
to decoder_initialized() once per file.
Replace all direct music_pipe struct accesses with wrapper functions.
The compiled machine code is the same, but this way, we can change
struct internals more easily.
.. and rename dc.audioFormat to dc.in_audio_format. The music pipe
does not need to know the audio format, and its former "audioFormat"
property indicated the format of the most recently added chunk, which
might be confusing when you are reading the oldest chunks.
No CamelCase in the file name. The output_buffer struct is going to
be renamed to music_pipe. There are so many buffer levels in MPD, and
calling this one "output buffer" is wrong, because it's not the last
buffer before the music reaches the output devices.
Commit 1a4a3e1f moved decoders into a static array, but failed to
enable those plugins who did not have an init() method at all.
This patch corrects the "enabled" check.
Make map_directory_child_fs() refuse the names "." and "..". This is
currently the interface where an attacker may inject a manipulated
path (through the "update" command).
"LOG_H" is a macro which is also used by ffmpeg/log.h. This is
ffmpeg's fault, because short macros should be reserved for
applications, but since it's always a good idea to choose prefixed
macro names, even for applications, we are going to do that in MPD.
Depending on MPD's umask, the file permissions of the unix socket were
too restrictive, and many clients were not able to connect. Do a
chmod(0666) on the socket, to allow everybody to connect.
Similar to libmad, libmpcdec provides samples with higher quality than
16 bit. Send 24 bit samples to MPD, which allows MPD to apply
dithering just in case the output devices are only 16 bit capable.
The conversion of integer samples was completely broken, which
presumably didn't annoy anybody because libmpcdec provides float
samples on most installations.
Its only caller in mp3_decode() just compared its value with
DECODE_BREAK. Convert that to bool, and return false if the loop
should be ended. Also eliminate some superfluous command checking
code, which was already done in the preceding while loop.
When one of several output devices failed, MPD tried to reopen it
quite often, wasting a lot of resources. This patch adds a delay:
wait 10 seconds before retrying. This might be changed to exponential
delays later, but for now, it makes the problem go away.
When the decoder exited before the buffer has grown big enough
("buffer_before_play"), the player thread waited forever. Add an
additional check which disables buffering as soon as the decoder
exits.
The local variable "play_audio_format" is updated every time the
player starts playing a new song. This way, we always know exactly
which audio format is current. The old code broke when a new song had
a different format: ob.audio_format is the format of the next song,
not of the current one - using this caused breakage for the software
volume control.
A decoder_flush() invocation was missing in the FLAC plugin, resulting
in casual assertion failures due to a wrong assumption about the last
chunk's audio format. It's much easier to remove that decoder_flush()
function and make the decoder thread call ob_flush().
Request the next song from the playlist (by clearing pc.next_song)
only if the player command is empty. If it is not, the player may be
clearing the song that has already been queued, leading to an
assertion failure.
Remember the seek_where argument and call decoder_command_finished()
immediately. This way, the player thread can continue working, and we
can receive more commands.
This also fixes several issues which resulted in broken frames,
leading to erroneos "elapsed" values: frames weren't parsed properly,
since the code was checking for command!=NONE.
size_t and long aren't 64 bit safe (i.e. files larger than 2 GB on a
32 bit OS). Use off_t instead, which is a 64 bit integer if compiled
with large file support.
When the decoder failed to start, the function do_play() returned,
still having pc.command==PLAY. This is because pc.command was reset
only when the decoder started up successfully. Add another
player_command_finished() call in the error handler.
Replaced the local variable "colon" (which had only temporary meaning)
with the variable "value". It is a pointer to the first byte of the
header value.
Instead of managing a set of method pointers in each input_stream
struct, move these into the new input_plugin struct. Each
input_stream has only a pointer to the plugin struct. Pointers to all
implementations are kept in the array "input_plugins".
MPD's HTTP client code has always been broken, no matter how effort
was put into fixing it. Replace it with libcurl, which is known to be
quite stable. This adds a fat library dependency, but only for people
who need streaming.
MPD shouldn't integrate sources of other libraries. Since libmp4ff is
part of libfaad, we should remove the old copy from src/mp4ff and link
with the current version from libfaad instead.
PA_SAMPLE_S16NE is the only sample format which is suported by both
MPD and pulseaudio. Unfortunately, pulse does not accept 24 bit
samples.
Instead of bailing out with an error message, we should tell the MPD
core to convert all samples to 16 bit for pulse.
This bug caused the audio output devices to stay open, although MPD
wasn't playing: quitDecode() resetted player_control.command, assuming
that the command was STOP. This way, player_task() didn't see the
CLOSE_AUDIO command, and the device was kept open.
Don't clear player_control.command in quitDecode().
When the audio source provides 24 bit samples, don't bother to convert
(lossily) them to 16 bit before jack's floating point conversion - go
directly from 24 bit to float.
The JACK documentation postulates that the process() callback must not
block, therefore locking is forbidden. Anyway, the old code was racy.
Remove all locks, and don't wait for more data to become available -
just send to the port what is already in the buffer.
Another partial frame fix: the silence buffer was 1020 bytes, which
had room for 127.5 24 bit stereo frames. Don't send the partial last
frame in this case.
24 bit output is as important as 16 bit output. Provide a
pcm_convert() implementation which can convert to 24 bit with as
little quality loss as possible.
The old pcm_convert_size() ignored most of the destination format,
e.g. it did not check its sample size, and assumed it is 16 bit.
Simplify and universalize it by using audio_format_frame_size().
Similar to pcm_resample_16(), implement pcm_resample_24(). The 24 bit
implementation is very similar, but it uses src_int_to_float_array()
instead of src_short_to_float_array() before sending data to
libsamplerate.
Use sizeof(sample) instead of hard-coding "2". Although we're in 16
bit right now, this will make code sharing easier when we support
other sample sizes.
libmad produces samples of more than 24 bit. Rounding that down to 16
bits using dithering makes those people lose quality who have a 24 bit
capable sound device. Send 24 bit PCM data, and let the receiver
decide whether to apply 16 bit dithering.
I added 24 bit support a while ago, but it wasn't possible to force 24
bit output. Add 24 and 8 bit to the list of allowed sample sizes.
Although 8 bit audio isn't as widely used as 24 bit, there is no
reason to exclude it.
Splitting a frame between two buffer chunks causes distortion in the
output. MPD used to assume that the chunk size 1020 would never cause
splitted frames, but that isn't the case for 24 bit stereo (127.5
frames), and even less for files with even more channels.
Many command arguments must not be negative; add a separate
parser/checker function for that. For the same reason, add
check_bool(). This eliminates two strange special cases handlers from
check_int().
Pass index arguments as unsigned integers. They must not be negative,
and even if some caller accidently passes -1, it won't pass the bound
checks (since it's now 2**32-1).
There are some integers which have a "magic" -1 value which means
"undefined" or "nothing". All others can be converted to unsigned,
since they must not contain a negative number.
Also add names for "error" and "ok". I don't like passing anonymous
integer codes around.
This is not yet complete: lots of functions (e.g. in playlist.c)
follow the same convention of -1/0, and these have to be adapted, too.
spl_list() provides an interface for enumerating all stored playlists.
This separates the internal playlist logic from the protocol specific
function lsPlaylists().
The two functions clearStoredPlaylist() and addToStoredPlaylist()
don't belong into playlist.c. clearStoredPlaylist() was a wrapper for
spl_clear(), and is converted into a CPP macro for now.
The list of commands is known at compile time. Instead of creating a
linked list on startup, we can just register all commands in a static
sorted array.
The command pointers which are passed around aren't being modified -
in fact, no command pointer must be modified once it has been added to
the commandList.
Instead of manually calling memset(0) on the pcm_convert_state struct,
client code should use a library function from pcm_utils.c. This way,
we can change the semantics of the struct easily.
Casting a pointer to some sort of integer and formatting it into a
string isn't valid. A pointer derived from this hex string won't work
reliably. Since ffmpeg doesn't provide a nice API for passing our
pointer, we have to think of a different hack: ffmpeg passes the exact
URL pointer to mpdurl_open(), and we can make this string part of a
struct. This reduces the problem to casting the string back to the
struct.
This is still a workaround, but this is "sort of portable", unless the
ffmpeg people start messing with the URL pointer (which would be valid
according to the API definition).
Since ffmpeg svn r12865, you have to include libavcodec/avcodec.h
instead of avcodec.h. This cannot be checked at compile time, instead
we have to add a check to configure.ac. Viliam's original ffmpeg
plugin was based on the newer ffmpeg library, while my Debian
installation had the older version. My attempt to correct his include
statements wasn't correct after all.
{song,dir}vec_for_each each failed to gracefully handle deleted
files when iterating through. While we were thread-safe, we
were not safe within the calling thread. If a callback we
passed caused sv->nr to shring, our index would still increment;
causing files to stay in the database.
A way to test this is to remove 10 or so contiguous songs from a
>10 song directory.
Like the songvec nr_lock, only one lock is used for all
traversals since they're rarely changed. This only
projects traversals, but not the individual structures
themselves.
Use a literal in the struct declaration, and sizeof(client->buffer)
everywhere else. Also shrink the buffer from 40 kB to 4 kB. The
buffer must only be large enough to hold one line of input, and 4 kB
is still more than enough.
When adding a local file, clients have to use the "file" URI schema
described in RFC 1738 3.10. By adding this schema to "urlhandlers", a
client can detect whether this feature is available.
By default, glibc 2.8 hides struct ucred behind the _GNU_SOURCE
macro. I don't want to enable that globally, because it may encourage
the use of non-portable functions. Test if "struct ucred" is
available, and enable _GNU_SOURCE if required.
For details about that issue, see glib's bug database:
http://sources.redhat.com/bugzilla/show_bug.cgi?id=6545
Some functions assume that a song is not in the database when it is a
remote song. Based on that, they decide whether they are responsible
for freeing the song struct. Add a special function which checks
whether a song is in the database (currently equal to song_is_file()).
GLib provides an easier API for character set conversion than iconv().
Use g_convert() / g_convert_with_fallback() for all character
conversions. We should optimize the path.h API later to return a
newly allocated buffer, so we can just pass GLib's return value.
GLib is a nice and portable utility library. We are going to use it
from now on, and eliminate a lot of duplicated code from MPD. Why
invent the wheel again and again?
Use memchr() instead of manually traversing the input buffer. Update
the client's properties after all commands have been processed. Check
for buffer overflow once.
The caller already knows the protocol family, and we can eliminate the
complicated switch statement in establishListen() if we just pass this
information. This seems more robust.
"idle" waits until something noteworthy happens on the server,
e.g. song change, playlist modified, database updated. This allows
clients to keep up to date without polling.
Added mpd.conf options for disabling automatic resamling, sample
format and channel conversion. This way, users may choose to override
ALSA's automatic resampling, and use libsamplerate instead.
This git branch has become a real MPD fork now. Time to change the
package name to the code name "mpd-mk". Set the version number to
"0.14~git" to mark this as a non-released version.
Don't follow relative symlinks which point into the music directory.
This allows you to organize music with symbolic links, without MPD
managing separate copies of each song.
The mapper library maps directory and song objects to file system
paths. With this central library, the code mixture in path.c should
be cleaned up, and we will be able to add neat features like aliasing.
isMusic() used to be a very inefficient function: with every
invocation, it did another stat() on the specified file. There is
only one caller, do the stat() there manually and use hasMusicSuffix()
instead of isMusic().
By always creating the parent directory, we can use delete_name_in()
without further lookups. The parents which may non exist will be
pruned later. An update request for a non-existing or empty directory
should be quite unusual, so this doesn't add any measurable overhead.
In order to optimize buffer usage, pass only the base file name to
updateInDirectory(). This way, updateInDirectory() may choose when to
allocate a larger buffer for the full path.
It is invalid to pass a path with the wrong dirname to dirvec_find().
To be able to find a subdirectory only by its basename, compare only
the basename of both paths.
The only caller of deletePlaylist() appends PLAYLIST_FILE_SUFFIX, so
we can be sure it's already there. We don't need to stat the file,
since unlink() does all the checking.
Commit 80a2c937 broke resume after pause: it cleared the
input_audio_format when it attempted to simplify a complicated
expression. Don't clear it, just assign input_audio_format if a new
format was specified.
We only need to lock sv->nr changes to prevent traversals ( why
it's called "nr_lock"). free(3) is a "slow" function on my
system; so we can avoid unnecessarily holding a lock long for
longer than needed.
If the sample format isn't supported by the device (i.e. 24 bit on
low-end sound chips), fall back to 16 bit output. There is code in
pcm_utils.c which converts PCM data to 16 bit.
Convert any number of channels to stereo. In fact, this isn't really
stereo, it's rater mono blown up to stereo. This patch should only
make it possible to play 5.1 files at all; "real" conversion to stereo
should be implemented, but for now, this is better than nothing.
In order to be able to deal with non-trivial conversions,
pcm_convertChannels() needs to know both the input and the output
channel count. Simplify buffer allocation in that function.
Moved code from pcm_convertChannels() to pcm_convert_channels_1_to_2()
and pcm_convert_channels_2_to_1(). Improved the quality of
pcm_convert_channels_2_to_1() by calculating the arithmetic mean value
of both samples.
buffered_before_play was copied to struct player because it was used
to disable buffering when seeking. Instead of mainaining a copy of
this number, move just the flag to the player struct.
Renamed audio_configFormat to configured_audio_format. Renamed
audio_buffer.format to input_audio_format. Simplified its
initialization in openAudioDevice().
audio.c maintained one of MPD's many layers of audio buffers. It was
without any benefit, since playAudio() can simply send the source
buffer directly to the audio output plugin.
QUEUE adds a new song to the player's queue. CANCEL clears the queue.
These two commands replace the old and complex queueState and
queueLockState code.
Simplify and merge several if clauses before the clearPlayerQueue()
invocation. Call clearPlayerQueue() only if a song is actually
queued; add an assertion for that in clearPlayerQueue().
This variable is superfluous, it is only used to copy its value to
player_control.totalTime. Since the original source of this value
(song->tag->time) will still be available at this point, we can safely
remove fileTime.
Revert e4f5d6bd "re-enable-nonblocking, but sleep if busy".
Non-blocking mode with manual sleeping doesn't help at all (by the
way, the patch should have used snd_pcm_wait() instead of
my_usleep()). ALSA knows much more about the hardware quirks, so we
just let it do the job.
Leftover from the output API changes: oss_open_default() was changed
to return a void*, but it still returned "0" to report success.
Report the OssData pointer instead.
The decoder was woken up after each chunk which had been played. That
caused a lot of superfluous context switches. Wake up the decoder
only when a certain amount of the buffer has been consumed. This
formula is somewhat arbitrary, and has to be proven experimentally.
The mp3 plugin did not use the MAD_NCHANNELS() value correctly: when a
stream was not stereo, it was assumed to be mono, although the correct
number was passed to MPD. libmad doesn't support more than 2
channels, but this change allows gcc to optimize its inlining
strategy.
The dithering function audio_linear_dither() worked for signed 16 bits
only anyway, having a variable "bits" just disables important gcc
optimizations.
A frame contains one sample per channel, thus it is sample_size *
channels. This patch includes some cleanup for various locations
where the sample size for 24 bit audio was still 3 bytes (instead of
4).
There is only once update thread at a time. Make the "modified" flag
global and remove the return values of most functions. Propagating an
error is only useful for updateDirectory(), since updateInDirectory()
will delete failed subdirectories.
The documentation for directory_update_init() was incorrect: a job ID
must be positive, not non-negative. If the update queue is full and
no job was created, it makes more sense to return 0 instead of -1,
because it is more consistent with the return value of isUpdatingDB().
pthread_join() expects a "pointer to a pointer" parameter, but it got
a "pointer to an enum". On AMD64, an enum is smaller than a pointer,
leading to a buffer overflow.
In updateInDirectory(), add new directories immediately and
delete them when they turn out to be empty. This simplifies the code
and allows us to eliminate addSubDirectoryToDirectory().
If the user requests database update during startup, call
directory_update_init(). This should be changed to fully asynchronous
update later.
For this to work, main_notify has to be initialized before db_init().
The algorithm in addDirectoryPathToDB() can be simplified further if
it is combined with the function addParentPathToDB(). Since there is
no other caller of addDirectoryPathToDB(), we can do that. This saves
another large stack buffer.
This recursive function is very dangerous because it allocates a large
buffer on the stack in every iteration. That may be misused to
generate a stack overflow.
When a directory failed to update, it was removed from the database,
without freeing all children and songs (memory leak), and without
locking (race condition). Introduce the functions clear_directory()
and delete_directory(), which do both.
Don't use db_get_directory() and traverse the full path with every
directory being loaded. Just see if the current parent contains the
entry. Everything else would be invalid anyway..
A manipulated database could trigger an assertion failure, because the
parent didn't match. Do a proper check if the new directory is within
the parent's. This uses FATAL() to bail out, so MPD still dies, but
it doesn't crash.
Remove clutter from directory.c. Everything which saves or loads
to/from the hard disk goes to directory_save.c, and code which sends
directory information to the client is moved into directory_print.c.
Having an array with disabled entries sucks. Removed that
DISABLED_SHOUT_ENCODER_PLUGIN macro, and fill the plugin list only
with plugins which are actually enabled. This should be done for all
plugin types.
"volume" was passed as an unsigned integer, which is correct. It's
just that when it was multiplied with the sample value, the whole
operation was changed to unsigned, breaking the algorithm (and Qball's
ears). Internally change "volume" to signed.
With commit 6dcd7fea (if I am not mistaken) the error returned when
you try to save to an existing playlist is wrong. Instead of
MPD_ACK_ERROR_EXIST, MPD_ACK_ERROR_NO_EXIST is returned. This is
obviously wrong and breaks gmpc.
Commit 0bfe7802 broke update for new files in the root directory,
because music_root->path was an empty string and not NULL. There were
some NULL tests missing. Change them to !isRootDirectory(path)
instead of path!=NULL.
Taming the directory.c monster, part II: move the database management
stuff to database. directory.c should only contain code which works
on directory objects.
Instead of returning 0 or -1, return true on success and false on
failure. This seems more natural, and when the C library was
designed, there was no "bool" data type.
Provide separate constructors for creating a remote song, a local
song, and one for loading data from a song file. This way, we can add
more assertions.
