Merge tag 'v0.22.5'

release v0.22.5

11 files changed, 127 insertions, 24 deletions

diff --git a/src/command/QueueCommands.cxx b/src/command/QueueCommands.cxx
index c77b62b3e..ba8c5a423 100644
--- a/src/command/QueueCommands.cxx
+++ b/src/command/QueueCommands.cxx
@@ -326,6 +326,11 @@ CommandResult
 handle_move(Client &client, Request args, [[maybe_unused]] Response &r)
 {
 	RangeArg range = args.ParseRange(0);
+	if (range.IsOpenEnded()) {
+		r.Error(ACK_ERROR_ARG, "Open-ended range not supported");
+		return CommandResult::ERROR;
+	}
+
 	int to = args.ParseInt(1);
 	client.GetPartition().MoveRange(range.start, range.end, to);
 	return CommandResult::OK;
diff --git a/src/db/plugins/simple/ExportedSong.hxx b/src/db/plugins/simple/ExportedSong.hxx
index 886be078b..37e4109b5 100644
--- a/src/db/plugins/simple/ExportedSong.hxx
+++ b/src/db/plugins/simple/ExportedSong.hxx
@@ -37,6 +37,15 @@ public:
 	ExportedSong(const char *_uri, Tag &&_tag) noexcept
 		:LightSong(_uri, tag_buffer),
 		 tag_buffer(std::move(_tag)) {}
+
+	/* this custom move constructor is necessary so LightSong::tag
+	   points to this instance's #Tag field instead of leaving a
+	   dangling reference to the source object's #Tag field */
+	ExportedSong(ExportedSong &&src) noexcept
+		:LightSong(src, tag_buffer),
+		 tag_buffer(std::move(src.tag_buffer)) {}
+
+	ExportedSong &operator=(ExportedSong &&) = delete;
 };
 
 #endif
diff --git a/src/output/Control.hxx b/src/output/Control.hxx
index ba61ced6e..734b29e67 100644
--- a/src/output/Control.hxx
+++ b/src/output/Control.hxx
@@ -182,6 +182,14 @@ class AudioOutputControl {
 	bool open = false;
 
 	/**
+	 * Is the device currently playing, i.e. is its buffer
+	 * (likely) non-empty?  If not, then it will never be drained.
+	 *
+	 * This field is only valid while the output is open.
+	 */
+	bool playing;
+
+	/**
 	 * Is the device paused?  i.e. the output thread is in the
 	 * ao_pause() loop.
 	 */
diff --git a/src/output/Thread.cxx b/src/output/Thread.cxx
index 3da07c6b2..c0aed30b0 100644
--- a/src/output/Thread.cxx
+++ b/src/output/Thread.cxx
@@ -53,7 +53,7 @@ AudioOutputControl::InternalOpen2(const AudioFormat in_audio_format)
 	if (open && cf != output->filter_audio_format)
 		/* if the filter's output format changes, the output
 		   must be reopened as well */
-		InternalCloseOutput(true);
+		InternalCloseOutput(playing);
 
 	output->filter_audio_format = cf;
 
@@ -64,6 +64,7 @@ AudioOutputControl::InternalOpen2(const AudioFormat in_audio_format)
 		}
 
 		open = true;
+		playing = false;
 	} else if (in_audio_format != output->out_audio_format) {
 		/* reconfigure the final ConvertFilter for its new
 		   input AudioFormat */
@@ -285,6 +286,9 @@ AudioOutputControl::PlayChunk(std::unique_lock<Mutex> &lock) noexcept
 		assert(nbytes % output->out_audio_format.GetFrameSize() == 0);
 
 		source.ConsumeData(nbytes);
+
+		/* there's data to be drained from now on */
+		playing = true;
 	}
 
 	return true;
@@ -371,6 +375,9 @@ AudioOutputControl::InternalPause(std::unique_lock<Mutex> &lock) noexcept
 	}
 
 	skip_delay = true;
+
+	/* ignore drain commands until we got something new to play */
+	playing = false;
 }
 
 static void
@@ -390,6 +397,10 @@ PlayFull(FilteredAudioOutput &output, ConstBuffer<void> _buffer)
 inline void
 AudioOutputControl::InternalDrain() noexcept
 {
+	/* after this method finishes, there's nothing left to be
+	   drained */
+	playing = false;
+
 	try {
 		/* flush the filter and play its remaining output */
 
@@ -518,6 +529,7 @@ AudioOutputControl::Task() noexcept
 			source.Cancel();
 
 			if (open) {
+				playing = false;
 				const ScopeUnlock unlock(mutex);
 				output->Cancel();
 			}
diff --git a/src/output/plugins/PulseOutputPlugin.cxx b/src/output/plugins/PulseOutputPlugin.cxx
index 5c03c8428..ab1e8c551 100644
--- a/src/output/plugins/PulseOutputPlugin.cxx
+++ b/src/output/plugins/PulseOutputPlugin.cxx
@@ -56,8 +56,6 @@ class PulseOutput final : AudioOutput {
 
 	size_t writable;
 
-	bool pause;
-
 	/**
 	 * Was Interrupt() called?  This will unblock Play().  It will
 	 * be reset by Cancel() and Pause(), as documented by the
@@ -113,6 +111,7 @@ public:
 
 	[[nodiscard]] std::chrono::steady_clock::duration Delay() const noexcept override;
 	size_t Play(const void *chunk, size_t size) override;
+	void Drain() override;
 	void Cancel() noexcept override;
 	bool Pause() override;
 
@@ -688,7 +687,6 @@ PulseOutput::Open(AudioFormat &audio_format)
 				     "pa_stream_connect_playback() has failed");
 	}
 
-	pause = false;
 	interrupted = false;
 }
 
@@ -699,17 +697,6 @@ PulseOutput::Close() noexcept
 
 	Pulse::LockGuard lock(mainloop);
 
-	if (pa_stream_get_state(stream) == PA_STREAM_READY) {
-		pa_operation *o =
-			pa_stream_drain(stream,
-					pulse_output_stream_success_cb, this);
-		if (o == nullptr) {
-			LogPulseError(context,
-				      "pa_stream_drain() has failed");
-		} else
-			pulse_wait_for_operation(mainloop, o);
-	}
-
 	DeleteStream();
 
 	if (context != nullptr &&
@@ -780,7 +767,7 @@ PulseOutput::Delay() const noexcept
 	Pulse::LockGuard lock(mainloop);
 
 	auto result = std::chrono::steady_clock::duration::zero();
-	if (pause && pa_stream_is_corked(stream) &&
+	if (pa_stream_is_corked(stream) &&
 	    pa_stream_get_state(stream) == PA_STREAM_READY)
 		/* idle while paused */
 		result = std::chrono::seconds(1);
@@ -796,8 +783,6 @@ PulseOutput::Play(const void *chunk, size_t size)
 
 	Pulse::LockGuard lock(mainloop);
 
-	pause = false;
-
 	/* check if the stream is (already) connected */
 
 	WaitStream();
@@ -841,6 +826,25 @@ PulseOutput::Play(const void *chunk, size_t size)
 }
 
 void
+PulseOutput::Drain()
+{
+	Pulse::LockGuard lock(mainloop);
+
+	if (pa_stream_get_state(stream) != PA_STREAM_READY ||
+	    pa_stream_is_suspended(stream) ||
+	    pa_stream_is_corked(stream))
+		return;
+
+	pa_operation *o =
+		pa_stream_drain(stream,
+				pulse_output_stream_success_cb, this);
+	if (o == nullptr)
+		throw MakePulseError(context, "pa_stream_drain() failed");
+
+	pulse_wait_for_operation(mainloop, o);
+}
+
+void
 PulseOutput::Cancel() noexcept
 {
 	assert(mainloop != nullptr);
@@ -876,7 +880,6 @@ PulseOutput::Pause()
 
 	Pulse::LockGuard lock(mainloop);
 
-	pause = true;
 	interrupted = false;
 
 	/* check if the stream is (already/still) connected */
diff --git a/src/output/plugins/WasapiOutputPlugin.cxx b/src/output/plugins/WasapiOutputPlugin.cxx
index 0a114c9c3..a6c1d483b 100644
--- a/src/output/plugins/WasapiOutputPlugin.cxx
+++ b/src/output/plugins/WasapiOutputPlugin.cxx
@@ -24,6 +24,7 @@
 #include "mixer/MixerList.hxx"
 #include "thread/Cond.hxx"
 #include "thread/Mutex.hxx"
+#include "thread/Name.hxx"
 #include "thread/Thread.hxx"
 #include "util/AllocatedString.hxx"
 #include "util/Domain.hxx"
@@ -231,6 +232,7 @@ IAudioClient *wasapi_output_get_client(WasapiOutput &output) noexcept {
 }
 
 void WasapiOutputThread::Work() noexcept {
+	SetThreadName("Wasapi Output Worker");
 	FormatDebug(wasapi_output_domain, "Working thread started");
 	try {
 		com.emplace();
diff --git a/src/protocol/ArgParser.cxx b/src/protocol/ArgParser.cxx
index 98cf74150..b5d05b473 100644
--- a/src/protocol/ArgParser.cxx
+++ b/src/protocol/ArgParser.cxx
@@ -94,7 +94,7 @@ ParseCommandArgRange(const char *s)
 						  s);
 
 		if (test == test2)
-			value = std::numeric_limits<int>::max();
+			return RangeArg::OpenEnded(range.start);
 
 		if (value < 0)
 			throw FormatProtocolError(ACK_ERROR_ARG,
@@ -107,9 +107,13 @@ ParseCommandArgRange(const char *s)
 
 		range.end = (unsigned)value;
 	} else {
-		range.end = (unsigned)value + 1;
+		return RangeArg::Single(range.start);
 	}
 
+	if (!range.IsWellFormed())
+		throw FormatProtocolError(ACK_ERROR_ARG,
+					  "Malformed range: %s", s);
+
 	return range;
 }
 
diff --git a/src/protocol/RangeArg.hxx b/src/protocol/RangeArg.hxx
index a116920b1..347fa0aea 100644
--- a/src/protocol/RangeArg.hxx
+++ b/src/protocol/RangeArg.hxx
@@ -25,8 +25,22 @@
 struct RangeArg {
 	unsigned start, end;
 
-	static constexpr RangeArg All() {
-		return { 0, std::numeric_limits<unsigned>::max() };
+	/**
+	 * Construct an open-ended range starting at the given index.
+	 */
+	static constexpr RangeArg OpenEnded(unsigned start) noexcept {
+		return { start, std::numeric_limits<unsigned>::max() };
+	}
+
+	static constexpr RangeArg All() noexcept {
+		return OpenEnded(0);
+	}
+
+	/**
+	 * Construct an instance describing exactly one index.
+	 */
+	static constexpr RangeArg Single(unsigned i) noexcept {
+		return { i, i + 1 };
 	}
 
 	constexpr bool operator==(RangeArg other) const noexcept {
@@ -37,13 +51,45 @@ struct RangeArg {
 		return !(*this == other);
 	}
 
+	constexpr bool IsOpenEnded() const noexcept {
+		return end == All().end;
+	}
+
 	constexpr bool IsAll() const noexcept {
 		return *this == All();
 	}
 
+	constexpr bool IsWellFormed() const noexcept {
+		return start <= end;
+	}
+
+	/**
+	 * Is this range empty?  A malformed range also counts as
+	 * "empty" for this method.
+	 */
+	constexpr bool IsEmpty() const noexcept {
+		return start >= end;
+	}
+
+	/**
+	 * Check if the range contains at least this number of items.
+	 * Unlike Count(), this allows the object to be malformed.
+	 */
+	constexpr bool HasAtLeast(unsigned n) const noexcept {
+		return start + n <= end;
+	}
+
 	constexpr bool Contains(unsigned i) const noexcept {
 		return i >= start && i < end;
 	}
+
+	/**
+	 * Count the number of items covered by this range.  This requires the
+	 * object to be well-formed.
+	 */
+	constexpr unsigned Count() const noexcept {
+		return end - start;
+	}
 };
 
 #endif
diff --git a/src/song/LightSong.hxx b/src/song/LightSong.hxx
index ee372576a..06f0a41f9 100644
--- a/src/song/LightSong.hxx
+++ b/src/song/LightSong.hxx
@@ -88,6 +88,19 @@ struct LightSong {
 	LightSong(const char *_uri, const Tag &_tag) noexcept
 		:uri(_uri), tag(_tag) {}
 
+	/**
+	 * A copy constructor which copies all fields, but only sets
+	 * the tag to a caller-provided reference.  This is used by
+	 * the #ExportedSong move constructor.
+	 */
+	LightSong(const LightSong &src, const Tag &_tag) noexcept
+		:directory(src.directory), uri(src.uri),
+		 real_uri(src.real_uri),
+		 tag(_tag),
+		 mtime(src.mtime),
+		 start_time(src.start_time), end_time(src.end_time),
+		 audio_format(src.audio_format) {}
+
 	gcc_pure
 	std::string GetURI() const noexcept {
 		if (directory == nullptr)
diff --git a/src/win32/ComPtr.hxx b/src/win32/ComPtr.hxx
index 6f0f37050..325c06c10 100644
--- a/src/win32/ComPtr.hxx
+++ b/src/win32/ComPtr.hxx
@@ -112,6 +112,6 @@ template <typename T>
 void swap(ComPtr<T> &lhs, ComPtr<T> &rhs) noexcept {
 	lhs.swap(rhs);
 }
-}
+} // namespace std
 
 #endif
diff --git a/src/win32/HResult.hxx b/src/win32/HResult.hxx
index a4fbcb8b6..2bc851f73 100644
--- a/src/win32/HResult.hxx
+++ b/src/win32/HResult.hxx
@@ -59,6 +59,7 @@ case x:
 		C(E_INVALIDARG);
 		C(E_OUTOFMEMORY);
 		C(E_POINTER);
+		C(NO_ERROR);
 #undef C
 	}
 	return std::string_view();