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

[Thomas Guillem]

On Tue, Feb 27, 2018, at 14:39, Rémi Denis-Courmont wrote:
> 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.

We are in 2018, I don't think any new users will expect to restart a program when they change a preference.
I'm not saying to change everything now, but we should focus on improving it for future VLC versions.

> 
> 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
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel