From 87e6837980e265b697709ae3a62d2098b691f974 Mon Sep 17 00:00:00 2001
From: Leon Styhre <leon@leonstyhre.com>
Date: Thu, 18 Mar 2021 21:55:56 +0100
Subject: [PATCH] Complete overhaul of VolumeControl with fixes for some
 related bugs.

---
 es-app/src/FileData.cpp      |   1 -
 es-app/src/VolumeControl.cpp | 121 ++++++++++-------------------------
 es-app/src/VolumeControl.h   |  45 +++++--------
 es-app/src/guis/GuiMenu.cpp  |   4 ++
 es-core/src/AudioManager.cpp |   2 +-
 5 files changed, 56 insertions(+), 117 deletions(-)

diff --git a/es-app/src/FileData.cpp b/es-app/src/FileData.cpp
index 623790a26..dbe74b671 100644
--- a/es-app/src/FileData.cpp
+++ b/es-app/src/FileData.cpp
@@ -24,7 +24,6 @@
 #include "Platform.h"
 #include "Scripting.h"
 #include "SystemData.h"
-#include "VolumeControl.h"
 #include "Window.h"
 
 #include <assert.h>
diff --git a/es-app/src/VolumeControl.cpp b/es-app/src/VolumeControl.cpp
index f659a7899..0a9261aa0 100644
--- a/es-app/src/VolumeControl.cpp
+++ b/es-app/src/VolumeControl.cpp
@@ -21,9 +21,7 @@
 
 // The ALSA Audio Card and Audio Device selection code is disabled at the moment.
 // As PulseAudio controls the sound devices for the desktop environment, it doesn't
-// make much sense to be able to select ALSA devices directly. Normally (always?)
-// the selection doesn't make any difference at all. But maybe some PulseAudio
-// settings could be added later on, if needed.
+// make much sense to be able to select ALSA devices directly.
 // The code is still active for Raspberry Pi though as I'm not sure if this is
 // useful for that device.
 // Keeping mixerName and mixerCard at their default values should make sure that
@@ -37,82 +35,51 @@ std::string VolumeControl::mixerName = "Master";
 std::string VolumeControl::mixerCard = "default";
 #endif
 
-std::weak_ptr<VolumeControl> VolumeControl::sInstance;
+VolumeControl* VolumeControl::sInstance = nullptr;
 
 VolumeControl::VolumeControl()
-        : originalVolume(0),
-        internalVolume(0)
-        #if defined(__APPLE__)
-//        #error TODO: Not implemented for MacOS yet!!!
-        #elif defined(__linux__)
-        , mixerIndex(0),
+        #if defined(__linux__)
+        : mixerIndex(0),
         mixerHandle(nullptr),
         mixerElem(nullptr),
         mixerSelemId(nullptr)
         #elif defined(_WIN64)
-        , mixerHandle(nullptr),
+        : mixerHandle(nullptr),
         endpointVolume(nullptr)
         #endif
 {
     init();
-
-    // Get original volume levels for system.
-    originalVolume = getVolume();
-}
-
-VolumeControl::VolumeControl(
-        const VolumeControl& right):
-        originalVolume(0),
-        internalVolume(0)
-        #if defined(__APPLE__)
-//        #error TODO: Not implemented for MacOS yet!!!
-        #elif defined(__linux__)
-        , mixerIndex(0),
-        mixerHandle(nullptr),
-        mixerElem(nullptr),
-        mixerSelemId(nullptr)
-        #elif defined(_WIN64)
-        , mixerHandle(nullptr),
-        endpointVolume(nullptr)
-        #endif
-{
-    static_cast<void>(right);
-    sInstance = right.sInstance;
-}
-
-VolumeControl& VolumeControl::operator=(const VolumeControl& right)
-{
-    if (this != &right)
-        sInstance = right.sInstance;
-
-    return *this;
 }
 
 VolumeControl::~VolumeControl()
 {
-    // Set original volume levels for system.
-    //setVolume(originalVolume);
-
     deinit();
+    #if defined(__linux__)
+    snd_config_update_free_global();
+    #endif
 }
 
-std::shared_ptr<VolumeControl>& VolumeControl::getInstance()
+VolumeControl* VolumeControl::getInstance()
 {
-    // Check if an VolumeControl instance is already created, if not then create it.
-    static std::shared_ptr<VolumeControl> sharedInstance = sInstance.lock();
-    if (sharedInstance == nullptr) {
-        sharedInstance.reset(new VolumeControl);
-        sInstance = sharedInstance;
+    // Check if a VolumeControl instance is already created, and if not then create it.
+    if (!sInstance)
+        sInstance = new VolumeControl();
+
+    return sInstance;
+}
+
+void VolumeControl::deleteInstance()
+{
+    if (sInstance) {
+        delete sInstance;
+        sInstance = nullptr;
     }
-    return sharedInstance;
 }
 
 void VolumeControl::init()
 {
     // Initialize audio mixer interface.
-    #if defined(__APPLE__)
-//    #error TODO: Not implemented for MacOS yet!!!
-    #elif defined(__linux__)
+    #if defined(__linux__)
     // Try to open mixer device.
     if (mixerHandle == nullptr) {
         // Allow user to override the AudioCard and AudioDevice in es_settings.cfg.
@@ -125,22 +92,17 @@ void VolumeControl::init()
         // Sets simple-mixer index and name.
         snd_mixer_selem_id_set_index(mixerSelemId, mixerIndex);
         snd_mixer_selem_id_set_name(mixerSelemId, mixerName.c_str());
-        // Open mixer.
         if (snd_mixer_open(&mixerHandle, 0) >= 0) {
             LOG(LogDebug) << "VolumeControl::init(): Opened ALSA mixer";
-            // Ok, attach to defualt card.
             if (snd_mixer_attach(mixerHandle, mixerCard.c_str()) >= 0) {
                 LOG(LogDebug) << "VolumeControl::init(): Attached to default card";
-                // Ok, register simple element class.
                 if (snd_mixer_selem_register(mixerHandle, nullptr, nullptr) >= 0) {
                     LOG(LogDebug) << "VolumeControl::init(): Registered simple element class";
-                    // Ok, load registered elements.
                     if (snd_mixer_load(mixerHandle) >= 0) {
                         LOG(LogDebug) << "VolumeControl::init(): Loaded mixer elements";
-                        // Ok, find elements now.
+                        // Find elements.
                         mixerElem = snd_mixer_find_selem(mixerHandle, mixerSelemId);
                         if (mixerElem != nullptr) {
-                            // Wohoo. good to go...
                             LOG(LogDebug) << "VolumeControl::init(): Mixer initialized";
                         }
                         else {
@@ -213,14 +175,11 @@ void VolumeControl::init()
 void VolumeControl::deinit()
 {
     // Deinitialize audio mixer interface.
-    #if defined(__APPLE__)
-//    #error TODO: Not implemented for MacOS yet!!!
-    #elif defined(__linux__)
+    #if defined(__linux__)
     if (mixerHandle != nullptr) {
         snd_mixer_detach(mixerHandle, mixerCard.c_str());
         snd_mixer_free(mixerHandle);
         snd_mixer_close(mixerHandle);
-        snd_config_update_free_global();
         mixerHandle = nullptr;
         mixerElem = nullptr;
     }
@@ -237,36 +196,31 @@ int VolumeControl::getVolume() const
 {
     int volume = 0;
 
-    #if defined(__APPLE__)
-//    #error TODO: Not implemented for MacOS yet!!!
-    #elif defined(__linux__)
+    #if defined(__linux__)
     if (mixerElem != nullptr) {
         // Get volume range.
         long minVolume;
         long maxVolume;
         if (snd_mixer_selem_get_playback_volume_range(mixerElem, &minVolume, &maxVolume) == 0) {
-            // Ok, now get volume.
             long rawVolume;
             if (snd_mixer_selem_get_playback_volume(mixerElem,
                     SND_MIXER_SCHN_MONO, &rawVolume) == 0) {
-                // Worked. bring into range 0-100.
+                // Bring into range 0-100.
                 rawVolume -= minVolume;
                 if (rawVolume > 0)
                     volume = (rawVolume * 100.0) / (maxVolume - minVolume) + 0.5;
-                //else
-                //   volume = 0;
             }
             else {
-                LOG(LogError) << "VolumeControl::getVolume(): Failed to get mixer volume!";
+                LOG(LogError) << "VolumeControl::getVolume(): Failed to get mixer volume";
             }
         }
         else {
-            LOG(LogError) << "VolumeControl::getVolume(): Failed to get volume range!";
+            LOG(LogError) << "VolumeControl::getVolume(): Failed to get volume range";
         }
     }
     #elif defined(_WIN64)
     if (endpointVolume != nullptr) {
-        // Windows Vista or above. uses EndpointVolume API.
+        // Windows Vista or above, uses EndpointVolume API.
         float floatVolume = 0.0f; // 0-1
         if (endpointVolume->GetMasterVolumeLevelScalar(&floatVolume) == S_OK) {
             volume = static_cast<int>(std::round(floatVolume * 100.0f));
@@ -278,38 +232,31 @@ int VolumeControl::getVolume() const
     }
     #endif
 
-    // Clamp to 0-100 range.
     volume = Math::clamp(volume, 0, 100);
-
     return volume;
 }
 
 void VolumeControl::setVolume(int volume)
 {
-    // Clamp to 0-100 range.
     volume = Math::clamp(volume, 0, 100);
 
-    // Store values in internal variables.
-    internalVolume = volume;
-    #if defined(__APPLE__)
-//    #error TODO: Not implemented for MacOS yet!!!
-    #elif defined(__linux__)
+    #if defined(__linux__)
     if (mixerElem != nullptr) {
         // Get volume range.
         long minVolume;
         long maxVolume;
         if (snd_mixer_selem_get_playback_volume_range(mixerElem, &minVolume, &maxVolume) == 0) {
-            // Ok, bring into minVolume-maxVolume range and set.
+            // Bring into minVolume-maxVolume range and set.
             long rawVolume = (volume * (maxVolume - minVolume) / 100) + minVolume;
             if (snd_mixer_selem_set_playback_volume(mixerElem,
                     SND_MIXER_SCHN_FRONT_LEFT, rawVolume) < 0 ||
                     snd_mixer_selem_set_playback_volume(mixerElem,
                     SND_MIXER_SCHN_FRONT_RIGHT, rawVolume) < 0) {
-                LOG(LogError) << "VolumeControl::getVolume(): Failed to set mixer volume!";
+                LOG(LogError) << "VolumeControl::getVolume(): Failed to set mixer volume";
             }
         }
         else {
-            LOG(LogError) << "VolumeControl::getVolume(): Failed to get volume range!";
+            LOG(LogError) << "VolumeControl::getVolume(): Failed to get volume range";
         }
     }
     #elif defined(_WIN64)
@@ -319,7 +266,7 @@ void VolumeControl::setVolume(int volume)
         if (volume > 0)
             floatVolume = static_cast<float>(volume) / 100.0f;
         if (endpointVolume->SetMasterVolumeLevelScalar(floatVolume, nullptr) != S_OK)
-            LOG(LogError) << "VolumeControl::setVolume(): Failed to set master volume!";
+            LOG(LogError) << "VolumeControl::setVolume(): Failed to set master volume";
     }
     #endif
 }
diff --git a/es-app/src/VolumeControl.h b/es-app/src/VolumeControl.h
index 3bc5e9844..624a98b02 100644
--- a/es-app/src/VolumeControl.h
+++ b/es-app/src/VolumeControl.h
@@ -11,9 +11,7 @@
 
 #include <memory>
 
-#if defined(__APPLE__)
-//#error TODO: Not implemented for MacOS yet!!!
-#elif defined(__linux__)
+#if defined(__linux__)
 #include <alsa/asoundlib.h>
 #include <fcntl.h>
 #include <unistd.h>
@@ -21,15 +19,26 @@
 #include <Windows.h>
 #include <endpointvolume.h>
 #include <mmdeviceapi.h>
-#include <mmsystem.h>
 #endif
 
-//  Singleton pattern. Call getInstance() to get an object.
 class VolumeControl
 {
-    #if defined(__APPLE__)
-//    #error TODO: Not implemented for MacOS yet!!!
-    #elif defined(__linux__)
+public:
+    VolumeControl();
+    ~VolumeControl();
+
+    static VolumeControl* getInstance();
+    void deleteInstance();
+
+    void init();
+    void deinit();
+
+    int getVolume() const;
+    void setVolume(int volume);
+
+    static VolumeControl* sInstance;
+
+    #if defined(__linux__)
     static std::string mixerName;
     static std::string mixerCard;
     int mixerIndex;
@@ -41,26 +50,6 @@ class VolumeControl
     MIXERCONTROL mixerControl;
     IAudioEndpointVolume * endpointVolume;
     #endif
-
-    int originalVolume;
-    int internalVolume;
-
-    static std::weak_ptr<VolumeControl> sInstance;
-
-    VolumeControl();
-    VolumeControl(const VolumeControl & right);
-    VolumeControl & operator=(const VolumeControl & right);
-
-public:
-    static std::shared_ptr<VolumeControl> & getInstance();
-
-    void init();
-    void deinit();
-
-    int getVolume() const;
-    void setVolume(int volume);
-
-    ~VolumeControl();
 };
 
 #endif // ES_APP_VOLUME_CONTROL_H
diff --git a/es-app/src/guis/GuiMenu.cpp b/es-app/src/guis/GuiMenu.cpp
index 3b86b0b78..85630ddb8 100644
--- a/es-app/src/guis/GuiMenu.cpp
+++ b/es-app/src/guis/GuiMenu.cpp
@@ -513,6 +513,10 @@ void GuiMenu::openSoundSettings()
     s->addSaveFunc([system_volume] {
         VolumeControl::getInstance()->
                 setVolume(static_cast<int>(std::round(system_volume->getValue())));
+        // Explicitly delete the VolumeControl instance so that it will reinitialize the
+        // next time the menu is entered. This is the easiest way to detect new default
+        // audio devices or changes in audio volume done by the operating system.
+        VolumeControl::getInstance()->deleteInstance();
     });
     #endif
 
diff --git a/es-core/src/AudioManager.cpp b/es-core/src/AudioManager.cpp
index c5c0d3963..8b091ccea 100644
--- a/es-core/src/AudioManager.cpp
+++ b/es-core/src/AudioManager.cpp
@@ -33,7 +33,7 @@ AudioManager::~AudioManager()
 
 std::shared_ptr<AudioManager>& AudioManager::getInstance()
 {
-    // Check if an AudioManager instance is already created, if not, create it.
+    // Check if an AudioManager instance is already created, and if not then create it.
     if (sInstance == nullptr)
         sInstance = std::shared_ptr<AudioManager>(new AudioManager);