Merge pull request #2 from extramaster/audio-2

Audio Core: Response to the main repo's PR Recommendations

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,13 +24,9 @@ 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() {
@@ -43,6 +42,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 +60,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"