Merge branch 'v0.21.x'

8 files changed, 200 insertions, 89 deletions

diff --git a/NEWS b/NEWS
index 2ca947caa..3fad71c81 100644
--- a/NEWS
+++ b/NEWS
@@ -51,6 +51,7 @@ ver 0.21.25 (not yet released)
   - opus: fix memory leak
 * output
   - osx: improve sample rate selection
+  - osx: fix noise while stopping
 * Windows/Android:
   - fix Boost detection after breaking change in Meson 0.54
 
diff --git a/meson.build b/meson.build
index 31f1837d7..f01e078de 100644
--- a/meson.build
+++ b/meson.build
@@ -350,6 +350,8 @@ subdir('src/thread')
 subdir('src/net')
 subdir('src/event')
 
+subdir('src/apple')
+
 subdir('src/lib/dbus')
 subdir('src/lib/icu')
 subdir('src/lib/smbclient')
diff --git a/src/apple/AudioObject.cxx b/src/apple/AudioObject.cxx
new file mode 100644
index 000000000..f81f82e2f
--- /dev/null
+++ b/src/apple/AudioObject.cxx
@@ -0,0 +1,40 @@
+/*
+ * Copyright 2020 Max Kellermann <max.kellermann@gmail.com>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the
+ * distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL THE
+ * FOUNDATION OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
+ * OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "AudioObject.hxx"
+#include "StringRef.hxx"
+
+Apple::StringRef
+AudioObjectGetStringProperty(AudioObjectID inObjectID,
+			     const AudioObjectPropertyAddress &inAddress)
+{
+	auto s = AudioObjectGetPropertyDataT<CFStringRef>(inObjectID,
+							  inAddress);
+	return Apple::StringRef(s);
+}
diff --git a/src/apple/AudioObject.hxx b/src/apple/AudioObject.hxx
index 99b5b7bf2..ff0362dd4 100644
--- a/src/apple/AudioObject.hxx
+++ b/src/apple/AudioObject.hxx
@@ -37,7 +37,11 @@
 
 #include <cstddef>
 
-std::size_t
+namespace Apple {
+class StringRef;
+}
+
+inline std::size_t
 AudioObjectGetPropertyDataSize(AudioObjectID inObjectID,
 			       const AudioObjectPropertyAddress &inAddress)
 {
@@ -69,6 +73,10 @@ AudioObjectGetPropertyDataT(AudioObjectID inObjectID,
 	return value;
 }
 
+Apple::StringRef
+AudioObjectGetStringProperty(AudioObjectID inObjectID,
+			     const AudioObjectPropertyAddress &inAddress);
+
 template<typename T>
 AllocatedArray<T>
 AudioObjectGetPropertyDataArray(AudioObjectID inObjectID,
diff --git a/src/apple/AudioUnit.hxx b/src/apple/AudioUnit.hxx
index bc6f72b53..c4a5b2ef7 100644
--- a/src/apple/AudioUnit.hxx
+++ b/src/apple/AudioUnit.hxx
@@ -52,4 +52,60 @@ AudioUnitGetPropertyT(AudioUnit inUnit, AudioUnitPropertyID inID,
 	return value;
 }
 
+template<typename T>
+void
+AudioUnitSetPropertyT(AudioUnit inUnit, AudioUnitPropertyID inID,
+		      AudioUnitScope inScope,
+		      AudioUnitElement inElement,
+		      const T &value)
+{
+	OSStatus status = AudioUnitSetProperty(inUnit, inID, inScope,
+					       inElement,
+					       &value, sizeof(value));
+	if (status != noErr)
+		Apple::ThrowOSStatus(status);
+}
+
+inline void
+AudioUnitSetCurrentDevice(AudioUnit inUnit, const AudioDeviceID &value)
+{
+	AudioUnitSetPropertyT(inUnit, kAudioOutputUnitProperty_CurrentDevice,
+			      kAudioUnitScope_Global, 0,
+			      value);
+}
+
+inline void
+AudioUnitSetInputStreamFormat(AudioUnit inUnit,
+			      const AudioStreamBasicDescription &value)
+{
+	AudioUnitSetPropertyT(inUnit, kAudioUnitProperty_StreamFormat,
+			      kAudioUnitScope_Input, 0,
+			      value);
+}
+
+inline void
+AudioUnitSetInputRenderCallback(AudioUnit inUnit,
+				const AURenderCallbackStruct &value)
+{
+	AudioUnitSetPropertyT(inUnit, kAudioUnitProperty_SetRenderCallback,
+			      kAudioUnitScope_Input, 0,
+			      value);
+}
+
+inline UInt32
+AudioUnitGetBufferFrameSize(AudioUnit inUnit)
+{
+	return AudioUnitGetPropertyT<UInt32>(inUnit,
+					     kAudioDevicePropertyBufferFrameSize,
+					     kAudioUnitScope_Global, 0);
+}
+
+inline void
+AudioUnitSetBufferFrameSize(AudioUnit inUnit, const UInt32 &value)
+{
+	AudioUnitSetPropertyT(inUnit, kAudioDevicePropertyBufferFrameSize,
+			      kAudioUnitScope_Global, 0,
+			      value);
+}
+
 #endif
diff --git a/src/apple/meson.build b/src/apple/meson.build
new file mode 100644
index 000000000..160c62bc4
--- /dev/null
+++ b/src/apple/meson.build
@@ -0,0 +1,28 @@
+if not is_darwin
+  apple_dep = dependency('', required: false)
+  subdir_done()
+endif
+
+audiounit_dep = declare_dependency(
+  link_args: ['-framework', 'AudioUnit', '-framework', 'CoreAudio', '-framework', 'CoreServices'],
+  dependencies: [
+    boost_dep,
+  ],
+)
+
+apple = static_library(
+  'apple',
+  'AudioObject.cxx',
+  'Throw.cxx',
+  include_directories: inc,
+  dependencies: [
+    audiounit_dep,
+  ],
+)
+
+apple_dep = declare_dependency(
+  link_with: apple,
+  dependencies: [
+    audiounit_dep,
+  ],
+)
diff --git a/src/output/plugins/OSXOutputPlugin.cxx b/src/output/plugins/OSXOutputPlugin.cxx
index ae2bdf6c7..f4c7f25a0 100644
--- a/src/output/plugins/OSXOutputPlugin.cxx
+++ b/src/output/plugins/OSXOutputPlugin.cxx
@@ -73,7 +73,14 @@ struct OSXOutput final : AudioOutput {
 	const char *device_name;
 	const char *const channel_map;
 	const bool hog_device;
+
 	bool pause;
+
+	/**
+	 * Is the audio unit "started", i.e. was AudioOutputUnitStart() called?
+	 */
+	bool started;
+
 #ifdef ENABLE_DSD
 	/**
 	 * Enable DSD over PCM according to the DoP standard?
@@ -450,24 +457,14 @@ osx_output_set_buffer_size(AudioUnit au, AudioStreamBasicDescription desc)
 									kAudioUnitScope_Global,
 									0);
 
-	UInt32 buffer_frame_size = value_range.mMaximum;
-	OSStatus err;
-	err = AudioUnitSetProperty(au,
-				   kAudioDevicePropertyBufferFrameSize,
-				   kAudioUnitScope_Global,
-				   0,
-				   &buffer_frame_size,
-				   sizeof(buffer_frame_size));
-	if (err != noErr)
-		FormatWarning(osx_output_domain,
-			      "Failed to set maximum buffer size: %d",
-			      err);
-
-	buffer_frame_size = AudioUnitGetPropertyT<UInt32>(au,
-							  kAudioDevicePropertyBufferFrameSize,
-							  kAudioUnitScope_Global,
-							  0);
+	try {
+		AudioUnitSetBufferFrameSize(au, value_range.mMaximum);
+	} catch (...) {
+		LogError(std::current_exception(),
+			 "Failed to set maximum buffer size");
+	}
 
+	auto buffer_frame_size = AudioUnitGetBufferFrameSize(au);
 	buffer_frame_size *= desc.mBytesPerFrame;
 
 	// We set the frame size to a power of two integer that
@@ -535,19 +532,15 @@ IsAudioDeviceName(AudioDeviceID id, const char *expected_name) noexcept
 		kAudioObjectPropertyElementMaster,
 	};
 
-	CFStringRef cfname;
-	UInt32 size = sizeof(cfname);
-
-	if (AudioObjectGetPropertyData(id, &aopa_name,
-				       0, nullptr,
-				       &size, &cfname) != noErr)
-		return false;
-
-	const Apple::StringRef cfname_(cfname);
-
 	char actual_name[256];
-	if (!cfname_.GetCString(actual_name, sizeof(actual_name)))
+
+	try {
+		auto cfname = AudioObjectGetStringProperty(id, aopa_name);
+		if (!cfname.GetCString(actual_name, sizeof(actual_name)))
+			return false;
+	} catch (...) {
 		return false;
+	}
 
 	return StringIsEqual(actual_name, expected_name);
 }
@@ -587,15 +580,7 @@ osx_output_set_device(OSXOutput *oo)
 		    "found matching device: ID=%u, name=%s",
 		    (unsigned)id, oo->device_name);
 
-	OSStatus status;
-	status = AudioUnitSetProperty(oo->au,
-				      kAudioOutputUnitProperty_CurrentDevice,
-				      kAudioUnitScope_Global,
-				      0,
-				      &id, sizeof(id));
-	if (status != noErr)
-		Apple::ThrowOSStatus(status,
-				     "Unable to set OS X audio output device");
+	AudioUnitSetCurrentDevice(oo->au, id);
 
 	oo->dev_id = id;
 	FormatDebug(osx_output_domain,
@@ -682,7 +667,8 @@ OSXOutput::Disable() noexcept
 void
 OSXOutput::Close() noexcept
 {
-	AudioOutputUnitStop(au);
+	if (started)
+		AudioOutputUnitStop(au);
 	AudioUnitUninitialize(au);
 	delete ring_buffer;
 }
@@ -745,29 +731,15 @@ OSXOutput::Open(AudioFormat &audio_format)
 	dop_enabled = params.dsd_mode == PcmExport::DsdMode::DOP;
 #endif
 
-	OSStatus status =
-		AudioUnitSetProperty(au, kAudioUnitProperty_StreamFormat,
-				     kAudioUnitScope_Input, 0,
-				     &asbd,
-				     sizeof(asbd));
-	if (status != noErr)
-		throw std::runtime_error("Unable to set format on OS X device");
+	AudioUnitSetInputStreamFormat(au, asbd);
 
 	AURenderCallbackStruct callback;
 	callback.inputProc = osx_render;
 	callback.inputProcRefCon = this;
 
-	status =
-		AudioUnitSetProperty(au,
-				     kAudioUnitProperty_SetRenderCallback,
-				     kAudioUnitScope_Input, 0,
-				     &callback, sizeof(callback));
-	if (status != noErr) {
-		AudioComponentInstanceDispose(au);
-		throw std::runtime_error("Unable to set callback for OS X audio unit");
-	}
+	AudioUnitSetInputRenderCallback(au, callback);
 
-	status = AudioUnitInitialize(au);
+	OSStatus status = AudioUnitInitialize(au);
 	if (status != noErr)
 		Apple::ThrowOSStatus(status, "Unable to initialize OS X audio unit");
 
@@ -785,36 +757,43 @@ OSXOutput::Open(AudioFormat &audio_format)
 #endif
 	ring_buffer = new boost::lockfree::spsc_queue<uint8_t>(ring_buffer_size);
 
-	status = AudioOutputUnitStart(au);
-	if (status != 0)
-		Apple::ThrowOSStatus(status, "Unable to start audio output");
-
 	pause = false;
+	started = false;
 }
 
 size_t
 OSXOutput::Play(const void *chunk, size_t size)
 {
 	assert(size > 0);
-	if (pause) {
-		pause = false;
-		OSStatus status = AudioOutputUnitStart(au);
-		if (status != 0) {
-			AudioUnitUninitialize(au);
-			throw std::runtime_error("Unable to restart audio output after pause");
-		}
-	}
+
+	pause = false;
+
+	ConstBuffer<uint8_t> input((const uint8_t *)chunk, size);
+
 #ifdef ENABLE_DSD
 	if (dop_enabled) {
-		const auto e = pcm_export->Export({chunk, size});
-		if (e.empty())
+		input = ConstBuffer<uint8_t>::FromVoid(pcm_export->Export(input.ToVoid()));
+		if (input.empty())
 			return size;
+	}
+#endif
+
+	size_t bytes_written = ring_buffer->push(input.data, input.size);
 
-		size_t bytes_written = ring_buffer->push((const uint8_t *)e.data, e.size);
-		return pcm_export->CalcInputSize(bytes_written);
+	if (!started) {
+		OSStatus status = AudioOutputUnitStart(au);
+		if (status != noErr)
+			throw std::runtime_error("Unable to restart audio output after pause");
+
+		started = true;
 	}
+
+#ifdef ENABLE_DSD
+	if (dop_enabled)
+		bytes_written = pcm_export->CalcInputSize(bytes_written);
 #endif
-	return ring_buffer->push((const uint8_t *)chunk, size);
+
+	return bytes_written;
 }
 
 std::chrono::steady_clock::duration
@@ -827,22 +806,30 @@ OSXOutput::Delay() const noexcept
 
 bool OSXOutput::Pause()
 {
-	if (!pause) {
-		pause = true;
+	pause = true;
+
+	if (started) {
 		AudioOutputUnitStop(au);
+		started = false;
 	}
+
 	return true;
 }
 
 void
 OSXOutput::Cancel() noexcept
 {
-	AudioOutputUnitStop(au);
+	if (started) {
+		AudioOutputUnitStop(au);
+		started = false;
+	}
+
 	ring_buffer->reset();
 #ifdef ENABLE_DSD
 	pcm_export->Reset();
 #endif
-	AudioOutputUnitStart(au);
+
+	/* the AudioUnit will be restarted by the next Play() call */
 }
 
 int
diff --git a/src/output/plugins/meson.build b/src/output/plugins/meson.build
index 13cec10cd..8d48674ae 100644
--- a/src/output/plugins/meson.build
+++ b/src/output/plugins/meson.build
@@ -76,18 +76,7 @@ endif
 if is_darwin
   output_plugins_sources += [
     'OSXOutputPlugin.cxx',
-    '../../apple/Throw.cxx',
   ]
-  audiounit_dep = declare_dependency(
-    link_args: [
-      '-framework', 'AudioUnit', '-framework', 'CoreAudio', '-framework', 'CoreServices',
-    ],
-    dependencies: [
-      boost_dep,
-    ],
-  )
-else
-  audiounit_dep = dependency('', required: false)
 endif
 output_features.set('HAVE_OSX', is_darwin)
 
@@ -163,7 +152,7 @@ output_plugins = static_library(
   include_directories: inc,
   dependencies: [
     alsa_dep,
-    audiounit_dep,
+    apple_dep,
     libao_dep,
     libjack_dep,
     pulse_dep,