[vlc-devel] [PATCH 6/7] qt: add AudioDeviceConfigControl

[Rémi Denis-Courmont]

Le 27 février 2018 12:41:10 GMT+02:00, Thomas Guillem <thomas at gllm.fr> a écrit :
>Same as StringListConfigControl() but it also calls aout_DeviceSet()
>when
>applying the configuration.
>---
>modules/gui/qt/components/preferences_widgets.cpp | 19
>+++++++++++++++++++
> modules/gui/qt/components/preferences_widgets.hpp | 15 +++++++++++++--
> 2 files changed, 32 insertions(+), 2 deletions(-)
>
>diff --git a/modules/gui/qt/components/preferences_widgets.cpp
>b/modules/gui/qt/components/preferences_widgets.cpp
>index 1419bac114..6bbcb91765 100644
>--- a/modules/gui/qt/components/preferences_widgets.cpp
>+++ b/modules/gui/qt/components/preferences_widgets.cpp
>@@ -35,6 +35,7 @@
> #include "util/customwidgets.hpp"
> #include "util/searchlineedit.hpp"
> #include "util/qt_dirs.hpp"
>+#include "input_manager.hpp"
> #include <vlc_actions.h>
> #include <vlc_intf_strings.h>
> #include <vlc_modules.h>
>@@ -466,6 +467,24 @@ QString StringListConfigControl::getValue() const
>     return combo->itemData( combo->currentIndex() ).toString();
> }
> 
>+AudioDeviceConfigControl::AudioDeviceConfigControl( intf_thread_t
>*_p_intf,
>+                module_config_t *_p_item, QLabel *_label, QComboBox
>*_combo )
>+    : StringListConfigControl( VLC_OBJECT(_p_intf), _p_item, _label,
>_combo, false )
>+    , p_intf(_p_intf)
>+{
>+}
>+
>+void AudioDeviceConfigControl::doApply()
>+{
>+    StringListConfigControl::doApply();
>+    audio_output_t *aout = THEMIM->getAout();
>+    if (aout != NULL)
>+    {
>+        aout_DeviceSet(aout, qtu(getValue()));
>+        vlc_object_release(aout);
>+    }
>+}
>+
>void setfillVLCConfigCombo( const char *configname, intf_thread_t
>*p_intf,
>                             QComboBox *combo )
> {
>diff --git a/modules/gui/qt/components/preferences_widgets.hpp
>b/modules/gui/qt/components/preferences_widgets.hpp
>index 5f516fbd5d..00b466fd13 100644
>--- a/modules/gui/qt/components/preferences_widgets.hpp
>+++ b/modules/gui/qt/components/preferences_widgets.hpp
>@@ -302,7 +302,7 @@ class VStringConfigControl : public ConfigControl
> public:
>     virtual QString getValue() const = 0;
>     int getType() const Q_DECL_OVERRIDE;
>-    void doApply() Q_DECL_OVERRIDE;
>+    virtual void doApply() Q_DECL_OVERRIDE;
> protected:
>     VStringConfigControl( vlc_object_t *a, module_config_t *b ) :
>                 ConfigControl(a,b) {}
>@@ -451,13 +451,24 @@ protected:
>     void fillGrid( QGridLayout*, int ) Q_DECL_OVERRIDE;
>     QComboBox *combo;
> private:
>-    void finish(module_config_t * );
>+    virtual void finish(module_config_t * );
>     QLabel *label;
>     QList<QPushButton *> buttons;
> private slots:
>     void comboIndexChanged( int );
> };
> 
>+class AudioDeviceConfigControl : public StringListConfigControl
>+{
>+    Q_OBJECT
>+public:
>+    AudioDeviceConfigControl( intf_thread_t *, module_config_t *,
>QLabel *,
>+                              QComboBox* );
>+    virtual void doApply() Q_DECL_OVERRIDE;
>+private:
>+    intf_thread_t *p_intf;
>+};
>+
>void setfillVLCConfigCombo(const char *configname, intf_thread_t
>*p_intf,
>                         QComboBox *combo );
> 
>-- 
>2.11.0
>
>_______________________________________________
>vlc-devel mailing list
>To unsubscribe or modify your subscription options:
>https://mailman.videolan.org/listinfo/vlc-devel

This is a bad idea on multiple levels.

At technical level, there are no obvious or documented reasons why audio volume should be a special snowflake in preferences. There are plenty of settings that could be applied without restart through custom code, including many in the simple preferences.

On user level, there is a clear and simple story now that VLC should be restarted after changing preferences. This only makes stuff more complicated. This ia bound to cascade into even more support problems as users will expect everything to work like audio volume.

Unlike the ancient preferences live update code, this has no glaring bugs. But it looks like setting a very dangerous precedent / stepping on a slippery slope.
-- 
Remi Denis-Courmont