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

@@ -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();

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();
 

src/audio_core/null_sink.h

@@ -28,7 +28,7 @@ public:
 
     void SetDevice(int device_id) override {}
 
-    std::map<int, std::string>* GetDeviceMap() override {
+    std::vector<std::string>* GetDeviceMap() override {
         return nullptr;
     }
 };

src/audio_core/sdl2_sink.cpp

@@ -50,12 +50,12 @@ SDL2Sink::SDL2Sink() : impl(std::make_unique<Impl>()) {
     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<std::string>* SDL2Sink::GetDeviceMap() {
+    return &device_map;
+}
+
 void SDL2Sink::EnqueueSamples(const std::vector<s16>& 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<int, std::string>* SDL2Sink::GetDeviceMap() {
-    return &device_map;
-}
-
 void SDL2Sink::Impl::Callback(void* impl_, u8* buffer, int buffer_size_in_bytes) {
     Impl* impl = reinterpret_cast<Impl*>(impl_);
 

src/audio_core/sdl2_sink.h

@@ -23,16 +23,14 @@ public:
 
     size_t SamplesInQueue() const override;
 
-    std::map<int, std::string>* GetDeviceMap();
+    std::vector<std::string>* GetDeviceMap() override;
     void SetDevice(int _device_id);
 
-
 private:
     struct Impl;
     std::unique_ptr<Impl> impl;
     int device_id;
-    std::map<int, std::string> device_map;
+    std::vector<std::string> device_map;
 };
 
-
 } // namespace AudioCore

src/audio_core/sink.h

@@ -5,7 +5,6 @@
 #pragma once
 
 #include <vector>
-#include <map>
 
 #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<int, std::string>* GetDeviceMap() = 0;
+    virtual std::vector<std::string>* GetDeviceMap() = 0;
 };
 
 } // namespace

src/audio_core/sink_details.cpp

@@ -22,11 +22,4 @@ const std::vector<SinkDetails> g_sink_details = {
     { "null", []() { return std::make_unique<NullSink>(); } },
 };
 
-#ifdef HAVE_SDL2
-SDL2Sink sink;
-const std::map<int, std::string> g_device_map = *sink.GetDeviceMap();
-#else
-const std::map<int, std::string> g_device_map = { {0, "null"} };
-#endif
-
 } // namespace AudioCore

src/audio_core/sink_details.h

@@ -7,7 +7,6 @@
 #include <functional>
 #include <memory>
 #include <vector>
-#include <map>
 
 namespace AudioCore {
 
@@ -24,6 +23,5 @@ struct SinkDetails {
 };
 
 extern const std::vector<SinkDetails> g_sink_details;
-extern const std::map<int, std::string> g_device_map;
 
 } // namespace AudioCore

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);

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 <memory>
 
 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());
+        }
+    }
+}

src/citra_qt/configure_audio.h

@@ -20,6 +20,9 @@ public:
 
     void applyConfiguration();
 
+public slots:
+    void updateAudioDevices(int sinkIndex);
+
 private:
     void setConfiguration();
 

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

src/core/settings.h

@@ -6,7 +6,6 @@
 
 #include <string>
 #include <array>
-#include <map>
 
 #include "common/common_types.h"