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.