From 96cba3e719ad82276ee21e5347c9416ddc64de63 Mon Sep 17 00:00:00 2001 From: Andy Tran Date: Fri, 15 Jul 2016 10:42:13 +1000 Subject: [PATCH] Audio Core: Implemented feedback from comments Worked with the Sink abstraction and tuned the "Device Selection" configuration so that the Device List is automatically populated when the Sink is changed. This hopefully addresses the concerns and recommendations mentioned in the comments of the PR. --- src/audio_core/audio_core.cpp | 7 ------ src/audio_core/audio_core.h | 3 --- src/audio_core/null_sink.h | 2 +- src/audio_core/sdl2_sink.cpp | 12 +++++------ src/audio_core/sdl2_sink.h | 6 ++---- src/audio_core/sink.h | 3 +-- src/audio_core/sink_details.cpp | 7 ------ src/audio_core/sink_details.h | 2 -- src/citra/config.cpp | 1 + src/citra_qt/configure_audio.cpp | 37 ++++++++++++++++++++++++++------ src/citra_qt/configure_audio.h | 3 +++ src/core/settings.cpp | 1 - src/core/settings.h | 1 - 13 files changed, 45 insertions(+), 40 deletions(-) diff --git a/src/audio_core/audio_core.cpp b/src/audio_core/audio_core.cpp index 76839cdf8..d42249ebd 100644 --- a/src/audio_core/audio_core.cpp +++ b/src/audio_core/audio_core.cpp @@ -71,13 +71,6 @@ void SelectSink(std::string sink_id) { DSP::HLE::SetSink(iter->factory()); } -void SelectDevice(std::string device_id){ - if (device_id == "") { - LOG_ERROR(Audio, "AudioCore::SelectDevice given invalid device_id"); - return; - } -} - void Shutdown() { CoreTiming::UnscheduleEvent(tick_event, 0); DSP::HLE::Shutdown(); diff --git a/src/audio_core/audio_core.h b/src/audio_core/audio_core.h index bd8622411..f618361f3 100644 --- a/src/audio_core/audio_core.h +++ b/src/audio_core/audio_core.h @@ -23,9 +23,6 @@ void AddAddressSpace(Kernel::VMManager& vm_manager); /// Select the sink to use based on sink id. void SelectSink(std::string sink_id); -/// Select the device to use based on device id. -void SelectDevice(std::string device_id); - /// Shutdown Audio Core void Shutdown(); diff --git a/src/audio_core/null_sink.h b/src/audio_core/null_sink.h index 619302d37..b7b182879 100644 --- a/src/audio_core/null_sink.h +++ b/src/audio_core/null_sink.h @@ -28,7 +28,7 @@ public: void SetDevice(int device_id) override {} - std::map* GetDeviceMap() override { + std::vector* GetDeviceMap() override { return nullptr; } }; diff --git a/src/audio_core/sdl2_sink.cpp b/src/audio_core/sdl2_sink.cpp index 6426d20f3..34c9da09d 100644 --- a/src/audio_core/sdl2_sink.cpp +++ b/src/audio_core/sdl2_sink.cpp @@ -50,12 +50,12 @@ SDL2Sink::SDL2Sink() : impl(std::make_unique()) { int device_count = SDL_GetNumAudioDevices(0); device_map.clear(); for (int i = 0; i < device_count; ++i) { - device_map[i] = SDL_GetAudioDeviceName(i, 0); + device_map.push_back(SDL_GetAudioDeviceName(i, 0)); } if (device_count < 1 || Settings::values.audio_device_id == "auto" || - Settings::values.audio_device_id == "") { + Settings::values.audio_device_id.empty()) { impl->audio_device_id = SDL_OpenAudioDevice(nullptr, false, &desired_audiospec, &obtained_audiospec, SDL_AUDIO_ALLOW_ANY_CHANGE); if (impl->audio_device_id <= 0) { LOG_CRITICAL(Audio_Sink, "SDL_OpenAudioDevice failed with code %d for device \"auto\"", impl->audio_device_id); @@ -91,6 +91,10 @@ unsigned int SDL2Sink::GetNativeSampleRate() const { return impl->sample_rate; } +std::vector* SDL2Sink::GetDeviceMap() { + return &device_map; +} + void SDL2Sink::EnqueueSamples(const std::vector& samples) { if (impl->audio_device_id <= 0) return; @@ -123,10 +127,6 @@ void SDL2Sink::SetDevice(int _device_id) { this->device_id = _device_id; } -std::map* SDL2Sink::GetDeviceMap() { - return &device_map; -} - void SDL2Sink::Impl::Callback(void* impl_, u8* buffer, int buffer_size_in_bytes) { Impl* impl = reinterpret_cast(impl_); diff --git a/src/audio_core/sdl2_sink.h b/src/audio_core/sdl2_sink.h index 6552c088e..b901de863 100644 --- a/src/audio_core/sdl2_sink.h +++ b/src/audio_core/sdl2_sink.h @@ -23,16 +23,14 @@ public: size_t SamplesInQueue() const override; - std::map* GetDeviceMap(); + std::vector* GetDeviceMap() override; void SetDevice(int _device_id); - private: struct Impl; std::unique_ptr impl; int device_id; - std::map device_map; + std::vector device_map; }; - } // namespace AudioCore diff --git a/src/audio_core/sink.h b/src/audio_core/sink.h index 23f621193..3c7e53339 100644 --- a/src/audio_core/sink.h +++ b/src/audio_core/sink.h @@ -5,7 +5,6 @@ #pragma once #include -#include #include "common/common_types.h" @@ -32,7 +31,7 @@ public: virtual std::size_t SamplesInQueue() const = 0; virtual void SetDevice(int device_id) = 0; - virtual std::map* GetDeviceMap() = 0; + virtual std::vector* GetDeviceMap() = 0; }; } // namespace diff --git a/src/audio_core/sink_details.cpp b/src/audio_core/sink_details.cpp index 9ee92ac0e..ba5e83d17 100644 --- a/src/audio_core/sink_details.cpp +++ b/src/audio_core/sink_details.cpp @@ -22,11 +22,4 @@ const std::vector g_sink_details = { { "null", []() { return std::make_unique(); } }, }; -#ifdef HAVE_SDL2 -SDL2Sink sink; -const std::map g_device_map = *sink.GetDeviceMap(); -#else -const std::map g_device_map = { {0, "null"} }; -#endif - } // namespace AudioCore diff --git a/src/audio_core/sink_details.h b/src/audio_core/sink_details.h index 783a314a5..4b30cf835 100644 --- a/src/audio_core/sink_details.h +++ b/src/audio_core/sink_details.h @@ -7,7 +7,6 @@ #include #include #include -#include namespace AudioCore { @@ -24,6 +23,5 @@ struct SinkDetails { }; extern const std::vector g_sink_details; -extern const std::map g_device_map; } // namespace AudioCore diff --git a/src/citra/config.cpp b/src/citra/config.cpp index 22cb51ea8..be33d9815 100644 --- a/src/citra/config.cpp +++ b/src/citra/config.cpp @@ -78,6 +78,7 @@ void Config::ReadValues() { // Audio Settings::values.sink_id = sdl2_config->Get("Audio", "output_engine", "auto"); + Settings::values.audio_device_id = sdl2_config->Get("Audio", "output_device", "auto"); // Data Storage Settings::values.use_virtual_sd = sdl2_config->GetBoolean("Data Storage", "use_virtual_sd", true); diff --git a/src/citra_qt/configure_audio.cpp b/src/citra_qt/configure_audio.cpp index 5db2077ea..669f5c0a8 100644 --- a/src/citra_qt/configure_audio.cpp +++ b/src/citra_qt/configure_audio.cpp @@ -2,12 +2,15 @@ // Licensed under GPLv2 or any later version // Refer to the license.txt file included. +#include "audio_core/audio_core.h" +#include "audio_core/sink.h" #include "audio_core/sink_details.h" #include "citra_qt/configure_audio.h" #include "ui_configure_audio.h" #include "core/settings.h" +#include ConfigureAudio::ConfigureAudio(QWidget* parent) : QWidget(parent), @@ -21,18 +24,15 @@ ConfigureAudio::ConfigureAudio(QWidget* parent) : ui->output_sink_combo_box->addItem(sink_detail.id); } - ui->audio_device_combo_box->clear(); - ui->audio_device_combo_box->addItem("auto"); - for (const auto& device : AudioCore::g_device_map) { - ui->audio_device_combo_box->addItem(device.second.c_str()); - } - this->setConfiguration(); + connect(ui->output_sink_combo_box, SIGNAL(currentIndexChanged(int)), + this, SLOT(updateAudioDevices(int))); } ConfigureAudio::~ConfigureAudio() { } + void ConfigureAudio::setConfiguration() { int new_sink_index = 0; for (int index = 0; index < ui->output_sink_combo_box->count(); index++) { @@ -43,6 +43,8 @@ void ConfigureAudio::setConfiguration() { } ui->output_sink_combo_box->setCurrentIndex(new_sink_index); + // The device list cannot be pre-populated (nor listed) until the output sink is known. + updateAudioDevices(new_sink_index); int new_device_index = -1; for (int index = 0; index < ui->audio_device_combo_box->count(); index++) { @@ -59,3 +61,26 @@ void ConfigureAudio::applyConfiguration() { Settings::values.audio_device_id = ui->audio_device_combo_box->itemText(ui->audio_device_combo_box->currentIndex()).toStdString(); Settings::Apply(); } + +void ConfigureAudio::updateAudioDevices(int sink_index) { + + ui->audio_device_combo_box->clear(); + ui->audio_device_combo_box->addItem("auto"); + + // If the current sink is set to "auto", the "front" sink is selected. + auto quick_sink_populate_device = AudioCore::g_sink_details[0]; + if (sink_index > 0) { + + // Case where the sink is pointed to a directly known sink device (NullSink, SDL2Sink). + // The first index (0) should be "auto", which is not in the vector list (-1). + quick_sink_populate_device = AudioCore::g_sink_details[sink_index - 1]; + } + auto iter = quick_sink_populate_device.factory(); + + // Prevent accessing a null device list. + if (std::move(iter)->GetDeviceMap()) { + for (const auto& device : *(std::move(iter)->GetDeviceMap())) { + ui->audio_device_combo_box->addItem(device.c_str()); + } + } +} \ No newline at end of file diff --git a/src/citra_qt/configure_audio.h b/src/citra_qt/configure_audio.h index 51df2e27b..40fbf7f05 100644 --- a/src/citra_qt/configure_audio.h +++ b/src/citra_qt/configure_audio.h @@ -20,6 +20,9 @@ public: void applyConfiguration(); +public slots: + void updateAudioDevices(int sinkIndex); + private: void setConfiguration(); diff --git a/src/core/settings.cpp b/src/core/settings.cpp index f4ee129ee..2e27e381a 100644 --- a/src/core/settings.cpp +++ b/src/core/settings.cpp @@ -24,7 +24,6 @@ void Apply() { VideoCore::g_scaled_resolution_enabled = values.use_scaled_resolution; AudioCore::SelectSink(values.sink_id); - AudioCore::SelectDevice(values.audio_device_id); } } // namespace diff --git a/src/core/settings.h b/src/core/settings.h index 3ae14f008..245470167 100644 --- a/src/core/settings.h +++ b/src/core/settings.h @@ -6,7 +6,6 @@ #include #include -#include #include "common/common_types.h"