From 344d10a8e3466345c41209656fd91ede39ff93f4 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 11 Jul 2014 19:29:25 +0200 Subject: [PATCH 1/9] PlaylistControl: update code comment --- src/PlaylistControl.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PlaylistControl.cxx b/src/PlaylistControl.cxx index 2dbd75d6e..58971a4b4 100644 --- a/src/PlaylistControl.cxx +++ b/src/PlaylistControl.cxx @@ -153,7 +153,7 @@ playlist::PlayNext(PlayerControl &pc) queue.ShuffleOrder(); /* note that current and queued are - now invalid, but playlist_play_order() will + now invalid, but PlayOrder() will discard them anyway */ } From e2cc328eef4d7c19a6bc7755779babe88ff51105 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 11 Jul 2014 19:41:39 +0200 Subject: [PATCH 2/9] Playlist: randomize next song when enabling "random" mode while not playing Don't restore the current song after shufflung when MPD is stopped (but still remembers the current song internally). Fixes the first part of Mantis ticket 0004005. --- NEWS | 1 + src/Playlist.cxx | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index b67eb9314..afd37bb75 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,7 @@ ver 0.18.12 (not yet released) - audiofile: improve responsiveness - audiofile: fix WAV stream playback - dsdiff, dsf: fix stream playback +* randomize next song when enabling "random" mode while not playing ver 0.18.11 (2014/05/12) * decoder diff --git a/src/Playlist.cxx b/src/Playlist.cxx index 8d9ab24a3..ac2cc494b 100644 --- a/src/Playlist.cxx +++ b/src/Playlist.cxx @@ -294,7 +294,9 @@ playlist::SetRandom(PlayerControl &pc, bool status) if (queue.random) { /* shuffle the queue order, but preserve current */ - const int current_position = GetCurrentPosition(); + const int current_position = playing + ? GetCurrentPosition() + : -1; queue.ShuffleOrder(); From a8a85143f6e0b0fdc2467722d057a4b8526f7e69 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 11 Jul 2014 20:22:22 +0200 Subject: [PATCH 3/9] QueueCommands: make "result" more local --- src/command/QueueCommands.cxx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/command/QueueCommands.cxx b/src/command/QueueCommands.cxx index a21eb75f0..a20b24132 100644 --- a/src/command/QueueCommands.cxx +++ b/src/command/QueueCommands.cxx @@ -43,7 +43,6 @@ CommandResult handle_add(Client &client, gcc_unused int argc, char *argv[]) { char *uri = argv[1]; - PlaylistResult result; if (memcmp(uri, "file:///", 8) == 0) { const char *path_utf8 = uri + 7; @@ -59,7 +58,7 @@ handle_add(Client &client, gcc_unused int argc, char *argv[]) if (!client_allow_file(client, path_fs, error)) return print_error(client, error); - result = client.partition.AppendFile(path_utf8); + auto result = client.partition.AppendFile(path_utf8); return print_playlist_result(client, result); } @@ -70,7 +69,7 @@ handle_add(Client &client, gcc_unused int argc, char *argv[]) return CommandResult::ERROR; } - result = client.partition.AppendURI(uri); + auto result = client.partition.AppendURI(uri); return print_playlist_result(client, result); } From 11a5ee821b99da7c58da4cb2251c8686e2d615cf Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 11 Jul 2014 20:01:53 +0200 Subject: [PATCH 4/9] PlaylistEdit: postpone UpdateQueuedSong() when adding multiple songs Implement a "bulk" edit mode that postpones both UpdateQueuedSong() and OnModified(). This way, the playlist version gets incremented only once. More importantly: when adding multiple songs to a queue that consists of only one song, the first song that got added will always be played next. By postponing this choice, all newly added songs get a chance to become the next song. Fixes the second (and last) part of Mantis ticket 0004005. --- Makefile.am | 1 + NEWS | 1 + src/BulkEdit.hxx | 41 ++++++++++++++++++++++++++++++++ src/Playlist.cxx | 6 +++++ src/Playlist.hxx | 19 ++++++++++++++- src/PlaylistEdit.cxx | 35 +++++++++++++++++++++++++++ src/command/DatabaseCommands.cxx | 3 +++ src/command/PlaylistCommands.cxx | 3 +++ src/command/QueueCommands.cxx | 3 +++ 9 files changed, 111 insertions(+), 1 deletion(-) create mode 100644 src/BulkEdit.hxx diff --git a/Makefile.am b/Makefile.am index 3eb8fac0a..25352e362 100644 --- a/Makefile.am +++ b/Makefile.am @@ -181,6 +181,7 @@ src_mpd_SOURCES = \ src/PlaylistInfo.hxx \ src/PlaylistDatabase.cxx src/PlaylistDatabase.hxx \ src/PlaylistUpdate.cxx \ + src/BulkEdit.hxx \ src/IdTable.hxx \ src/Queue.cxx src/Queue.hxx \ src/QueuePrint.cxx src/QueuePrint.hxx \ diff --git a/NEWS b/NEWS index afd37bb75..198ada6a9 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,7 @@ ver 0.18.12 (not yet released) - audiofile: fix WAV stream playback - dsdiff, dsf: fix stream playback * randomize next song when enabling "random" mode while not playing +* randomize next song when adding to single-song queue ver 0.18.11 (2014/05/12) * decoder diff --git a/src/BulkEdit.hxx b/src/BulkEdit.hxx new file mode 100644 index 000000000..422dc4f38 --- /dev/null +++ b/src/BulkEdit.hxx @@ -0,0 +1,41 @@ +/* + * Copyright (C) 2003-2014 The Music Player Daemon Project + * http://www.musicpd.org + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#ifndef MPD_BULK_EDIT_HXX +#define MPD_BULK_EDIT_HXX + +#include "Partition.hxx" + +/** + * Begin a "bulk edit" and commit it automatically. + */ +class ScopeBulkEdit { + Partition &partition; + +public: + ScopeBulkEdit(Partition &_partition):partition(_partition) { + partition.playlist.BeginBulk(); + } + + ~ScopeBulkEdit() { + partition.playlist.CommitBulk(partition.pc); + } +}; + +#endif diff --git a/src/Playlist.cxx b/src/Playlist.cxx index ac2cc494b..526f35298 100644 --- a/src/Playlist.cxx +++ b/src/Playlist.cxx @@ -103,6 +103,12 @@ playlist::UpdateQueuedSong(PlayerControl &pc, const Song *prev) if (!playing) return; + if (prev == nullptr && bulk_edit) + /* postponed until CommitBulk() to avoid always + queueing the first song that is being added (in + random mode) */ + return; + assert(!queue.IsEmpty()); assert((queued < 0) == (prev == nullptr)); diff --git a/src/Playlist.hxx b/src/Playlist.hxx index 7d7e9b154..b660ecb40 100644 --- a/src/Playlist.hxx +++ b/src/Playlist.hxx @@ -45,6 +45,18 @@ struct playlist { */ bool stop_on_error; + /** + * If true, then a bulk edit has been initiated by + * BeginBulk(), and UpdateQueuedSong() and OnModified() will + * be postponed until CommitBulk() + */ + bool bulk_edit; + + /** + * Has the queue been modified during bulk edit mode? + */ + bool bulk_modified; + /** * Number of errors since playback was started. If this * number exceeds the length of the playlist, MPD gives up, @@ -69,7 +81,9 @@ struct playlist { int queued; playlist(unsigned max_length) - :queue(max_length), playing(false), current(-1), queued(-1) { + :queue(max_length), playing(false), + bulk_edit(false), + current(-1), queued(-1) { } ~playlist() { @@ -126,6 +140,9 @@ protected: void UpdateQueuedSong(PlayerControl &pc, const Song *prev); public: + void BeginBulk(); + void CommitBulk(PlayerControl &pc); + void Clear(PlayerControl &pc); /** diff --git a/src/PlaylistEdit.cxx b/src/PlaylistEdit.cxx index 3eea2491e..0dffd3d80 100644 --- a/src/PlaylistEdit.cxx +++ b/src/PlaylistEdit.cxx @@ -40,6 +40,12 @@ void playlist::OnModified() { + if (bulk_edit) { + /* postponed to CommitBulk() */ + bulk_modified = true; + return; + } + queue.IncrementVersion(); idle_add(IDLE_PLAYLIST); @@ -56,6 +62,35 @@ playlist::Clear(PlayerControl &pc) OnModified(); } +void +playlist::BeginBulk() +{ + assert(!bulk_edit); + + bulk_edit = true; + bulk_modified = false; +} + +void +playlist::CommitBulk(PlayerControl &pc) +{ + assert(bulk_edit); + + bulk_edit = false; + if (!bulk_modified) + return; + + if (queued < 0) + /* if no song was queued, UpdateQueuedSong() is being + ignored in "bulk" edit mode; now that we have + shuffled all new songs, we can pick a random one + (instead of always picking the first one that was + added) */ + UpdateQueuedSong(pc, nullptr); + + OnModified(); +} + PlaylistResult playlist::AppendFile(PlayerControl &pc, const char *path_utf8, unsigned *added_id) diff --git a/src/command/DatabaseCommands.cxx b/src/command/DatabaseCommands.cxx index b86cbdae7..a7d2467b8 100644 --- a/src/command/DatabaseCommands.cxx +++ b/src/command/DatabaseCommands.cxx @@ -30,6 +30,7 @@ #include "util/Error.hxx" #include "SongFilter.hxx" #include "protocol/Result.hxx" +#include "BulkEdit.hxx" #include #include @@ -92,6 +93,8 @@ handle_match_add(Client &client, int argc, char *argv[], bool fold_case) return CommandResult::ERROR; } + const ScopeBulkEdit bulk_edit(client.partition); + const DatabaseSelection selection("", true, &filter); Error error; return AddFromDatabase(client.partition, selection, error) diff --git a/src/command/PlaylistCommands.cxx b/src/command/PlaylistCommands.cxx index d178fa097..c4441293e 100644 --- a/src/command/PlaylistCommands.cxx +++ b/src/command/PlaylistCommands.cxx @@ -26,6 +26,7 @@ #include "PlaylistFile.hxx" #include "PlaylistVector.hxx" #include "PlaylistQueue.hxx" +#include "BulkEdit.hxx" #include "TimePrint.hxx" #include "Client.hxx" #include "protocol/ArgParser.hxx" @@ -67,6 +68,8 @@ handle_load(Client &client, int argc, char *argv[]) } else if (!check_range(client, &start_index, &end_index, argv[2])) return CommandResult::ERROR; + const ScopeBulkEdit bulk_edit(client.partition); + const PlaylistResult result = playlist_open_into_queue(argv[1], start_index, end_index, diff --git a/src/command/QueueCommands.cxx b/src/command/QueueCommands.cxx index a20b24132..a987e1bc9 100644 --- a/src/command/QueueCommands.cxx +++ b/src/command/QueueCommands.cxx @@ -28,6 +28,7 @@ #include "ClientFile.hxx" #include "Client.hxx" #include "Partition.hxx" +#include "BulkEdit.hxx" #include "protocol/ArgParser.hxx" #include "protocol/Result.hxx" #include "ls.hxx" @@ -73,6 +74,8 @@ handle_add(Client &client, gcc_unused int argc, char *argv[]) return print_playlist_result(client, result); } + const ScopeBulkEdit bulk_edit(client.partition); + const DatabaseSelection selection(uri, true); Error error; return AddFromDatabase(client.partition, selection, error) From 69bb086ba535bb9f2631240c3f1b206f807522e6 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 11 Jul 2014 21:18:37 +0200 Subject: [PATCH 5/9] decoder/audiofile: fix typo in comment --- src/decoder/AudiofileDecoderPlugin.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/decoder/AudiofileDecoderPlugin.cxx b/src/decoder/AudiofileDecoderPlugin.cxx index 3fb23bc20..ae76a4302 100644 --- a/src/decoder/AudiofileDecoderPlugin.cxx +++ b/src/decoder/AudiofileDecoderPlugin.cxx @@ -44,7 +44,7 @@ struct AudioFileInputStream { size_t Read(void *buffer, size_t size) { /* libaudiofile does not like partial reads at all, - and wil abort playback; therefore always force full + and will abort playback; therefore always force full reads */ return decoder_read_full(decoder, is, buffer, size) ? size From ca1a11493d5d5a8637530fa88b2a05794b92b456 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 11 Jul 2014 21:10:49 +0200 Subject: [PATCH 6/9] decoder/audiofile: log seek errors --- src/decoder/AudiofileDecoderPlugin.cxx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/decoder/AudiofileDecoderPlugin.cxx b/src/decoder/AudiofileDecoderPlugin.cxx index ae76a4302..9f097f90b 100644 --- a/src/decoder/AudiofileDecoderPlugin.cxx +++ b/src/decoder/AudiofileDecoderPlugin.cxx @@ -110,6 +110,7 @@ audiofile_file_seek(AFvirtualfile *vfile, AFfileoffset offset, int is_relative) Error error; if (is.LockSeek(offset, whence, error)) { + LogError(error, "Seek failed"); return is.GetOffset(); } else { return -1; From eb79d830517e9668fa31c5615083296ebdff04fe Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 11 Jul 2014 21:11:33 +0200 Subject: [PATCH 7/9] decoder/sndfile: log seek errors --- src/decoder/SndfileDecoderPlugin.cxx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/decoder/SndfileDecoderPlugin.cxx b/src/decoder/SndfileDecoderPlugin.cxx index 77b132962..4082f3e0f 100644 --- a/src/decoder/SndfileDecoderPlugin.cxx +++ b/src/decoder/SndfileDecoderPlugin.cxx @@ -44,8 +44,11 @@ sndfile_vio_seek(sf_count_t offset, int whence, void *user_data) { InputStream &is = *(InputStream *)user_data; - if (!is.LockSeek(offset, whence, IgnoreError())) + Error error; + if (!is.LockSeek(offset, whence, error)) { + LogError(error, "Seek failed"); return -1; + } return is.GetOffset(); } From 0ef843f138c3eef1119e393287ca484bf32f8738 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 11 Jul 2014 20:50:44 +0200 Subject: [PATCH 8/9] decoder/sndfile: use decoder_read() .. instead of InputStream::LockRead(). The former is cancellable. --- NEWS | 1 + src/decoder/SndfileDecoderPlugin.cxx | 38 ++++++++++++++++------------ 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/NEWS b/NEWS index 198ada6a9..1deecbe56 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,7 @@ ver 0.18.12 (not yet released) - audiofile: improve responsiveness - audiofile: fix WAV stream playback - dsdiff, dsf: fix stream playback + - sndfile: improve responsiveness * randomize next song when enabling "random" mode while not playing * randomize next song when adding to single-song queue diff --git a/src/decoder/SndfileDecoderPlugin.cxx b/src/decoder/SndfileDecoderPlugin.cxx index 4082f3e0f..5b2913c13 100644 --- a/src/decoder/SndfileDecoderPlugin.cxx +++ b/src/decoder/SndfileDecoderPlugin.cxx @@ -31,10 +31,20 @@ static constexpr Domain sndfile_domain("sndfile"); +struct SndfileInputStream { + Decoder *const decoder; + InputStream &is; + + size_t Read(void *buffer, size_t size) { + return decoder_read(decoder, is, buffer, size); + } +}; + static sf_count_t sndfile_vio_get_filelen(void *user_data) { - const InputStream &is = *(const InputStream *)user_data; + SndfileInputStream &sis = *(SndfileInputStream *)user_data; + const InputStream &is = sis.is; return is.GetSize(); } @@ -42,7 +52,8 @@ sndfile_vio_get_filelen(void *user_data) static sf_count_t sndfile_vio_seek(sf_count_t offset, int whence, void *user_data) { - InputStream &is = *(InputStream *)user_data; + SndfileInputStream &sis = *(SndfileInputStream *)user_data; + InputStream &is = sis.is; Error error; if (!is.LockSeek(offset, whence, error)) { @@ -56,25 +67,18 @@ sndfile_vio_seek(sf_count_t offset, int whence, void *user_data) static sf_count_t sndfile_vio_read(void *ptr, sf_count_t count, void *user_data) { - InputStream &is = *(InputStream *)user_data; + SndfileInputStream &sis = *(SndfileInputStream *)user_data; sf_count_t total_bytes = 0; - Error error; /* this loop is necessary because libsndfile chokes on partial reads */ do { - size_t nbytes = is.LockRead((char *)ptr + total_bytes, - count - total_bytes, error); - if (nbytes == 0) { - if (error.IsDefined()) { - LogError(error); - return -1; - } - - break; - } + size_t nbytes = sis.Read((char *)ptr + total_bytes, + count - total_bytes); + if (nbytes == 0) + return -1; total_bytes += nbytes; } while (total_bytes < count); @@ -94,7 +98,8 @@ sndfile_vio_write(gcc_unused const void *ptr, static sf_count_t sndfile_vio_tell(void *user_data) { - const InputStream &is = *(const InputStream *)user_data; + SndfileInputStream &sis = *(SndfileInputStream *)user_data; + const InputStream &is = sis.is; return is.GetOffset(); } @@ -140,7 +145,8 @@ sndfile_stream_decode(Decoder &decoder, InputStream &is) info.format = 0; - sf = sf_open_virtual(&vio, SFM_READ, &info, &is); + SndfileInputStream sis{&decoder, is}; + sf = sf_open_virtual(&vio, SFM_READ, &info, &sis); if (sf == nullptr) { LogWarning(sndfile_domain, "sf_open_virtual() failed"); return; From ecb67a1ed16e93f5fe552b28631e517060115648 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 11 Jul 2014 21:17:43 +0200 Subject: [PATCH 9/9] decoder/sndfile: use decoder_read_full() Replaces the loop in sndfile_vio_read(), eliminating duplicate and fragile code. --- src/decoder/SndfileDecoderPlugin.cxx | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/src/decoder/SndfileDecoderPlugin.cxx b/src/decoder/SndfileDecoderPlugin.cxx index 5b2913c13..bcdf6d7ca 100644 --- a/src/decoder/SndfileDecoderPlugin.cxx +++ b/src/decoder/SndfileDecoderPlugin.cxx @@ -36,7 +36,11 @@ struct SndfileInputStream { InputStream &is; size_t Read(void *buffer, size_t size) { - return decoder_read(decoder, is, buffer, size); + /* libsndfile chokes on partial reads; therefore + always force full reads */ + return decoder_read_full(decoder, is, buffer, size) + ? size + : 0; } }; @@ -69,21 +73,7 @@ sndfile_vio_read(void *ptr, sf_count_t count, void *user_data) { SndfileInputStream &sis = *(SndfileInputStream *)user_data; - sf_count_t total_bytes = 0; - - /* this loop is necessary because libsndfile chokes on partial - reads */ - - do { - size_t nbytes = sis.Read((char *)ptr + total_bytes, - count - total_bytes); - if (nbytes == 0) - return -1; - - total_bytes += nbytes; - } while (total_bytes < count); - - return total_bytes; + return sis.Read(ptr, count); } static sf_count_t