From 66f640527d144df68685d807e95a2ea2ff75b742 Mon Sep 17 00:00:00 2001 From: Stefanos Costa Date: Sat, 1 Aug 2026 16:18:21 +0300 Subject: [PATCH] audio: fix pacing and AudioOut2 port lifetime Extracted from 6a60f1b17481a0e5e14242c0fb4dc22f963545e1 in KytyPS5/KytyPS5#147. --- .github/workflows/build.yml | 12 +- src/CMakeLists.txt | 9 ++ src/libs/audio.cpp | 18 ++- src/libs/libAudio2.cpp | 89 +++++++------ tests/AudioOut2PortTests.cpp | 249 +++++++++++++++++++++++++++++++++++ 5 files changed, 326 insertions(+), 51 deletions(-) create mode 100644 tests/AudioOut2PortTests.cpp diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 0216615cf..ca1d9598b 100644 --- a/.github/workflows/build.yml +++ b/.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 diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index eeb6885ac..bd60568cf 100644 --- a/src/CMakeLists.txt +++ b/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 $) add_test(NAME resource_mutex COMMAND $) add_test(NAME event_queue_lifetime COMMAND $) + add_test(NAME audio_out2_port COMMAND $) add_test(NAME shader_recompiler_compute COMMAND $) add_test(NAME virtual_memory_allocation COMMAND $) diff --git a/src/libs/audio.cpp b/src/libs/audio.cpp index 97cb48501..32cb751d5 100644 --- a/src/libs/audio.cpp +++ b/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((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); } diff --git a/src/libs/libAudio2.cpp b/src/libs/libAudio2.cpp index cc8b87969..054859f31 100644 --- a/src/libs/libAudio2.cpp +++ b/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 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 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); - if (port_state == nullptr) { - for (auto& candidate: g_audioout2_ports) { - if (!candidate.used) { - port_state = &candidate; - break; - } + AudioOut2PortStateEntry* port_state = nullptr; + for (auto& candidate: g_audioout2_ports) { + if (!candidate.used) { + port_state = &candidate; + 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; - - 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; + 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 {}; - port_state->used = true; - port_state->handle = *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; - port_state->audio_handle = audio_handle; + const bool reserved = port_state->used && port_state->handle == next_port; + if (reserved) { + 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(); diff --git a/tests/AudioOut2PortTests.cpp b/tests/AudioOut2PortTests.cpp new file mode 100644 index 000000000..67baee8f4 --- /dev/null +++ b/tests/AudioOut2PortTests.cpp @@ -0,0 +1,249 @@ +#include "libs/audio.h" +#include "libs/audio_internal.h" +#include "libs/errno.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace { + +namespace AudioOut2 = Libs::Audio::AudioOut2; + +std::mutex g_device_mutex; +std::condition_variable g_device_cv; +std::vector 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(param); +} + +const auto* AsParam(const ContextParam* param) { + return reinterpret_cast(param); +} + +auto* AsState(PortState* state) { + return reinterpret_cast(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(¶m), 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(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(¶m), &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 ports; + ports.reserve(256); + + for (int i = 0; i < 256; i++) { + AudioOut2::AudioOut2PortHandle port = 0; + Check(AudioOut2::AudioOut2PortCreate(context, AsParam(¶m), &port) == OK, + "port table filled early"); + ports.push_back(port); + } + + AudioOut2::AudioOut2PortHandle overflow = 0; + Check(AudioOut2::AudioOut2PortCreate(context, AsParam(¶m), &overflow) != OK, + "full port table accepted another port"); + + for (auto port: ports) { + AudioOut2::AudioOut2PortDestroy(port); + } + + AudioOut2::AudioOut2PortHandle port = 0; + Check(AudioOut2::AudioOut2PortCreate(context, AsParam(¶m), &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 ports(thread_count); + std::vector results(thread_count); + std::vector threads; + + BlockDeviceOpens(); + for (int i = 0; i < thread_count; i++) { + threads.emplace_back([&, i]() { + results[i] = AudioOut2::AudioOut2PortCreate(context, AsParam(¶m), &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(¶m), &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; +}