Skip to content

Commit 55b5017

Browse files
squidbussquidbus
authored
audio: Improve port state guards. (#1998)
1 parent 596f4cd commit 55b5017

File tree

3 files changed

+103
-83
lines changed

3 files changed

+103
-83
lines changed

‎src/core/libraries/audio/audioout.cpp

Lines changed: 78 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,13 @@
33

44
#include <memory>
55
#include <mutex>
6-
#include <shared_mutex>
6+
#include <stop_token>
7+
#include <thread>
78
#include <magic_enum/magic_enum.hpp>
89

910
#include "common/assert.h"
1011
#include "common/config.h"
1112
#include "common/logging/log.h"
12-
#include "common/polyfill_thread.h"
1313
#include "common/thread.h"
1414
#include "core/libraries/audio/audioout.h"
1515
#include "core/libraries/audio/audioout_backend.h"
@@ -18,7 +18,7 @@
1818

1919
namespace Libraries::AudioOut {
2020

21-
std::shared_mutex ports_mutex;
21+
std::mutex port_open_mutex{};
2222
std::array<PortOut, SCE_AUDIO_OUT_NUM_PORTS> ports_out{};
2323

2424
static std::unique_ptr<AudioOutBackend> audio;
@@ -93,17 +93,20 @@ int PS4_SYSV_ABI sceAudioOutClose(s32 handle) {
9393
return ORBIS_AUDIO_OUT_ERROR_INVALID_PORT;
9494
}
9595

96-
std::scoped_lock lock(ports_mutex);
96+
std::unique_lock open_lock{port_open_mutex};
9797
auto& port = ports_out.at(handle - 1);
98-
if (!port.impl) {
99-
return ORBIS_AUDIO_OUT_ERROR_INVALID_PORT;
98+
{
99+
std::unique_lock lock{port.mutex};
100+
if (!port.IsOpen()) {
101+
return ORBIS_AUDIO_OUT_ERROR_INVALID_PORT;
102+
}
103+
std::free(port.output_buffer);
104+
port.output_buffer = nullptr;
105+
port.output_ready = false;
106+
port.impl = nullptr;
100107
}
101-
108+
// Stop outside of port lock scope to prevent deadlocks.
102109
port.output_thread.Stop();
103-
std::free(port.output_buffer);
104-
port.output_buffer = nullptr;
105-
port.output_ready = false;
106-
port.impl = nullptr;
107110
return ORBIS_OK;
108111
}
109112

@@ -172,35 +175,34 @@ int PS4_SYSV_ABI sceAudioOutGetPortState(s32 handle, OrbisAudioOutPortState* sta
172175
return ORBIS_AUDIO_OUT_ERROR_INVALID_PORT;
173176
}
174177

175-
std::scoped_lock lock(ports_mutex);
176-
const auto& port = ports_out.at(handle - 1);
177-
if (!port.impl) {
178-
return ORBIS_AUDIO_OUT_ERROR_INVALID_PORT;
179-
}
180-
181-
state->rerouteCounter = 0;
182-
state->volume = 127;
183-
184-
switch (port.type) {
185-
case OrbisAudioOutPort::Main:
186-
case OrbisAudioOutPort::Bgm:
187-
case OrbisAudioOutPort::Voice:
188-
state->output = 1;
189-
state->channel = port.format_info.num_channels > 2 ? 2 : port.format_info.num_channels;
190-
break;
191-
case OrbisAudioOutPort::Personal:
192-
case OrbisAudioOutPort::Padspk:
193-
state->output = 4;
194-
state->channel = 1;
195-
break;
196-
case OrbisAudioOutPort::Aux:
197-
state->output = 0;
198-
state->channel = 0;
199-
break;
200-
default:
201-
UNREACHABLE();
178+
auto& port = ports_out.at(handle - 1);
179+
{
180+
std::unique_lock lock{port.mutex};
181+
if (!port.IsOpen()) {
182+
return ORBIS_AUDIO_OUT_ERROR_INVALID_PORT;
183+
}
184+
switch (port.type) {
185+
case OrbisAudioOutPort::Main:
186+
case OrbisAudioOutPort::Bgm:
187+
case OrbisAudioOutPort::Voice:
188+
state->output = 1;
189+
state->channel = port.format_info.num_channels > 2 ? 2 : port.format_info.num_channels;
190+
break;
191+
case OrbisAudioOutPort::Personal:
192+
case OrbisAudioOutPort::Padspk:
193+
state->output = 4;
194+
state->channel = 1;
195+
break;
196+
case OrbisAudioOutPort::Aux:
197+
state->output = 0;
198+
state->channel = 0;
199+
break;
200+
default:
201+
UNREACHABLE();
202+
}
203+
state->rerouteCounter = 0;
204+
state->volume = 127;
202205
}
203-
204206
return ORBIS_OK;
205207
}
206208

@@ -279,15 +281,16 @@ static void AudioOutputThread(PortOut* port, const std::stop_token& stop) {
279281
while (true) {
280282
timer.Start();
281283
{
282-
std::unique_lock lock{port->output_mutex};
283-
Common::CondvarWait(port->output_cv, lock, stop, [&] { return port->output_ready; });
284-
if (stop.stop_requested()) {
285-
break;
284+
std::unique_lock lock{port->mutex};
285+
if (port->output_cv.wait(lock, stop, [&] { return port->output_ready; })) {
286+
port->impl->Output(port->output_buffer);
287+
port->output_ready = false;
286288
}
287-
port->impl->Output(port->output_buffer);
288-
port->output_ready = false;
289289
}
290290
port->output_cv.notify_one();
291+
if (stop.stop_requested()) {
292+
break;
293+
}
291294
timer.End();
292295
}
293296
}
@@ -332,27 +335,30 @@ s32 PS4_SYSV_ABI sceAudioOutOpen(UserService::OrbisUserServiceUserId user_id,
332335
return ORBIS_AUDIO_OUT_ERROR_INVALID_FORMAT;
333336
}
334337

335-
std::scoped_lock lock{ports_mutex};
338+
std::unique_lock open_lock{port_open_mutex};
336339
const auto port =
337-
std::ranges::find_if(ports_out, [&](const PortOut& p) { return p.impl == nullptr; });
340+
std::ranges::find_if(ports_out, [&](const PortOut& p) { return !p.IsOpen(); });
338341
if (port == ports_out.end()) {
339342
LOG_ERROR(Lib_AudioOut, "Audio ports are full");
340343
return ORBIS_AUDIO_OUT_ERROR_PORT_FULL;
341344
}
342345

343-
port->type = port_type;
344-
port->format_info = GetFormatInfo(format);
345-
port->sample_rate = sample_rate;
346-
port->buffer_frames = length;
347-
port->volume.fill(SCE_AUDIO_OUT_VOLUME_0DB);
346+
{
347+
std::unique_lock port_lock(port->mutex);
348348

349-
port->impl = audio->Open(*port);
349+
port->type = port_type;
350+
port->format_info = GetFormatInfo(format);
351+
port->sample_rate = sample_rate;
352+
port->buffer_frames = length;
353+
port->volume.fill(SCE_AUDIO_OUT_VOLUME_0DB);
350354

351-
port->output_buffer = std::malloc(port->BufferSize());
352-
port->output_ready = false;
353-
port->output_thread.Run(
354-
[port](const std::stop_token& stop) { AudioOutputThread(&*port, stop); });
355+
port->impl = audio->Open(*port);
355356

357+
port->output_buffer = std::malloc(port->BufferSize());
358+
port->output_ready = false;
359+
port->output_thread.Run(
360+
[port](const std::stop_token& stop) { AudioOutputThread(&*port, stop); });
361+
}
356362
return std::distance(ports_out.begin(), port) + 1;
357363
}
358364

@@ -367,14 +373,13 @@ s32 PS4_SYSV_ABI sceAudioOutOutput(s32 handle, void* ptr) {
367373
}
368374

369375
auto& port = ports_out.at(handle - 1);
370-
if (!port.impl) {
371-
return ORBIS_AUDIO_OUT_ERROR_INVALID_PORT;
372-
}
373-
374376
{
375-
std::unique_lock lock{port.output_mutex};
377+
std::unique_lock lock{port.mutex};
378+
if (!port.IsOpen()) {
379+
return ORBIS_AUDIO_OUT_ERROR_INVALID_PORT;
380+
}
376381
port.output_cv.wait(lock, [&] { return !port.output_ready; });
377-
if (ptr != nullptr) {
382+
if (ptr != nullptr && port.IsOpen()) {
378383
std::memcpy(port.output_buffer, ptr, port.BufferSize());
379384
port.output_ready = true;
380385
}
@@ -488,19 +493,19 @@ s32 PS4_SYSV_ABI sceAudioOutSetVolume(s32 handle, s32 flag, s32* vol) {
488493
return ORBIS_AUDIO_OUT_ERROR_INVALID_PORT;
489494
}
490495

491-
std::scoped_lock lock(ports_mutex);
492496
auto& port = ports_out.at(handle - 1);
493-
if (!port.impl) {
494-
return ORBIS_AUDIO_OUT_ERROR_INVALID_PORT;
495-
}
496-
497-
for (int i = 0; i < port.format_info.num_channels; i++, flag >>= 1u) {
498-
if (flag & 0x1u) {
499-
port.volume[i] = vol[i];
497+
{
498+
std::unique_lock lock{port.mutex};
499+
if (!port.IsOpen()) {
500+
return ORBIS_AUDIO_OUT_ERROR_INVALID_PORT;
500501
}
502+
for (int i = 0; i < port.format_info.num_channels; i++, flag >>= 1u) {
503+
if (flag & 0x1u) {
504+
port.volume[i] = vol[i];
505+
}
506+
}
507+
port.impl->SetVolume(port.volume);
501508
}
502-
503-
port.impl->SetVolume(port.volume);
504509
return ORBIS_OK;
505510
}
506511

‎src/core/libraries/audio/audioout.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33

44
#pragma once
55

6+
#include <condition_variable>
67
#include <memory>
8+
#include <mutex>
79

810
#include "common/bit_field.h"
911
#include "core/libraries/kernel/threads.h"
@@ -74,10 +76,10 @@ struct AudioFormatInfo {
7476
};
7577

7678
struct PortOut {
79+
std::mutex mutex;
7780
std::unique_ptr<PortBackend> impl{};
7881

7982
void* output_buffer;
80-
std::mutex output_mutex;
8183
std::condition_variable_any output_cv;
8284
bool output_ready;
8385
Kernel::Thread output_thread{};
@@ -88,6 +90,10 @@ struct PortOut {
8890
u32 buffer_frames;
8991
std::array<s32, 8> volume;
9092

93+
[[nodiscard]] bool IsOpen() const {
94+
return impl != nullptr;
95+
}
96+
9197
[[nodiscard]] u32 BufferSize() const {
9298
return buffer_frames * format_info.FrameSize();
9399
}

‎src/core/libraries/audio/sdl_audio.cpp

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ namespace Libraries::AudioOut {
1414
class SDLPortBackend : public PortBackend {
1515
public:
1616
explicit SDLPortBackend(const PortOut& port)
17-
: frame_size(port.format_info.FrameSize()), buffer_size(port.BufferSize()) {
17+
: frame_size(port.format_info.FrameSize()), guest_buffer_size(port.BufferSize()) {
1818
// We want the latency for delivering frames out to be as small as possible,
1919
// so set the sample frames hint to the number of frames per buffer.
2020
const auto samples_num_str = std::to_string(port.buffer_frames);
@@ -33,7 +33,7 @@ class SDLPortBackend : public PortBackend {
3333
LOG_ERROR(Lib_AudioOut, "Failed to create SDL audio stream: {}", SDL_GetError());
3434
return;
3535
}
36-
queue_threshold = CalculateQueueThreshold();
36+
CalculateQueueThreshold();
3737
if (!SDL_SetAudioStreamInputChannelMap(stream, port.format_info.channel_layout.data(),
3838
port.format_info.num_channels)) {
3939
LOG_ERROR(Lib_AudioOut, "Failed to configure SDL audio stream channel map: {}",
@@ -71,9 +71,9 @@ class SDLPortBackend : public PortBackend {
7171
queue_threshold);
7272
SDL_ClearAudioStream(stream);
7373
// Recalculate the threshold in case this happened because of a device change.
74-
queue_threshold = CalculateQueueThreshold();
74+
CalculateQueueThreshold();
7575
}
76-
if (!SDL_PutAudioStreamData(stream, ptr, static_cast<int>(buffer_size))) {
76+
if (!SDL_PutAudioStreamData(stream, ptr, static_cast<int>(guest_buffer_size))) {
7777
LOG_ERROR(Lib_AudioOut, "Failed to output to SDL audio stream: {}", SDL_GetError());
7878
}
7979
}
@@ -91,7 +91,7 @@ class SDLPortBackend : public PortBackend {
9191
}
9292

9393
private:
94-
[[nodiscard]] u32 CalculateQueueThreshold() const {
94+
void CalculateQueueThreshold() {
9595
SDL_AudioSpec discard;
9696
int sdl_buffer_frames;
9797
if (!SDL_GetAudioDeviceFormat(SDL_GetAudioStreamDevice(stream), &discard,
@@ -100,13 +100,22 @@ class SDLPortBackend : public PortBackend {
100100
SDL_GetError());
101101
sdl_buffer_frames = 0;
102102
}
103-
return std::max<u32>(buffer_size, sdl_buffer_frames * frame_size) * 4;
103+
const auto sdl_buffer_size = sdl_buffer_frames * frame_size;
104+
const auto new_threshold = std::max(guest_buffer_size, sdl_buffer_size) * 4;
105+
if (host_buffer_size != sdl_buffer_size || queue_threshold != new_threshold) {
106+
host_buffer_size = sdl_buffer_size;
107+
queue_threshold = new_threshold;
108+
LOG_INFO(Lib_AudioOut,
109+
"SDL audio buffers: guest = {} bytes, host = {} bytes, threshold = {} bytes",
110+
guest_buffer_size, host_buffer_size, queue_threshold);
111+
}
104112
}
105113

106114
u32 frame_size;
107-
u32 buffer_size;
108-
u32 queue_threshold;
109-
SDL_AudioStream* stream;
115+
u32 guest_buffer_size;
116+
u32 host_buffer_size{};
117+
u32 queue_threshold{};
118+
SDL_AudioStream* stream{};
110119
};
111120

112121
std::unique_ptr<PortBackend> SDLAudioOut::Open(PortOut& port) {

0 commit comments

Comments
 (0)