audio: fix pacing and AudioOut2 port lifetime

Extracted from 6a60f1b17481a0e5e14242c0fb4dc22f963545e1 in KytyPS5/KytyPS5#147.

.github/workflows/build.yml

@@ -83,12 +83,12 @@ jobs:
       - name: Build
         shell: cmd
         run: |
-          cmake --build _Build/windows --target launcher virtual_memory_allocation_tests --parallel
+          cmake --build _Build/windows --target launcher audio_out2_port_tests virtual_memory_allocation_tests --parallel
 
       - name: Test
         shell: cmd
         run: |
-          ctest --test-dir _Build/windows --output-on-failure -R "^virtual_memory_allocation$"
+          ctest --test-dir _Build/windows --output-on-failure -R "^(audio_out2_port|virtual_memory_allocation)$"
 
       - name: Install
         shell: cmd
@@ -159,14 +159,14 @@ jobs:
         shell: bash
         run: |
           cmake --build _Build/macos \
-            --target launcher virtual_memory_allocation_tests \
+            --target launcher audio_out2_port_tests virtual_memory_allocation_tests \
             --parallel
 
       - name: Test
         shell: bash
         run: |
           ctest --test-dir _Build/macos --output-on-failure \
-            -R '^virtual_memory_allocation$'
+            -R '^(audio_out2_port|virtual_memory_allocation)$'
 
       - name: Install
         shell: bash
@@ -297,14 +297,14 @@ jobs:
         run: |
           cmake --build _Build/linux \
             --target launcher page_manager_tests memory_tracker_tests \
-              virtual_memory_allocation_tests \
+              audio_out2_port_tests virtual_memory_allocation_tests \
             --parallel
 
       - name: Test
         shell: bash
         run: |
           ctest --test-dir _Build/linux --output-on-failure \
-            -R '^(page_manager|memory_tracker|virtual_memory_allocation)$'
+            -R '^(audio_out2_port|page_manager|memory_tracker|virtual_memory_allocation)$'
 
       - name: Install
         shell: bash

src/CMakeLists.txt

@@ -394,6 +394,14 @@ add_executable(resource_mutex_tests EXCLUDE_FROM_ALL
 target_link_libraries(resource_mutex_tests common)
 target_include_directories(resource_mutex_tests PRIVATE ${inc_headers})
 
+add_executable(audio_out2_port_tests EXCLUDE_FROM_ALL
+	../tests/AudioOut2PortTests.cpp
+	libs/libAudio2.cpp
+	loader/timer.cpp
+)
+target_link_libraries(audio_out2_port_tests common fmt::fmt)
+target_include_directories(audio_out2_port_tests PRIVATE ${inc_headers})
+
 add_executable(event_queue_lifetime_tests EXCLUDE_FROM_ALL
 	../tests/EventQueueLifetimeTests.cpp
 	kernel/eventQueue.cpp
@@ -454,6 +462,7 @@ if(BUILD_TESTING)
 	add_test(NAME resource_tracking COMMAND $<TARGET_FILE:resource_tracking_tests>)
 	add_test(NAME resource_mutex COMMAND $<TARGET_FILE:resource_mutex_tests>)
 	add_test(NAME event_queue_lifetime COMMAND $<TARGET_FILE:event_queue_lifetime_tests>)
+	add_test(NAME audio_out2_port COMMAND $<TARGET_FILE:audio_out2_port_tests>)
 	add_test(NAME shader_recompiler_compute COMMAND $<TARGET_FILE:shader_recompiler_compute_tests>)
 	add_test(NAME virtual_memory_allocation
 		COMMAND $<TARGET_FILE:virtual_memory_allocation_tests>)

src/libs/audio.cpp

@@ -364,7 +364,12 @@ bool Audio::QueueSdlAudio(PortOut* port, const void* data, bool blocking) {
 	}
 
 	if (blocking) {
-		const auto min_queued_size = queue_size * 2u;
+		constexpr uint64_t target_latency_us = 40000;
+		const auto buffer_us = port->freq != 0 ? (1000000ULL * port->samples_num) / port->freq : 0;
+		const auto buffers =
+		    buffer_us != 0 ? static_cast<uint32_t>((target_latency_us + buffer_us - 1) / buffer_us)
+		                   : 2u;
+		const auto min_queued_size = queue_size * std::clamp(buffers, 2u, 16u);
 		const auto wait_start      = LibKernel::KernelGetProcessTime();
 		while (SDL_GetQueuedAudioSize(port->audio_device) > min_queued_size) {
 			if (LibKernel::KernelGetProcessTime() - wait_start > 200000) {
@@ -511,7 +516,16 @@ uint32_t Audio::AudioOutOutputs(OutputParam* params, uint32_t num, bool blocking
 		max_wait_time      = (wait_time > max_wait_time ? wait_time : max_wait_time);
 	}
 
-	if (blocking && max_wait_time != 0) {
+	bool all_ports_have_device = true;
+	for (uint32_t i = 0; i < num; i++) {
+		if (m_out_ports[params[i].handle.GetId()].audio_device == 0) {
+			all_ports_have_device = false;
+			break;
+		}
+	}
+
+	// Device-backed ports are paced by the SDL queue above.
+	if (blocking && max_wait_time != 0 && !all_ports_have_device) {
 		Common::Thread::SleepMicro(max_wait_time);
 	}
 

src/libs/libAudio2.cpp

@@ -336,18 +336,6 @@ static AudioOut2PortStateEntry* audioout2_find_port_locked(AudioOut2PortHandle p
 	return nullptr;
 }
 
-static uint32_t audioout2_context_grains(AudioOut2ContextHandle ctx) {
-	uint32_t samples_num = 512;
-
-	g_audioout2_context_mutex.Lock();
-	if (auto* state = audioout2_find_context_locked(ctx); state != nullptr) {
-		samples_num = (state->num_grains == 0 ? 512u : state->num_grains);
-	}
-	g_audioout2_context_mutex.Unlock();
-
-	return samples_num;
-}
-
 static void audioout2_queue_context_audio(AudioOut2ContextHandle ctx, bool blocking) {
 	std::vector<AudioInternal::OutputParam> params;
 	params.reserve(AudioInternal::OUT_PORTS_MAX);
@@ -455,6 +443,12 @@ int KYTY_SYSV_ABI AudioOut2ContextDestroy(AudioOut2ContextHandle ctx) {
 	PRINT_NAME();
 	LOGF("\t ctx = 0x%016" PRIx64 "\n", ctx);
 
+	g_audioout2_context_mutex.Lock();
+	if (auto* state = audioout2_find_context_locked(ctx); state != nullptr) {
+		*state = AudioOut2ContextState {};
+	}
+	g_audioout2_context_mutex.Unlock();
+
 	std::array<int, 256> audio_handles {};
 	size_t               audio_handles_num = 0;
 
@@ -473,12 +467,6 @@ int KYTY_SYSV_ABI AudioOut2ContextDestroy(AudioOut2ContextHandle ctx) {
 		audioout2_close_audio_handle(audio_handles[i]);
 	}
 
-	g_audioout2_context_mutex.Lock();
-	if (auto* state = audioout2_find_context_locked(ctx); state != nullptr) {
-		*state = AudioOut2ContextState {};
-	}
-	g_audioout2_context_mutex.Unlock();
-
 	return OK;
 }
 
@@ -559,30 +547,45 @@ int KYTY_SYSV_ABI AudioOut2PortCreate(AudioOut2ContextHandle ctx, const AudioOut
 	EXIT_NOT_IMPLEMENTED(params == nullptr);
 	EXIT_NOT_IMPLEMENTED(port == nullptr);
 
-	const auto next_port = g_audioout2_next_port.fetch_add(1, std::memory_order_relaxed);
+	const auto next_port    = g_audioout2_next_port.fetch_add(1, std::memory_order_relaxed);
+	const auto audio_format = audioout2_data_format_to_audio_format(params->data_format);
+	const auto audio_type   = audioout2_port_type_to_audio_out_type(params->port_type);
+
+	g_audioout2_context_mutex.Lock();
+	const auto* context_state = audioout2_find_context_locked(ctx);
+	if (context_state == nullptr) {
+		g_audioout2_context_mutex.Unlock();
+		return AUDIO_OUT2_ERROR_INVALID_PARAM;
+	}
+	const auto samples_num = context_state->num_grains == 0 ? 512u : context_state->num_grains;
 
 	g_audioout2_port_mutex.Lock();
-	auto* port_state = audioout2_find_port_locked(0);
+	AudioOut2PortStateEntry* port_state = nullptr;
-	if (port_state == nullptr) {
+	for (auto& candidate: g_audioout2_ports) {
-		for (auto& candidate: g_audioout2_ports) {
+		if (!candidate.used) {
-			if (!candidate.used) {
+			port_state = &candidate;
-				port_state = &candidate;
+			break;
-				break;
-			}
 		}
 	}
+	if (port_state != nullptr) {
+		*port_state               = AudioOut2PortStateEntry {};
+		port_state->used          = true;
+		port_state->handle        = next_port;
+		port_state->context       = ctx;
+		port_state->port_type     = params->port_type;
+		port_state->data_format   = params->data_format;
+		port_state->sampling_freq = params->sampling_freq;
+		port_state->samples_num   = samples_num;
+		port_state->audio_format  = audio_format;
+	}
 	g_audioout2_port_mutex.Unlock();
+	g_audioout2_context_mutex.Unlock();
 
-	if (next_port > g_audioout2_ports.size() || port_state == nullptr) {
+	if (port_state == nullptr) {
 		return AUDIO_OUT2_ERROR_PORT_FULL;
 	}
 
-	*port = next_port;
+	int audio_handle = 0;
-
-	const auto samples_num  = audioout2_context_grains(ctx);
-	const auto audio_format = audioout2_data_format_to_audio_format(params->data_format);
-	const auto audio_type   = audioout2_port_type_to_audio_out_type(params->port_type);
-	int        audio_handle = 0;
 
 	if (audio_format != AudioInternal::Format::Unknown &&
 	    !audioout2_port_type_is_object(params->port_type)) {
@@ -591,17 +594,17 @@ int KYTY_SYSV_ABI AudioOut2PortCreate(AudioOut2ContextHandle ctx, const AudioOut
 	}
 
 	g_audioout2_port_mutex.Lock();
-	*port_state               = AudioOut2PortStateEntry {};
+	const bool reserved = port_state->used && port_state->handle == next_port;
-	port_state->used          = true;
+	if (reserved) {
-	port_state->handle        = *port;
+		port_state->audio_handle = audio_handle;
-	port_state->context       = ctx;
+	}
-	port_state->port_type     = params->port_type;
-	port_state->data_format   = params->data_format;
-	port_state->sampling_freq = params->sampling_freq;
-	port_state->samples_num   = samples_num;
-	port_state->audio_format  = audio_format;
-	port_state->audio_handle  = audio_handle;
 	g_audioout2_port_mutex.Unlock();
+	if (!reserved) {
+		audioout2_close_audio_handle(audio_handle);
+		return AUDIO_OUT2_ERROR_INVALID_PARAM;
+	}
+
+	*port = next_port;
 
 	if (next_port <= 16 || (next_port % 600) == 0) {
 		PRINT_NAME();

tests/AudioOut2PortTests.cpp

@@ -0,0 +1,249 @@
+#include "libs/audio.h"
+#include "libs/audio_internal.h"
+#include "libs/errno.h"
+
+#include <algorithm>
+#include <atomic>
+#include <condition_variable>
+#include <cstdint>
+#include <cstdio>
+#include <cstdlib>
+#include <mutex>
+#include <thread>
+#include <vector>
+
+namespace {
+
+namespace AudioOut2 = Libs::Audio::AudioOut2;
+
+std::mutex              g_device_mutex;
+std::condition_variable g_device_cv;
+std::vector<int>        g_live_devices;
+int                     g_next_device  = 1;
+int                     g_open_waiters = 0;
+bool                    g_block_opens  = false;
+
+void Check(bool value, const char* text) {
+	if (!value) {
+		std::fprintf(stderr, "AudioOut2PortTests: failed: %s\n", text);
+		std::abort();
+	}
+}
+
+struct PortParam {
+	uint16_t port_type;
+	uint16_t pad;
+	uint32_t data_format;
+	uint32_t sampling_freq;
+	uint32_t flags;
+	uint64_t user_handle;
+	uint32_t reserved[10];
+};
+
+struct ContextParam {
+	uint32_t max_ports;
+	uint32_t max_object_ports;
+	uint32_t guarantee_object_ports;
+	uint32_t queue_depth;
+	uint32_t num_grains;
+	uint32_t flags;
+	uint32_t reserved[10];
+};
+
+struct PortState {
+	uint16_t output;
+	uint8_t  num_channels;
+	uint8_t  pad1;
+	int16_t  volume;
+	uint16_t reroute_counter;
+	uint32_t flags;
+	uint32_t pad2;
+	uint64_t reserved[6];
+};
+
+const auto* AsParam(const PortParam* param) {
+	return reinterpret_cast<const AudioOut2::AudioOut2PortParam*>(param);
+}
+
+const auto* AsParam(const ContextParam* param) {
+	return reinterpret_cast<const AudioOut2::AudioOut2ContextParam*>(param);
+}
+
+auto* AsState(PortState* state) {
+	return reinterpret_cast<AudioOut2::AudioOut2PortState*>(state);
+}
+
+PortParam MakeParam(uint32_t data_format = 0x200) {
+	PortParam param {};
+	param.data_format   = data_format;
+	param.sampling_freq = 48000;
+	return param;
+}
+
+AudioOut2::AudioOut2ContextHandle CreateContext() {
+	ContextParam param {};
+	param.queue_depth                         = 4;
+	param.num_grains                          = 512;
+	AudioOut2::AudioOut2ContextHandle context = 0;
+	Check(AudioOut2::AudioOut2ContextCreate(AsParam(&param), nullptr, 0, &context) == OK,
+	      "context create failed");
+	return context;
+}
+
+void BlockDeviceOpens() {
+	std::lock_guard lock(g_device_mutex);
+	g_open_waiters = 0;
+	g_block_opens  = true;
+}
+
+void WaitForDeviceOpens(int count) {
+	std::unique_lock lock(g_device_mutex);
+	g_device_cv.wait(lock, [count]() { return g_open_waiters >= count; });
+}
+
+void ReleaseDeviceOpens() {
+	std::lock_guard lock(g_device_mutex);
+	g_block_opens = false;
+	g_device_cv.notify_all();
+}
+
+int LiveDeviceCount() {
+	std::lock_guard lock(g_device_mutex);
+	return static_cast<int>(g_live_devices.size());
+}
+
+void TestSlotReuse() {
+	const auto context = CreateContext();
+	const auto param   = MakeParam();
+	for (int i = 0; i < 300; i++) {
+		AudioOut2::AudioOut2PortHandle port = 0;
+		Check(AudioOut2::AudioOut2PortCreate(context, AsParam(&param), &port) == OK,
+		      "port slot was not reusable");
+		Check(port != 0, "port handle is zero");
+		AudioOut2::AudioOut2PortDestroy(port);
+	}
+	AudioOut2::AudioOut2ContextDestroy(context);
+}
+
+void TestFullTableRecovers() {
+	const auto                                  context = CreateContext();
+	const auto                                  param   = MakeParam();
+	std::vector<AudioOut2::AudioOut2PortHandle> ports;
+	ports.reserve(256);
+
+	for (int i = 0; i < 256; i++) {
+		AudioOut2::AudioOut2PortHandle port = 0;
+		Check(AudioOut2::AudioOut2PortCreate(context, AsParam(&param), &port) == OK,
+		      "port table filled early");
+		ports.push_back(port);
+	}
+
+	AudioOut2::AudioOut2PortHandle overflow = 0;
+	Check(AudioOut2::AudioOut2PortCreate(context, AsParam(&param), &overflow) != OK,
+	      "full port table accepted another port");
+
+	for (auto port: ports) {
+		AudioOut2::AudioOut2PortDestroy(port);
+	}
+
+	AudioOut2::AudioOut2PortHandle port = 0;
+	Check(AudioOut2::AudioOut2PortCreate(context, AsParam(&param), &port) == OK,
+	      "port table did not recover");
+	AudioOut2::AudioOut2PortDestroy(port);
+	AudioOut2::AudioOut2ContextDestroy(context);
+}
+
+void TestConcurrentCreates() {
+	constexpr int                               thread_count = 8;
+	const auto                                  context      = CreateContext();
+	const auto                                  param        = MakeParam(0x800);
+	std::vector<AudioOut2::AudioOut2PortHandle> ports(thread_count);
+	std::vector<int>                            results(thread_count);
+	std::vector<std::thread>                    threads;
+
+	BlockDeviceOpens();
+	for (int i = 0; i < thread_count; i++) {
+		threads.emplace_back([&, i]() {
+			results[i] = AudioOut2::AudioOut2PortCreate(context, AsParam(&param), &ports[i]);
+		});
+	}
+	WaitForDeviceOpens(thread_count);
+	ReleaseDeviceOpens();
+	for (auto& thread: threads) {
+		thread.join();
+	}
+
+	for (int i = 0; i < thread_count; i++) {
+		Check(results[i] == OK, "concurrent port create failed");
+		PortState state {};
+		AudioOut2::AudioOut2PortGetState(ports[i], AsState(&state));
+		Check(state.num_channels == 8, "concurrent create lost its reserved slot");
+		AudioOut2::AudioOut2PortDestroy(ports[i]);
+	}
+	Check(LiveDeviceCount() == 0, "concurrent create leaked a device");
+	AudioOut2::AudioOut2ContextDestroy(context);
+}
+
+void TestContextDestroyCancelsPendingCreate() {
+	const auto                     context = CreateContext();
+	const auto                     param   = MakeParam();
+	AudioOut2::AudioOut2PortHandle port    = 0;
+	int                            result  = OK;
+
+	BlockDeviceOpens();
+	std::thread creator(
+	    [&]() { result = AudioOut2::AudioOut2PortCreate(context, AsParam(&param), &port); });
+	WaitForDeviceOpens(1);
+	AudioOut2::AudioOut2ContextDestroy(context);
+	ReleaseDeviceOpens();
+	creator.join();
+
+	Check(result != OK, "destroyed context retained a pending port create");
+	Check(LiveDeviceCount() == 0, "cancelled port create leaked a device");
+}
+
+} // namespace
+
+namespace Libs::Audio::AudioInternal {
+
+int AudioOutOpen(int /*type*/, uint32_t /*samples_num*/, uint32_t /*freq*/, Format /*format*/) {
+	std::unique_lock lock(g_device_mutex);
+	const int        handle = g_next_device++;
+	g_live_devices.push_back(handle);
+	g_open_waiters++;
+	g_device_cv.notify_all();
+	g_device_cv.wait(lock, []() { return !g_block_opens; });
+	return handle;
+}
+
+void AudioOutClose(int handle) {
+	std::lock_guard lock(g_device_mutex);
+	const auto      it = std::find(g_live_devices.begin(), g_live_devices.end(), handle);
+	if (it != g_live_devices.end()) {
+		g_live_devices.erase(it);
+	}
+}
+
+uint32_t AudioOutOutputs(const OutputParam* /*params*/, uint32_t /*num*/, bool /*blocking*/) {
+	return 0;
+}
+
+} // namespace Libs::Audio::AudioInternal
+
+namespace Libs::LibKernel {
+
+uint64_t KYTY_SYSV_ABI KernelGetProcessTime() {
+	static std::atomic_uint64_t now {0};
+	return now.fetch_add(1000);
+}
+
+} // namespace Libs::LibKernel
+
+int main() {
+	TestSlotReuse();
+	TestFullTableRecovers();
+	TestConcurrentCreates();
+	TestContextDestroyCancelsPendingCreate();
+	std::printf("AudioOut2PortTests: all cases passed\n");
+	return 0;
+}