PcmConvert: add methods Open(), Close()

Replaces Reset() and eliminates the AudioFormat parameters from the
Convert() method.

9 files changed, 151 insertions, 74 deletions

diff --git a/src/DecoderAPI.cxx b/src/DecoderAPI.cxx
index 7b850265b..1494e3f80 100644
--- a/src/DecoderAPI.cxx
+++ b/src/DecoderAPI.cxx
@@ -71,6 +71,12 @@ decoder_initialized(Decoder &decoder,
 						   &af_string));
 
 		decoder.convert = new PcmConvert();
+
+		Error error;
+		if (!decoder.convert->Open(dc.in_audio_format,
+					   dc.out_audio_format,
+					   error))
+			decoder.error = std::move(error);
 	}
 
 	dc.Lock();
@@ -401,9 +407,7 @@ decoder_data(Decoder &decoder,
 		assert(dc.in_audio_format != dc.out_audio_format);
 
 		Error error;
-		data = decoder.convert->Convert(dc.in_audio_format,
-						data, length,
-						dc.out_audio_format,
+		data = decoder.convert->Convert(data, length,
 						&length,
 						error);
 		if (data == nullptr) {
diff --git a/src/DecoderInternal.cxx b/src/DecoderInternal.cxx
index 26f91b2d9..38e54a36c 100644
--- a/src/DecoderInternal.cxx
+++ b/src/DecoderInternal.cxx
@@ -33,7 +33,11 @@ Decoder::~Decoder()
 	/* caller must flush the chunk */
 	assert(chunk == nullptr);
 
-	delete convert;
+	if (convert != nullptr) {
+		convert->Close();
+		delete convert;
+	}
+
 	delete song_tag;
 	delete stream_tag;
 	delete decoder_tag;
diff --git a/src/OutputThread.cxx b/src/OutputThread.cxx
index 30d3ba30f..dc81643f3 100644
--- a/src/OutputThread.cxx
+++ b/src/OutputThread.cxx
@@ -184,7 +184,15 @@ ao_open(struct audio_output *ao)
 		return;
 	}
 
-	convert_filter_set(ao->convert_filter, ao->out_audio_format);
+	if (!convert_filter_set(ao->convert_filter, ao->out_audio_format,
+				error)) {
+		FormatError(error, "Failed to convert for \"%s\" [%s]",
+			    ao->name, ao->plugin->name);
+
+		ao_filter_close(ao);
+		ao->fail_timer = g_timer_new();
+		return;
+	}
 
 	ao->open = true;
 
@@ -233,7 +241,9 @@ ao_reopen_filter(struct audio_output *ao)
 	ao_filter_close(ao);
 	const AudioFormat filter_audio_format =
 		ao_filter_open(ao, ao->in_audio_format, error);
-	if (!filter_audio_format.IsDefined()) {
+	if (!filter_audio_format.IsDefined() ||
+	    !convert_filter_set(ao->convert_filter, ao->out_audio_format,
+				error)) {
 		FormatError(error,
 			    "Failed to open filter for \"%s\" [%s]",
 			    ao->name, ao->plugin->name);
@@ -254,8 +264,6 @@ ao_reopen_filter(struct audio_output *ao)
 
 		return;
 	}
-
-	convert_filter_set(ao->convert_filter, ao->out_audio_format);
 }
 
 static void
diff --git a/src/filter/AutoConvertFilterPlugin.cxx b/src/filter/AutoConvertFilterPlugin.cxx
index 918a16e53..44adc8a66 100644
--- a/src/filter/AutoConvertFilterPlugin.cxx
+++ b/src/filter/AutoConvertFilterPlugin.cxx
@@ -88,7 +88,11 @@ AutoConvertFilter::Open(AudioFormat &in_audio_format, Error &error)
 
 		assert(audio_format2 == in_audio_format);
 
-		convert_filter_set(convert, child_audio_format);
+		if (!convert_filter_set(convert, child_audio_format, error)) {
+			delete convert;
+			filter->Close();
+			return AudioFormat::Undefined();
+		}
 	} else
 		/* no */
 		convert = nullptr;
diff --git a/src/filter/ConvertFilterPlugin.cxx b/src/filter/ConvertFilterPlugin.cxx
index 040f8426f..416b6e81a 100644
--- a/src/filter/ConvertFilterPlugin.cxx
+++ b/src/filter/ConvertFilterPlugin.cxx
@@ -39,21 +39,18 @@ class ConvertFilter final : public Filter {
 
 	/**
 	 * The output audio format; the consumer of this plugin
-	 * expects PCM data in this format.  This defaults to
-	 * #in_audio_format, and can be set with convert_filter_set().
+	 * expects PCM data in this format.
+	 *
+	 * If this is AudioFormat::Undefined(), then the #PcmConvert
+	 * attribute is not open.  This can mean that Set() has failed
+	 * or that no conversion is necessary.
 	 */
 	AudioFormat out_audio_format;
 
 	Manual<PcmConvert> state;
 
 public:
-	void Set(const AudioFormat &_out_audio_format) {
-		assert(in_audio_format.IsValid());
-		assert(out_audio_format.IsValid());
-		assert(_out_audio_format.IsValid());
-
-		out_audio_format = _out_audio_format;
-	}
+	bool Set(const AudioFormat &_out_audio_format, Error &error);
 
 	virtual AudioFormat Open(AudioFormat &af, Error &error) override;
 	virtual void Close() override;
@@ -69,12 +66,40 @@ convert_filter_init(gcc_unused const config_param &param,
 	return new ConvertFilter();
 }
 
+bool
+ConvertFilter::Set(const AudioFormat &_out_audio_format, Error &error)
+{
+	assert(in_audio_format.IsValid());
+	assert(_out_audio_format.IsValid());
+
+	if (_out_audio_format == out_audio_format)
+		/* no change */
+		return true;
+
+	if (out_audio_format.IsValid()) {
+		out_audio_format.Clear();
+		state->Close();
+	}
+
+	if (_out_audio_format == in_audio_format)
+		/* optimized special case: no-op */
+		return true;
+
+	if (!state->Open(in_audio_format, _out_audio_format, error))
+		return false;
+
+	out_audio_format = _out_audio_format;
+	return true;
+}
+
 AudioFormat
 ConvertFilter::Open(AudioFormat &audio_format, gcc_unused Error &error)
 {
 	assert(audio_format.IsValid());
 
-	in_audio_format = out_audio_format = audio_format;
+	in_audio_format = audio_format;
+	out_audio_format.Clear();
+
 	state.Construct();
 
 	return in_audio_format;
@@ -83,6 +108,11 @@ ConvertFilter::Open(AudioFormat &audio_format, gcc_unused Error &error)
 void
 ConvertFilter::Close()
 {
+	assert(in_audio_format.IsValid());
+
+	if (out_audio_format.IsValid())
+		state->Close();
+
 	state.Destruct();
 
 	poison_undefined(&in_audio_format, sizeof(in_audio_format));
@@ -93,15 +123,15 @@ const void *
 ConvertFilter::FilterPCM(const void *src, size_t src_size,
 			 size_t *dest_size_r, Error &error)
 {
-	if (in_audio_format == out_audio_format) {
+	assert(in_audio_format.IsValid());
+
+	if (!out_audio_format.IsValid()) {
 		/* optimized special case: no-op */
 		*dest_size_r = src_size;
 		return src;
 	}
 
-	return state->Convert(in_audio_format,
-			      src, src_size,
-			      out_audio_format, dest_size_r,
+	return state->Convert(src, src_size, dest_size_r,
 			      error);
 }
 
@@ -110,10 +140,11 @@ const struct filter_plugin convert_filter_plugin = {
 	convert_filter_init,
 };
 
-void
-convert_filter_set(Filter *_filter, const AudioFormat out_audio_format)
+bool
+convert_filter_set(Filter *_filter, AudioFormat out_audio_format,
+		   Error &error)
 {
 	ConvertFilter *filter = (ConvertFilter *)_filter;
 
-	filter->Set(out_audio_format);
+	return filter->Set(out_audio_format, error);
 }
diff --git a/src/filter/ConvertFilterPlugin.hxx b/src/filter/ConvertFilterPlugin.hxx
index c814aaf49..f414e59bf 100644
--- a/src/filter/ConvertFilterPlugin.hxx
+++ b/src/filter/ConvertFilterPlugin.hxx
@@ -21,6 +21,7 @@
 #define MPD_CONVERT_FILTER_PLUGIN_HXX
 
 class Filter;
+class Error;
 struct AudioFormat;
 
 /**
@@ -29,7 +30,8 @@ struct AudioFormat;
  * format switch is a violation of the filter API, this filter must be
  * the last in a chain.
  */
-void
-convert_filter_set(Filter *filter, AudioFormat out_audio_format);
+bool
+convert_filter_set(Filter *filter, AudioFormat out_audio_format,
+		   Error &error);
 
 #endif
diff --git a/src/pcm/PcmConvert.cxx b/src/pcm/PcmConvert.cxx
index 8eafe527c..cca2f701e 100644
--- a/src/pcm/PcmConvert.cxx
+++ b/src/pcm/PcmConvert.cxx
@@ -32,23 +32,48 @@ const Domain pcm_convert_domain("pcm_convert");
 
 PcmConvert::PcmConvert()
 {
+#ifndef NDEBUG
+	src_format.Clear();
+	dest_format.Clear();
+#endif
 }
 
 PcmConvert::~PcmConvert()
 {
+	assert(!src_format.IsValid());
+	assert(!dest_format.IsValid());
+}
+
+bool
+PcmConvert::Open(AudioFormat _src_format, AudioFormat _dest_format,
+		 gcc_unused Error &error)
+{
+	assert(!src_format.IsValid());
+	assert(!dest_format.IsValid());
+	assert(_src_format.IsValid());
+	assert(_dest_format.IsValid());
+
+	src_format = _src_format;
+	dest_format = _dest_format;
+
+	return true;
 }
 
 void
-PcmConvert::Reset()
+PcmConvert::Close()
 {
 	dsd.Reset();
 	resampler.Reset();
+
+#ifndef NDEBUG
+	src_format.Clear();
+	dest_format.Clear();
+#endif
 }
 
 inline const int16_t *
-PcmConvert::Convert16(const AudioFormat src_format,
-		      const void *src_buffer, size_t src_size,
-		      const AudioFormat dest_format, size_t *dest_size_r,
+PcmConvert::Convert16(const void *src_buffer, size_t src_size,
+		      size_t *dest_size_r,
 		      Error &error)
 {
 	const int16_t *buf;
@@ -96,9 +121,8 @@ PcmConvert::Convert16(const AudioFormat src_format,
 }
 
 inline const int32_t *
-PcmConvert::Convert24(const AudioFormat src_format,
-		      const void *src_buffer, size_t src_size,
-		      const AudioFormat dest_format, size_t *dest_size_r,
+PcmConvert::Convert24(const void *src_buffer, size_t src_size,
+		      size_t *dest_size_r,
 		      Error &error)
 {
 	const int32_t *buf;
@@ -145,9 +169,8 @@ PcmConvert::Convert24(const AudioFormat src_format,
 }
 
 inline const int32_t *
-PcmConvert::Convert32(const AudioFormat src_format,
-		      const void *src_buffer, size_t src_size,
-		      const AudioFormat dest_format, size_t *dest_size_r,
+PcmConvert::Convert32(const void *src_buffer, size_t src_size,
+		      size_t *dest_size_r,
 		      Error &error)
 {
 	const int32_t *buf;
@@ -194,9 +217,8 @@ PcmConvert::Convert32(const AudioFormat src_format,
 }
 
 inline const float *
-PcmConvert::ConvertFloat(const AudioFormat src_format,
-			 const void *src_buffer, size_t src_size,
-			 const AudioFormat dest_format, size_t *dest_size_r,
+PcmConvert::ConvertFloat(const void *src_buffer, size_t src_size,
+			 size_t *dest_size_r,
 			 Error &error)
 {
 	const float *buffer = (const float *)src_buffer;
@@ -251,9 +273,7 @@ PcmConvert::ConvertFloat(const AudioFormat src_format,
 }
 
 const void *
-PcmConvert::Convert(AudioFormat src_format,
-		    const void *src, size_t src_size,
-		    const AudioFormat dest_format,
+PcmConvert::Convert(const void *src, size_t src_size,
 		    size_t *dest_size_r,
 		    Error &error)
 {
@@ -279,23 +299,23 @@ PcmConvert::Convert(AudioFormat src_format,
 
 	switch (dest_format.format) {
 	case SampleFormat::S16:
-		return Convert16(src_format, src, src_size,
-				 dest_format, dest_size_r,
+		return Convert16(src, src_size,
+				 dest_size_r,
 				 error);
 
 	case SampleFormat::S24_P32:
-		return Convert24(src_format, src, src_size,
-				 dest_format, dest_size_r,
+		return Convert24(src, src_size,
+				 dest_size_r,
 				 error);
 
 	case SampleFormat::S32:
-		return Convert32(src_format, src, src_size,
-				 dest_format, dest_size_r,
+		return Convert32(src, src_size,
+				 dest_size_r,
 				 error);
 
 	case SampleFormat::FLOAT:
-		return ConvertFloat(src_format, src, src_size,
-				    dest_format, dest_size_r,
+		return ConvertFloat(src, src_size,
+				    dest_size_r,
 				    error);
 
 	default:
diff --git a/src/pcm/PcmConvert.hxx b/src/pcm/PcmConvert.hxx
index 40f785179..4bace1d36 100644
--- a/src/pcm/PcmConvert.hxx
+++ b/src/pcm/PcmConvert.hxx
@@ -24,10 +24,10 @@
 #include "PcmDsd.hxx"
 #include "PcmResample.hxx"
 #include "PcmBuffer.hxx"
+#include "AudioFormat.hxx"
 
 #include <stddef.h>
 
-struct AudioFormat;
 class Error;
 
 /**
@@ -48,17 +48,23 @@ class PcmConvert {
 	/** the buffer for converting the channel count */
 	PcmBuffer channels_buffer;
 
+	AudioFormat src_format, dest_format;
+
 public:
 	PcmConvert();
 	~PcmConvert();
 
+	/**
+	 * Prepare the object.  Call Close() when done.
+	 */
+	bool Open(AudioFormat _src_format, AudioFormat _dest_format,
+		  Error &error);
 
 	/**
-	 * Reset the pcm_convert_state object.  Use this at the
-	 * boundary between two distinct songs and each time the
-	 * format changes.
+	 * Close the object after it was prepared with Open().  After
+	 * that, it may be reused by calling Open() again.
 	 */
-	void Reset();
+	void Close();
 
 	/**
 	 * Converts PCM data between two audio formats.
@@ -72,34 +78,24 @@ public:
 	 * ignore errors
 	 * @return the destination buffer, or NULL on error
 	 */
-	const void *Convert(AudioFormat src_format,
-			    const void *src, size_t src_size,
-			    AudioFormat dest_format,
+	const void *Convert(const void *src, size_t src_size,
 			    size_t *dest_size_r,
 			    Error &error);
 
 private:
-	const int16_t *Convert16(AudioFormat src_format,
-				 const void *src_buffer, size_t src_size,
-				 AudioFormat dest_format,
+	const int16_t *Convert16(const void *src_buffer, size_t src_size,
 				 size_t *dest_size_r,
 				 Error &error);
 
-	const int32_t *Convert24(AudioFormat src_format,
-				 const void *src_buffer, size_t src_size,
-				 AudioFormat dest_format,
+	const int32_t *Convert24(const void *src_buffer, size_t src_size,
 				 size_t *dest_size_r,
 				 Error &error);
 
-	const int32_t *Convert32(AudioFormat src_format,
-				 const void *src_buffer, size_t src_size,
-				 AudioFormat dest_format,
+	const int32_t *Convert32(const void *src_buffer, size_t src_size,
 				 size_t *dest_size_r,
 				 Error &error);
 
-	const float *ConvertFloat(AudioFormat src_format,
-				  const void *src_buffer, size_t src_size,
-				  AudioFormat dest_format,
+	const float *ConvertFloat(const void *src_buffer, size_t src_size,
 				  size_t *dest_size_r,
 				  Error &error);
 };
diff --git a/test/run_convert.cxx b/test/run_convert.cxx
index 0e873a3b3..d9a1b7f08 100644
--- a/test/run_convert.cxx
+++ b/test/run_convert.cxx
@@ -90,6 +90,11 @@ int main(int argc, char **argv)
 	const size_t in_frame_size = in_audio_format.GetFrameSize();
 
 	PcmConvert state;
+	if (!state.Open(in_audio_format, out_audio_format_mask, error)) {
+		g_printerr("Failed to open PcmConvert: %s\n",
+			   error.GetMessage());
+		return EXIT_FAILURE;
+	}
 
 	FifoBuffer<uint8_t, 4096> buffer;
 
@@ -115,9 +120,10 @@ int main(int argc, char **argv)
 		buffer.Consume(src.size);
 
 		size_t length;
-		output = state.Convert(in_audio_format, src.data, src.size,
-				       out_audio_format, &length, error);
+		output = state.Convert(src.data, src.size,
+				       &length, error);
 		if (output == NULL) {
+			state.Close();
 			g_printerr("Failed to convert: %s\n", error.GetMessage());
 			return 2;
 		}
@@ -125,5 +131,7 @@ int main(int argc, char **argv)
 		gcc_unused ssize_t ignored = write(1, output, length);
 	}
 
+	state.Close();
+
 	return EXIT_SUCCESS;
 }