The way we used non-blocking mode was HORRIBLE.
It was non-blocking to ALSA, but we end up blocking in a busy
loop that does absolutely NOTHING but retry. We don't check
for playback cancellation (like we do in decoders) or anything.
This is seriously broken and I can imagine it affects people on
fast CPUs more because we do asynchronous output buffering and
our ALSA device will always have data ready.
This is safer than the patch in
http://www.musicpd.org/mantis/view.php?id=1542
with multiple audio outputs enabled.
Sadly, I only noticed that patch/problem when I googled for
"snd_config_update_free_global"
Apparently snd_pcm_hw_params_can_resume() can return false even
though my hardware does in fact support resuming. So stop
carrying that value in the canResume flag and just try to resume
when we're in the suspended state; falling back to
snd_pcm_prepare only if resuming fails. libao does something
similar on resume, too.
While we're at it, use the E() macro which will enable us to
have better error reporting.
[mk: remove the E() macro stuff]
With a large music database, the linear string collection in
tagTracker.c becomes very slow. We implemented that in a
quick'n'dirty fashion when we removed tree.c, and now we rewrite it
using the fast hashed string set.
"struct strset" is a hashed string set: you can add strings to this
library, and it stores them as a set of unique strings. You can get
the size of the set, and you can enumerate through all values.
This will be used to replace the linear tagTracker library.
Instead of having to register each output plugin, store them
statically in an array. This eliminates the need for the List library
here, and saves some small allocations during startup.
Due to clumsy layout, the audio_format struct took 12 bytes. Move the
"channels" to the end, so it can be merged into the same 32 bit slot
as "bits", which reduces the struct size to 8 bytes.
print_playlist_result() had an assert(0) at the end, in case there was
an invalid result value. With NDEBUG, this resulted in a function not
returning a value - add a dummy "return -1" at the end to keep gcc
quiet.
Since all callers of song_id_exists() will map it to a song position
after the check, introduce a new function called song_id_to_position()
which performs both the check and the map lookup, including nice
assertions.
volatile provides absolutely no guarantee thread-safety in SMP
environments. volatile was designed to access memory locations
in peripheral hardware directly; not for SMP. If volatile is
needed to work properly on SMP, then it is only hiding subtle
bugs.
volatile only prevents the /compiler/ from making optimizations
when accessing variables. CPUs do their own optimizations at
runtime so it cannot guarantee registers of CPUs are flushed
to memory cache-coherent access on different CPUs.
Furthermore, the thread-communication via condition variables
between threads sharing audio formats already results in memory
barriers.
The tag pool is a shared global resource that is infrequently
modified. However, it can occasionally be modified by several
threads, especially by the metadata_pipe for streaming metadata
(both reading/writing).
The bulk tag_item pool is NOT locked as currently only the
update thread uses it.
Trying to read or remember
"tag->numOfItems * sizeof(*tag->items)"
requires too much thinking and mental effort on my part.
Also, favor "sizeof(struct mpd_tag)" over "sizeof(*tag->items)"
because the former is easier to read and follow, even though
the latter is easier to modify if the items member changes
to a different type.
The previous patch enabled these warnings. In Eric's branch, they
were worked around with a generic deconst_ptr() function. There are
several places where we can add "const" to pointers, and in others,
libraries want non-const strings. In the latter, convert string
literals to "static char[]" variables - this takes the same space, and
seems safer than deconsting a string literal.
All callers of fdprintf() have been converted to client_printf() or
fprintf(); it is time to remove this clumsy hack now. We can also
remove client_print() which took a file descriptor as parameter.
Now that we have removed all invocations of client_get_fd(), we can
safely remove this transitional function. All access to the file
descriptor is now hidden behind the interface declared in client.h.
The shared code in showPlaylist() isn't worth it, because we aim to
remove fdprintf(). Duplicate this small function, and enable stdio
buffering for saved playlists.
The function loadPlaylist() wants to report incremental errors to the
client, for this reason we cannot remove its protocol dependency right
now. Instead, make it use the client struct instead of the raw file
descriptor.
Don't pass the raw file descriptor around. This migration patch is
rather large, because all of the sources have inter dependencies - we
have to change all of them at the same time.
Pass the client struct to CommandHandlerFunction and
CommandListHandlerFunction. Most commands cannot take real advantage
of that yet, since most of them still work with the raw file
descriptor.
These two functions take a client struct instead of the file
descriptor. We will now begin passing the client struct around
instead of a raw file descriptor (which needed a linear lookup in the
client list to be useful).
