In !NDEBUG, remember which audio_format is stored in every chunk and
every pipe. Check the audio_format of every new data block appended
to the music_chunk, and the format of every new chunk appended to the
music_pipe.
Turn the music_pipe into a simple music_chunk queue. The music_chunk
allocation code is moved to music_buffer, and is now managed with a
linked list instead of a ring buffer. Two separate music_pipe objects
are used by the decoder for the "current" and the "next" song, which
greatly simplifies the cross-fading code.
The warning message "problems opening audio device while playing ..."
does not help at all, and should be removed. At this point, the real
error message has already been logged by the output thread.
When a file is not seekable, MPD dropped the audio buffers before even
attempting to seek. This caused noticable sound corruption. Fix:
first attempt to seek, and only if that succeeds, call
audio_output_all_cancel().
The crossfading code shouldn't depend on the audio output code. Pass
the current audio format to cross_fade_calc() and let it compare
directly, instead of using isCurrentAudioFormat().
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.
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.
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.
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.
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.
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.
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.
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.
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.
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().
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 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".
Max Kellermann