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().
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.
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 "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.
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.
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.
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.
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]]
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.
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.
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.
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.
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.
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()).
"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.
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.
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.
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().
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.
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.
We already know if a song is a URL or not based on whether it
has parentDir defined or not. Hopefully one day in the future
we can drop HTTP support from MPD entirely when an HTTP
filesystem comes along and we can access streams via open(2).
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.
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.
Continuing the effort of removing protocol specific calls from the
core libraries: let the command.c code call commandError() based on
PlaylistInfo's return value.
The playlist library shouldn't talk to the client if possible.
Introduce the "enum playlist_result" type which the caller
(i.e. command.c) may use to generate an error message.
Client's input values should be validated by the command
implementation, and the core libraries shouldn't talk to the client
directly if possible. Thus, setPlaylistRepeatStatus() and
setPlaylistRandomStatus() don't get the file descriptor, and cannot
fail (return void).
The function valid_playlist_name() checks the name, but it insists on
reporting an eventual error to the client. The new function
is_valid_playlist_name() is more generic: it just returns a boolean,
and does not care what the caller will use it for. The old function
valid_playlist_name() will be removed later.
Move everything which dumps song information (via tag_print.c) to a
separate source file. song_print.c gets code which writes song data
to the client; song_save.c is responsible for serializing songs from
the tag cache.
Max Kellermann