[vlc-commits] [Git][videolan/vlc][master] 11 commits: qt: remove QSIGNALMAPPER_XXX macros

Steve Lhomme (@robUx4) gitlab at videolan.org
Sun Sep 22 11:57:38 UTC 2024



Steve Lhomme pushed to branch master at VideoLAN / VLC


Commits:
b0a4c4aa by Pierre Lamot at 2024-09-22T11:42:55+00:00
qt: remove QSIGNALMAPPER_XXX macros

these macro where here for older Qt5 compatibility, they are not needed anymore

- - - - -
3b1c54b5 by Pierre Lamot at 2024-09-22T11:42:55+00:00
qt: remove unused signal in ExtensionDialog

- - - - -
35c4123f by Pierre Lamot at 2024-09-22T11:42:55+00:00
qt: remove unused member in ExtensionDialog

- - - - -
e763881d by Pierre Lamot at 2024-09-22T11:42:55+00:00
qt: factorise dialog creation and update in ExtensionDialog

- - - - -
6388aa95 by Pierre Lamot at 2024-09-22T11:42:55+00:00
qt: pass object instance to vlc_dialog_provider_set_ext_callback

no need to rely on singleton feature of the class to retreive the instance

- - - - -
5ce99a11 by Pierre Lamot at 2024-09-22T11:42:55+00:00
qt: refactor extension DialogCallback using Qt AutoConnection

* either we are in extension thread and this is safe to carry the extension_dialog_t
  through queued connection as the extension wait that we signal its condition
  variable

* either we are in Qt thread (during destruction for instance) and we want
  direct connection

- - - - -
f89e0d0c by Pierre Lamot at 2024-09-22T11:42:55+00:00
qt: remove necessary forward declaration in ExtensionDialog

- - - - -
e35094fe by Pierre Lamot at 2024-09-22T11:42:55+00:00
qt: fix extension unregistration order issue

- - - - -
502662d7 by Pierre Lamot at 2024-09-22T11:42:55+00:00
qt: remove unused CTor property from ExtensionsDialogProvider

- - - - -
92f10b6b by Pierre Lamot at 2024-09-22T11:42:55+00:00
qt: ExtensionsDialogProvider doesn't need to be a singleton

it's only owned and accessed by the ExtensionsManager

- - - - -
469a286e by Pierre Lamot at 2024-09-22T11:42:55+00:00
qt: remove extraneous QObject instance in DialogExtension

mapping can be centralized, no need for an additional QObject

- - - - -


9 changed files:

- modules/gui/qt/dialogs/extensions/extensions.cpp
- modules/gui/qt/dialogs/extensions/extensions.hpp
- modules/gui/qt/dialogs/extensions/extensions_manager.cpp
- modules/gui/qt/dialogs/extensions/extensions_manager.hpp
- modules/gui/qt/dialogs/plugins/plugins.cpp
- modules/gui/qt/dialogs/preferences/simple_preferences.cpp
- modules/gui/qt/menus/qml_menu_wrapper.cpp
- modules/gui/qt/player/player_controller.cpp
- modules/gui/qt/qt.hpp


Changes:

=====================================
modules/gui/qt/dialogs/extensions/extensions.cpp
=====================================
@@ -46,20 +46,19 @@ static void DialogCallback( extension_dialog_t *p_ext_dialog,
                             void *p_data );
 
 
-ExtensionsDialogProvider::ExtensionsDialogProvider( qt_intf_t *_p_intf,
-                                                    extensions_manager_t *p_mgr )
-        : QObject( NULL ), p_intf( _p_intf ), p_extensions_manager( p_mgr )
+ExtensionsDialogProvider::ExtensionsDialogProvider(qt_intf_t *_p_intf)
+        : QObject( NULL ), p_intf( _p_intf )
 {
     assert(p_intf);
-    assert(p_extensions_manager);
-
-    vlc_dialog_provider_set_ext_callback( p_intf, DialogCallback, NULL );
+    vlc_dialog_provider_set_ext_callback( p_intf, DialogCallback, this );
 }
 
 ExtensionsDialogProvider::~ExtensionsDialogProvider()
 {
     msg_Dbg( p_intf, "ExtensionsDialogProvider is quitting..." );
-    vlc_dialog_provider_set_ext_callback( p_intf, NULL, NULL );
+    for (ExtensionDialog* dialog: m_dialogs)
+        delete dialog;
+    vlc_dialog_provider_set_ext_callback( p_intf, nullptr, nullptr );
 }
 
 /** Create a dialog
@@ -68,11 +67,8 @@ ExtensionDialog* ExtensionsDialogProvider::CreateExtDialog(
         extension_dialog_t *p_dialog )
 {
     ExtensionDialog *dialog = new ExtensionDialog( p_intf,
-                                                   p_extensions_manager,
                                                    p_dialog );
-    p_dialog->p_sys_intf = (void*) dialog;
-    connect( dialog, &ExtensionDialog::destroyDialog,
-             this, &ExtensionsDialogProvider::DestroyExtDialog );
+    m_dialogs.insert(dialog);
     return dialog;
 }
 
@@ -84,8 +80,8 @@ int ExtensionsDialogProvider::DestroyExtDialog( extension_dialog_t *p_dialog )
     ExtensionDialog *dialog = ( ExtensionDialog* ) p_dialog->p_sys_intf;
     if( !dialog )
         return VLC_EGENERIC;
+    m_dialogs.erase(dialog);
     delete dialog;
-    p_dialog->p_sys_intf = NULL;
     vlc_cond_signal( &p_dialog->cond );
     return VLC_SUCCESS;
 }
@@ -93,8 +89,7 @@ int ExtensionsDialogProvider::DestroyExtDialog( extension_dialog_t *p_dialog )
 /**
  * Update/Create/Destroy a dialog
  **/
-ExtensionDialog* ExtensionsDialogProvider::UpdateExtDialog(
-        extension_dialog_t *p_dialog )
+void ExtensionsDialogProvider::UpdateExtDialog(extension_dialog_t *p_dialog )
 {
     assert( p_dialog );
 
@@ -103,49 +98,27 @@ ExtensionDialog* ExtensionsDialogProvider::UpdateExtDialog(
     {
         /* This extension could not be activated properly but tried
            to create a dialog. We must ignore it. */
-        return NULL;
+        return;
     }
 
     vlc_mutex_lock( &p_dialog->lock );
-    if( !p_dialog->b_kill && !dialog )
-    {
-        dialog = CreateExtDialog( p_dialog );
-        dialog->setVisible( !p_dialog->b_hide );
-        dialog->has_lock = false;
-    }
-    else if( !p_dialog->b_kill && dialog )
+    if( !p_dialog->b_kill )
     {
+        if (!dialog)
+        {
+            dialog = CreateExtDialog( p_dialog );
+        }
         dialog->has_lock = true;
         dialog->UpdateWidgets();
-        if( strcmp( qtu( dialog->windowTitle() ),
-                    p_dialog->psz_title ) != 0 )
-            dialog->setWindowTitle( qfu( p_dialog->psz_title ) );
         dialog->has_lock = false;
         dialog->setVisible( !p_dialog->b_hide );
     }
-    else if( p_dialog->b_kill )
+    else
     {
         DestroyExtDialog( p_dialog );
     }
     vlc_cond_signal( &p_dialog->cond );
     vlc_mutex_unlock( &p_dialog->lock );
-    return dialog;
-}
-
-/**
- * Ask the dialog manager to create/update/kill the dialog. Thread-safe.
- **/
-void ExtensionsDialogProvider::ManageDialog( extension_dialog_t *p_dialog )
-{
-    assert( p_dialog );
-    ExtensionsManager *extMgr = ExtensionsManager::getInstance( p_intf );
-    assert( extMgr != NULL );
-    if( !extMgr->isUnloading() )
-        QMetaObject::invokeMethod(this, [this, p_dialog](){
-            UpdateExtDialog( p_dialog );
-        }); // Safe because we signal Qt thread
-    else
-        UpdateExtDialog( p_dialog ); // This is safe, we're already in Qt thread
 }
 
 /**
@@ -156,21 +129,29 @@ static void DialogCallback( extension_dialog_t *p_ext_dialog,
 {
     (void) p_data;
 
-    ExtensionsDialogProvider *p_edp = ExtensionsDialogProvider::getInstance();
-    if( p_edp )
-        p_edp->ManageDialog( p_ext_dialog );
+    auto p_edp = static_cast<ExtensionsDialogProvider*>(p_data);
+    if (!p_edp)
+        return;
+
+    //use auto connection here
+    // * either we are in extension thread and this is safe to carry the extension_dialog_t
+    //   through queued connection as the extension wait that we signal its condition variable
+    // * either we are in Qt thread (during destruction for instance) and we want direct connection
+    QMetaObject::invokeMethod(p_edp, [p_edp, p_ext_dialog]() {
+        p_edp->UpdateExtDialog( p_ext_dialog );
+    });
+
 }
 
 
 ExtensionDialog::ExtensionDialog( qt_intf_t *_p_intf,
-                                  extensions_manager_t *p_mgr,
                                   extension_dialog_t *_p_dialog )
-         : QDialog( NULL ), p_intf( _p_intf ), p_extensions_manager( p_mgr )
-         , p_dialog( _p_dialog ), has_lock(true)
+    : QDialog( NULL )
+    , p_intf( _p_intf )
+    , p_dialog( _p_dialog )
+    , has_lock(true)
 {
     assert( p_dialog );
-    connect( ExtensionsDialogProvider::getInstance(), &ExtensionsDialogProvider::destroyed,
-             this, &ExtensionDialog::parentDestroyed );
 
     msg_Dbg( p_intf, "Creating a new dialog: '%s'", p_dialog->psz_title );
     this->setWindowFlags( Qt::WindowMinMaxButtonsHint
@@ -178,54 +159,47 @@ ExtensionDialog::ExtensionDialog( qt_intf_t *_p_intf,
     this->setWindowTitle( qfu( p_dialog->psz_title ) );
 
     layout = new QGridLayout( this );
-    clickMapper = new QSignalMapper( this );
-    connect( clickMapper, QSIGNALMAPPER_MAPPEDOBJ_SIGNAL, this, &ExtensionDialog::TriggerClick );
-    inputMapper = new QSignalMapper( this );
-    connect( inputMapper, QSIGNALMAPPER_MAPPEDOBJ_SIGNAL, this, &ExtensionDialog::SyncInput );
-    selectMapper = new QSignalMapper( this );
-    connect( selectMapper, QSIGNALMAPPER_MAPPEDOBJ_SIGNAL, this, &ExtensionDialog::SyncSelection );
-
-    UpdateWidgets();
+    p_dialog->p_sys_intf = this;
 }
 
 ExtensionDialog::~ExtensionDialog()
 {
     msg_Dbg( p_intf, "Deleting extension dialog '%s'", qtu(windowTitle()) );
+    p_dialog->p_sys_intf = nullptr;
+    vlc_cond_signal( &p_dialog->cond );
+
+    //we need to manually disconnect the objects as we are listenning to their destroyed
+    //signals or they will be emitted after m_widgetMapping is destroyed and cause use after free
+    for (QObject* obj: m_widgetMapping.keys())
+        disconnect(obj, nullptr, this, nullptr);
 }
 
 QWidget* ExtensionDialog::CreateWidget( extension_widget_t *p_widget )
 {
-    QLabel *label = NULL;
-    QPushButton *button = NULL;
-    QTextBrowser *textArea = NULL;
-    QLineEdit *textInput = NULL;
-    QCheckBox *checkBox = NULL;
-    QComboBox *comboBox = NULL;
-    QListWidget *list = NULL;
-    SpinningIcon *spinIcon = NULL;
-    struct extension_widget_t::extension_widget_value_t *p_value = NULL;
-
     assert( p_widget->p_sys_intf == NULL );
 
     switch( p_widget->type )
     {
         case EXTENSION_WIDGET_LABEL:
-            label = new QLabel( qfu( p_widget->psz_text ), this );
+        {
+            auto label = new QLabel( qfu( p_widget->psz_text ), this );
             p_widget->p_sys_intf = label;
             label->setTextFormat( Qt::RichText );
             label->setOpenExternalLinks( true );
             return label;
-
+        }
         case EXTENSION_WIDGET_BUTTON:
-            button = new QPushButton( qfu( p_widget->psz_text ), this );
-            clickMapper->setMapping( button, new WidgetMapper( button, p_widget ) );
+        {
+            auto button = new QPushButton( qfu( p_widget->psz_text ), this );
+            setWidgetMapping(button, p_widget);
             connect( button, &QPushButton::clicked,
-                     clickMapper, QOverload<>::of(&QSignalMapper::map) );
+                    this, &ExtensionDialog::TriggerClick );
             p_widget->p_sys_intf = button;
             return button;
-
+        }
         case EXTENSION_WIDGET_IMAGE:
-            label = new QLabel( this );
+        {
+            auto label = new QLabel( this );
             label->setPixmap( QPixmap( qfu( p_widget->psz_text ) ) );
             if( p_widget->i_width > 0 )
                 label->setMaximumWidth( p_widget->i_width );
@@ -234,52 +208,56 @@ QWidget* ExtensionDialog::CreateWidget( extension_widget_t *p_widget )
             label->setScaledContents( true );
             p_widget->p_sys_intf = label;
             return label;
-
+        }
         case EXTENSION_WIDGET_HTML:
-            textArea = new QTextBrowser( this );
+        {
+            auto textArea = new QTextBrowser( this );
             textArea->setOpenExternalLinks( true );
             textArea->setHtml( qfu( p_widget->psz_text ) );
             p_widget->p_sys_intf = textArea;
             return textArea;
-
+        }
         case EXTENSION_WIDGET_TEXT_FIELD:
-            textInput = new QLineEdit( this );
+        {
+            auto textInput = new QLineEdit( this );
             textInput->setText( qfu( p_widget->psz_text ) );
             textInput->setReadOnly( false );
             textInput->setEchoMode( QLineEdit::Normal );
-            inputMapper->setMapping( textInput, new WidgetMapper( textInput, p_widget ) );
-            /// @note: maybe it would be wiser to use textEdited here?
+            setWidgetMapping(textInput, p_widget);
             connect( textInput, &QLineEdit::textChanged,
-                     inputMapper, QOverload<>::of(&QSignalMapper::map) );
+                     this, &ExtensionDialog::SyncInput );
             p_widget->p_sys_intf = textInput;
             return textInput;
-
+        }
         case EXTENSION_WIDGET_PASSWORD:
-            textInput = new QLineEdit( this );
+        {
+            auto textInput = new QLineEdit( this );
             textInput->setText( qfu( p_widget->psz_text ) );
             textInput->setReadOnly( false );
             textInput->setEchoMode( QLineEdit::Password );
-            inputMapper->setMapping( textInput, new WidgetMapper( textInput, p_widget ) );
+            setWidgetMapping(textInput, p_widget);
             /// @note: maybe it would be wiser to use textEdited here?
             connect( textInput, &QLineEdit::textChanged,
-                     inputMapper, QOverload<>::of(&QSignalMapper::map) );
+                    this, &ExtensionDialog::SyncInput );
             p_widget->p_sys_intf = textInput;
             return textInput;
-
+        }
         case EXTENSION_WIDGET_CHECK_BOX:
-            checkBox = new QCheckBox( this );
+        {
+            auto checkBox = new QCheckBox( this );
             checkBox->setText( qfu( p_widget->psz_text ) );
             checkBox->setChecked( p_widget->b_checked );
-            clickMapper->setMapping( checkBox, new WidgetMapper( checkBox, p_widget ) );
+            setWidgetMapping(checkBox, p_widget);
             connect( checkBox, &QCheckBox::stateChanged,
-                     clickMapper, QOverload<>::of(&QSignalMapper::map) );
+                     this, &ExtensionDialog::TriggerClick );
             p_widget->p_sys_intf = checkBox;
             return checkBox;
-
+        }
         case EXTENSION_WIDGET_DROPDOWN:
-            comboBox = new QComboBox( this );
+        {
+            auto comboBox = new QComboBox( this );
             comboBox->setEditable( false );
-            for( p_value = p_widget->p_values;
+            for( auto p_value = p_widget->p_values;
                  p_value != NULL;
                  p_value = p_value->p_next )
             {
@@ -292,15 +270,16 @@ QWidget* ExtensionDialog::CreateWidget( extension_widget_t *p_widget )
                 if( idx >= 0 )
                     comboBox->setCurrentIndex( idx );
             }
-            selectMapper->setMapping( comboBox, new WidgetMapper( comboBox, p_widget ) );
+            setWidgetMapping(comboBox, p_widget);
             connect( comboBox, QOverload<int>::of(&QComboBox::currentIndexChanged),
-                     selectMapper, QOverload<>::of(&QSignalMapper::map) );
+                     this, &ExtensionDialog::SyncSelection );
             return comboBox;
-
+        }
         case EXTENSION_WIDGET_LIST:
-            list = new QListWidget( this );
+        {
+            auto list = new QListWidget( this );
             list->setSelectionMode( QAbstractItemView::ExtendedSelection );
-            for( p_value = p_widget->p_values;
+            for( auto p_value = p_widget->p_values;
                  p_value != NULL;
                  p_value = p_value->p_next )
             {
@@ -309,17 +288,18 @@ QWidget* ExtensionDialog::CreateWidget( extension_widget_t *p_widget )
                 item->setData( Qt::UserRole, p_value->i_id );
                 list->addItem( item );
             }
-            selectMapper->setMapping( list, new WidgetMapper( list, p_widget ) );
+            setWidgetMapping(list, p_widget);
             connect( list, &QListWidget::itemSelectionChanged,
-                     selectMapper, QOverload<>::of(&QSignalMapper::map) );
+                     this, &ExtensionDialog::SyncSelection );
             return list;
-
+        }
         case EXTENSION_WIDGET_SPIN_ICON:
-            spinIcon = new SpinningIcon( this );
+        {
+            auto spinIcon = new SpinningIcon( this );
             spinIcon->play( p_widget->i_spin_loops );
             p_widget->p_sys_intf = spinIcon;
             return spinIcon;
-
+        }
         default:
             msg_Err( p_intf, "Widget type %d unknown", p_widget->type );
             return NULL;
@@ -330,13 +310,11 @@ QWidget* ExtensionDialog::CreateWidget( extension_widget_t *p_widget )
  * Forward click event to the extension
  * @param object A WidgetMapper, whose data() is the p_widget
  **/
-int ExtensionDialog::TriggerClick( QObject *object )
+int ExtensionDialog::TriggerClick()
 {
-    assert( object != NULL );
-    WidgetMapper *mapping = static_cast< WidgetMapper* >( object );
-    extension_widget_t *p_widget = mapping->getWidget();
+    extension_widget_t *p_widget = getWidgetMapping(sender());
+    assert(p_widget);
 
-    QCheckBox *checkBox = NULL;
     int i_ret = VLC_EGENERIC;
 
     bool lockedHere = false;
@@ -354,11 +332,12 @@ int ExtensionDialog::TriggerClick( QObject *object )
             break;
 
         case EXTENSION_WIDGET_CHECK_BOX:
-            checkBox = static_cast< QCheckBox* >( p_widget->p_sys_intf );
+        {
+            auto checkBox = static_cast< QCheckBox* >( p_widget->p_sys_intf );
             p_widget->b_checked = checkBox->isChecked();
             i_ret = VLC_SUCCESS;
             break;
-
+        }
         default:
             msg_Dbg( p_intf, "A click event was triggered by a wrong widget" );
             break;
@@ -377,9 +356,10 @@ int ExtensionDialog::TriggerClick( QObject *object )
  * Synchronize psz_text with the widget's text() value on update
  * @param object A WidgetMapper
  **/
-void ExtensionDialog::SyncInput( QObject *object )
+void ExtensionDialog::SyncInput()
 {
-    assert( object != NULL );
+    extension_widget_t *p_widget = getWidgetMapping(sender());
+    assert(p_widget);
 
     bool lockedHere = false;
     if( !has_lock )
@@ -389,8 +369,6 @@ void ExtensionDialog::SyncInput( QObject *object )
         lockedHere = true;
     }
 
-    WidgetMapper *mapping = static_cast< WidgetMapper* >( object );
-    extension_widget_t *p_widget = mapping->getWidget();
     assert( p_widget->type == EXTENSION_WIDGET_TEXT_FIELD
             || p_widget->type == EXTENSION_WIDGET_PASSWORD );
     /* Synchronize psz_text with the new value */
@@ -410,11 +388,13 @@ void ExtensionDialog::SyncInput( QObject *object )
  * Synchronize parameter b_selected in the values list
  * @param object A WidgetMapper
  **/
-void ExtensionDialog::SyncSelection( QObject *object )
+void ExtensionDialog::SyncSelection()
 {
-    assert( object != NULL );
     struct extension_widget_t::extension_widget_value_t *p_value;
 
+    extension_widget_t *p_widget = getWidgetMapping(sender());
+    assert(p_widget);
+
     bool lockedHere = false;
     if( !has_lock )
     {
@@ -423,8 +403,6 @@ void ExtensionDialog::SyncSelection( QObject *object )
         lockedHere = true;
     }
 
-    WidgetMapper *mapping = static_cast< WidgetMapper* >( object );
-    extension_widget_t *p_widget = mapping->getWidget();
     assert( p_widget->type == EXTENSION_WIDGET_DROPDOWN
             || p_widget->type == EXTENSION_WIDGET_LIST );
 
@@ -481,6 +459,11 @@ void ExtensionDialog::SyncSelection( QObject *object )
 void ExtensionDialog::UpdateWidgets()
 {
     assert( p_dialog );
+    vlc_mutex_assert(&p_dialog->lock);
+
+    if( strcmp( qtu( windowTitle() ), p_dialog->psz_title ) != 0 )
+        setWindowTitle( qfu( p_dialog->psz_title ) );
+
     extension_widget_t *p_widget;
     ARRAY_FOREACH( p_widget, p_dialog->widgets )
     {
@@ -546,63 +529,60 @@ void ExtensionDialog::UpdateWidgets()
 
 QWidget* ExtensionDialog::UpdateWidget( extension_widget_t *p_widget )
 {
-    QLabel *label = NULL;
-    QPushButton *button = NULL;
-    QTextBrowser *textArea = NULL;
-    QLineEdit *textInput = NULL;
-    QCheckBox *checkBox = NULL;
-    QComboBox *comboBox = NULL;
-    QListWidget *list = NULL;
-    SpinningIcon *spinIcon = NULL;
-    struct extension_widget_t::extension_widget_value_t *p_value = NULL;
-
     assert( p_widget->p_sys_intf != NULL );
 
     switch( p_widget->type )
     {
         case EXTENSION_WIDGET_LABEL:
-            label = static_cast< QLabel* >( p_widget->p_sys_intf );
+        {
+            auto label = static_cast< QLabel* >( p_widget->p_sys_intf );
             label->setText( qfu( p_widget->psz_text ) );
             return label;
-
+        }
         case EXTENSION_WIDGET_BUTTON:
+        {
             // FIXME: looks like removeMappings does not work
-            button = static_cast< QPushButton* >( p_widget->p_sys_intf );
+            auto button = static_cast< QPushButton* >( p_widget->p_sys_intf );
             button->setText( qfu( p_widget->psz_text ) );
-            clickMapper->removeMappings( button );
-            clickMapper->setMapping( button, new WidgetMapper( button, p_widget ) );
+            setWidgetMapping(button, p_widget);
             connect( button, &QPushButton::clicked,
-                     clickMapper, QOverload<>::of(&QSignalMapper::map) );
+                     this, &ExtensionDialog::TriggerClick );
             return button;
-
+        }
         case EXTENSION_WIDGET_IMAGE:
-            label = static_cast< QLabel* >( p_widget->p_sys_intf );
+        {
+            auto label = static_cast< QLabel* >( p_widget->p_sys_intf );
             label->setPixmap( QPixmap( qfu( p_widget->psz_text ) ) );
             return label;
-
+        }
         case EXTENSION_WIDGET_HTML:
-            textArea = static_cast< QTextBrowser* >( p_widget->p_sys_intf );
+        {
+            auto textArea = static_cast< QTextBrowser* >( p_widget->p_sys_intf );
             textArea->setHtml( qfu( p_widget->psz_text ) );
             return textArea;
-
+        }
         case EXTENSION_WIDGET_TEXT_FIELD:
-            textInput = static_cast< QLineEdit* >( p_widget->p_sys_intf );
+        {
+            auto textInput = static_cast< QLineEdit* >( p_widget->p_sys_intf );
             textInput->setText( qfu( p_widget->psz_text ) );
             return textInput;
-
+        }
         case EXTENSION_WIDGET_PASSWORD:
-            textInput = static_cast< QLineEdit* >( p_widget->p_sys_intf );
+        {
+            auto textInput = static_cast< QLineEdit* >( p_widget->p_sys_intf );
             textInput->setText( qfu( p_widget->psz_text ) );
             return textInput;
-
+        }
         case EXTENSION_WIDGET_CHECK_BOX:
-            checkBox = static_cast< QCheckBox* >( p_widget->p_sys_intf );
+        {
+            auto checkBox = static_cast< QCheckBox* >( p_widget->p_sys_intf );
             checkBox->setText( qfu( p_widget->psz_text ) );
             checkBox->setChecked( p_widget->b_checked );
             return checkBox;
-
+        }
         case EXTENSION_WIDGET_DROPDOWN:
-            comboBox = static_cast< QComboBox* >( p_widget->p_sys_intf );
+        {
+            auto comboBox = static_cast< QComboBox* >( p_widget->p_sys_intf );
             // method widget:clear()
             if ( p_widget->p_values == NULL )
             {
@@ -610,7 +590,7 @@ QWidget* ExtensionDialog::UpdateWidget( extension_widget_t *p_widget )
                 return comboBox;
             }
             // method widget:addvalue()
-            for( p_value = p_widget->p_values;
+            for( auto p_value = p_widget->p_values;
                  p_value != NULL;
                  p_value = p_value->p_next )
             {
@@ -618,11 +598,12 @@ QWidget* ExtensionDialog::UpdateWidget( extension_widget_t *p_widget )
                     comboBox->addItem( qfu( p_value->psz_text ), p_value->i_id );
             }
             return comboBox;
-
+        }
         case EXTENSION_WIDGET_LIST:
-            list = static_cast< QListWidget* >( p_widget->p_sys_intf );
+        {
+            auto list = static_cast< QListWidget* >( p_widget->p_sys_intf );
             list->clear();
-            for( p_value = p_widget->p_values;
+            for( auto p_value = p_widget->p_values;
                  p_value != NULL;
                  p_value = p_value->p_next )
             {
@@ -632,16 +613,17 @@ QWidget* ExtensionDialog::UpdateWidget( extension_widget_t *p_widget )
                 list->addItem( item );
             }
             return list;
-
+        }
         case EXTENSION_WIDGET_SPIN_ICON:
-            spinIcon = static_cast< SpinningIcon* >( p_widget->p_sys_intf );
+        {
+            auto spinIcon = static_cast< SpinningIcon* >( p_widget->p_sys_intf );
             if( !spinIcon->isPlaying() && p_widget->i_spin_loops != 0 )
                 spinIcon->play( p_widget->i_spin_loops );
             else if( spinIcon->isPlaying() && p_widget->i_spin_loops == 0 )
                 spinIcon->stop();
             p_widget->i_height = p_widget->i_width = 16;
             return spinIcon;
-
+        }
         default:
             msg_Err( p_intf, "Widget type %d unknown", p_widget->type );
             return NULL;
@@ -659,6 +641,24 @@ void ExtensionDialog::DestroyWidget( extension_widget_t *p_widget,
         vlc_cond_signal( &p_dialog->cond );
 }
 
+void ExtensionDialog::setWidgetMapping(QObject* object, extension_widget_t *ext_widget)
+{
+    if (!ext_widget) {
+        m_widgetMapping.remove(object);
+        return;
+    }
+
+    m_widgetMapping.insert(object, ext_widget);
+    connect(object, &QWidget::destroyed, this, [this](QObject* obj){
+        m_widgetMapping.remove(obj);
+    });
+}
+
+extension_widget_t* ExtensionDialog::getWidgetMapping(QObject* obj) const
+{
+    return m_widgetMapping.value(obj, nullptr);
+}
+
 /** Implement closeEvent() in order to intercept the event */
 void ExtensionDialog::closeEvent( QCloseEvent * )
 {
@@ -682,11 +682,3 @@ void ExtensionDialog::keyPressEvent( QKeyEvent *event )
         return;
     }
 }
-
-void ExtensionDialog::parentDestroyed()
-{
-    msg_Dbg( p_intf, "About to destroy dialog '%s'", p_dialog->psz_title );
-    deleteLater(); // May not work at this point (event loop can be ended)
-    p_dialog->p_sys_intf = NULL;
-    vlc_cond_signal( &p_dialog->cond );
-}


=====================================
modules/gui/qt/dialogs/extensions/extensions.hpp
=====================================
@@ -24,9 +24,10 @@
 #define EXTENSIONS_HPP
 
 #include "qt.hpp"
-#include "util/singleton.hpp"
 
 #include <cassert>
+#include <unordered_set>
+#include <QHash>
 
 #include <QDialog>
 class QObject;
@@ -45,31 +46,25 @@ extern "C" {
     typedef struct extension_t extension_t;
 };
 
-class ExtensionsDialogProvider : public QObject, public Singleton<ExtensionsDialogProvider>
+class ExtensionsDialogProvider : public QObject
 {
     /** This is the dialog provider for Extensions dialogs
      * @todo Add a setExtManager() function (with vlc_object_hold)
      **/
-    friend class Singleton<ExtensionsDialogProvider>;
-
     Q_OBJECT
 
-private:
-    qt_intf_t *p_intf;
-    extensions_manager_t *p_extensions_manager;
+public:
+    ExtensionsDialogProvider( qt_intf_t *p_intf = nullptr);
+    virtual ~ExtensionsDialogProvider();
 
-private slots:
+    void UpdateExtDialog( extension_dialog_t *p_dialog );
+
+private:
     ExtensionDialog* CreateExtDialog( extension_dialog_t *p_dialog );
     int DestroyExtDialog( extension_dialog_t *p_dialog );
-    ExtensionDialog* UpdateExtDialog( extension_dialog_t *p_dialog );
-
-public:
-    void ManageDialog( extension_dialog_t *p_dialog );
 
-private:
-    ExtensionsDialogProvider( qt_intf_t *p_intf = nullptr,
-                             extensions_manager_t *p_mgr = nullptr );
-    virtual ~ExtensionsDialogProvider();
+    std::unordered_set<ExtensionDialog*> m_dialogs;
+    qt_intf_t *p_intf = nullptr;
 };
 
 
@@ -78,35 +73,30 @@ class ExtensionDialog : public QDialog
     Q_OBJECT
 private:
     qt_intf_t *p_intf;
-    extensions_manager_t *p_extensions_manager;
     extension_t *p_extension;
     extension_dialog_t *p_dialog;
     bool has_lock; ///< Indicates whether Qt thread owns the lock
     QGridLayout *layout;
-    QSignalMapper *clickMapper;
-    QSignalMapper *inputMapper;
-    QSignalMapper *selectMapper;
+    QHash<QObject*, extension_widget_t*> m_widgetMapping;
 
     QWidget *CreateWidget( extension_widget_t *p_widget );
     QWidget *UpdateWidget( extension_widget_t *p_widget );
     void DestroyWidget( extension_widget_t *p_widget, bool b_cond = true );
 
+    void setWidgetMapping(QObject* widget, extension_widget_t *ext_widget);
+    extension_widget_t* getWidgetMapping(QObject* widget) const;
+
 protected:
     void closeEvent( QCloseEvent* ) override;
     void keyPressEvent( QKeyEvent* ) override;
 
 private slots:
-    int TriggerClick( QObject *object );
-    void SyncInput( QObject *object );
-    void SyncSelection( QObject *object );
-    void parentDestroyed();
-
-signals:
-    void destroyDialog( extension_dialog_t *p_dialog );
+    int TriggerClick();
+    void SyncInput();
+    void SyncSelection();
 
 public:
     ExtensionDialog( qt_intf_t *p_intf,
-                     extensions_manager_t *p_mgr,
                      extension_dialog_t *p_dialog );
     virtual ~ExtensionDialog();
 
@@ -116,15 +106,4 @@ public:
     friend class ExtensionsDialogProvider;
 };
 
-class WidgetMapper : public QObject
-{
-    Q_OBJECT
-private:
-    extension_widget_t *p_widget;
-public:
-    WidgetMapper( QObject* parent, extension_widget_t *_p_widget ) :
-            QObject(parent), p_widget(_p_widget) {}
-    extension_widget_t* getWidget() { return p_widget; }
-};
-
 #endif // EXTENSIONS_HPP


=====================================
modules/gui/qt/dialogs/extensions/extensions_manager.cpp
=====================================
@@ -41,10 +41,9 @@
 
 ExtensionsManager::ExtensionsManager( qt_intf_t *_p_intf, QObject *parent )
         : QObject( parent ), p_intf( _p_intf ), p_extensions_manager( NULL )
-        , p_edp( NULL )
 {
     menuMapper = new QSignalMapper( this );
-    connect( menuMapper, QSIGNALMAPPER_MAPPEDINT_SIGNAL, this, &ExtensionsManager::triggerMenu );
+    connect( menuMapper, &QSignalMapper::mappedInt, this, &ExtensionsManager::triggerMenu );
     connect( THEMIM, &PlayerController::playingStateChanged, this, &ExtensionsManager::playingChanged );
     connect( THEMIM, &PlayerController::inputChanged, this, &ExtensionsManager::inputChanged, Qt::DirectConnection );
     connect( THEMIM, &PlayerController::metaChanged, this, &ExtensionsManager::metaChanged );
@@ -55,12 +54,7 @@ ExtensionsManager::ExtensionsManager( qt_intf_t *_p_intf, QObject *parent )
 ExtensionsManager::~ExtensionsManager()
 {
     msg_Dbg( p_intf, "Killing extension dialog provider" );
-    ExtensionsDialogProvider::killInstance();
-    if( p_extensions_manager )
-    {
-        module_unneed( p_extensions_manager, p_extensions_manager->p_module );
-        vlc_object_delete(p_extensions_manager);
-    }
+    unloadExtensions();
 }
 
 bool ExtensionsManager::loadExtensions()
@@ -91,19 +85,8 @@ bool ExtensionsManager::loadExtensions()
         }
 
         /* Initialize dialog provider */
-        p_edp = ExtensionsDialogProvider::getInstance( p_intf,
-                                                       p_extensions_manager );
-        if( !p_edp )
-        {
-            msg_Err( p_intf, "Unable to create dialogs provider for extensions" );
-            module_unneed( p_extensions_manager,
-                           p_extensions_manager->p_module );
-            vlc_object_delete(p_extensions_manager);
-            p_extensions_manager = NULL;
-            b_failed = true;
-            emit extensionsUpdated();
-            return false;
-        }
+        assert(!p_edp);
+        p_edp = std::make_unique<ExtensionsDialogProvider>(p_intf);
         b_unloading = false;
     }
     b_failed = false;
@@ -116,7 +99,12 @@ void ExtensionsManager::unloadExtensions()
     if( !p_extensions_manager )
         return;
     b_unloading = true;
-    ExtensionsDialogProvider::killInstance();
+    extension_t* p_ext = nullptr;
+    ARRAY_FOREACH( p_ext, p_extensions_manager->extensions )
+    {
+        extension_Deactivate(p_extensions_manager, p_ext);
+    }
+    p_edp.reset();
     module_unneed( p_extensions_manager, p_extensions_manager->p_module );
     vlc_object_delete(p_extensions_manager);
     p_extensions_manager = NULL;


=====================================
modules/gui/qt/dialogs/extensions/extensions_manager.hpp
=====================================
@@ -80,7 +80,7 @@ private:
     static ExtensionsManager* instance;
     qt_intf_t *p_intf;
     extensions_manager_t *p_extensions_manager;
-    ExtensionsDialogProvider *p_edp;
+    std::unique_ptr<ExtensionsDialogProvider> p_edp;
 
     QSignalMapper *menuMapper;
     bool b_unloading;  ///< Work around threads + emit issues, see isUnloading


=====================================
modules/gui/qt/dialogs/plugins/plugins.cpp
=====================================
@@ -464,7 +464,7 @@ AddonsTab::AddonsTab( qt_intf_t *p_intf_ ) : QVLCFrame( p_intf_ )
     addonsModel->setFilterRole( Qt::DisplayRole );
     addonsView->setModel( addonsModel );
 
-    connect( signalMapper, QSIGNALMAPPER_MAPPEDINT_SIGNAL,
+    connect( signalMapper, &QSignalMapper::mappedInt,
              addonsModel, &AddonsSortFilterProxyModel::setTypeFilter );
 
     connect( searchInput, &SearchLineEdit::textChanged,


=====================================
modules/gui/qt/dialogs/preferences/simple_preferences.cpp
=====================================
@@ -228,7 +228,7 @@ SPrefsCatList::SPrefsCatList( qt_intf_t *_p_intf, QWidget *_parent ) :
        set focus (keys) when it manages the buttons's exclusivity.
        See QT bugs 131 & 816 and QAbstractButton's source code. */
     QSignalMapper *mapper = new QSignalMapper( layout );
-    connect( mapper, QSIGNALMAPPER_MAPPEDINT_SIGNAL, this, &SPrefsCatList::switchPanel );
+    connect( mapper, &QSignalMapper::mappedInt, this, &SPrefsCatList::switchPanel );
     qreal dpr = devicePixelRatioF();
 
     auto addCategory = [&]( QString label, QString ltooltip, QString icon, int numb) {


=====================================
modules/gui/qt/menus/qml_menu_wrapper.cpp
=====================================
@@ -797,7 +797,7 @@ void PlaylistMediaContextMenu::popup(const QModelIndexList & selected, QPoint po
         connect(action, &QAction::triggered, mapper, QOverload<>::of(&QSignalMapper::map));
 
         mapper->setMapping(action, options["information"].toInt());
-        connect(mapper, QSIGNALMAPPER_MAPPEDINT_SIGNAL,
+        connect(mapper, &QSignalMapper::mappedInt,
                 this, &PlaylistMediaContextMenu::showMediaInformation);
     }
 


=====================================
modules/gui/qt/player/player_controller.cpp
=====================================
@@ -1165,7 +1165,7 @@ PlayerController::PlayerController( qt_intf_t *_p_intf )
 {
     /* Audio Menu */
     menusAudioMapper = new QSignalMapper(this);
-    connect( menusAudioMapper, QSIGNALMAPPER_MAPPEDSTR_SIGNAL,
+    connect( menusAudioMapper, &QSignalMapper::mappedString,
              this, &PlayerController::menusUpdateAudio );
     connect( &d_ptr->m_position_timer, &QTimer::timeout, this, &PlayerController::updatePositionFromTimer );
     connect( &d_ptr->m_time_timer, &QTimer::timeout, this, &PlayerController::updateTimeFromTimer );


=====================================
modules/gui/qt/qt.hpp
=====================================
@@ -42,11 +42,6 @@
 static_assert (QT_VERSION >= QT_VERSION_CHECK(6, 2, 0),
                "Update your Qt version to at least 6.2.0");
 
-# define QSIGNALMAPPER_MAPPEDINT_SIGNAL &QSignalMapper::mappedInt
-# define QSIGNALMAPPER_MAPPEDSTR_SIGNAL &QSignalMapper::mappedString
-# define QSIGNALMAPPER_MAPPEDOBJ_SIGNAL &QSignalMapper::mappedObject
-
-
 enum {
     IMEventTypeOffset     = 0,
     MsgEventTypeOffset    = 100



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/960e5a50b924112c19fe6ab62c411417cade556b...469a286ed898a8fef278f822f41333d519c3826c

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/960e5a50b924112c19fe6ab62c411417cade556b...469a286ed898a8fef278f822f41333d519c3826c
You're receiving this email because of your account on code.videolan.org.


VideoLAN code repository instance


More information about the vlc-commits mailing list