[vlc-devel] [PATCH 1/3] qt: use shared pointers in menu items
Hugo Beauzée-Luyssen
hugo at beauzee.fr
Mon Mar 18 14:20:14 CET 2019
Hi,
I like the idea but why using QSharedPointer instead of vlc_shared_data_ptr or wrap_cptr?
I also agree with Thomas that this is not the best way to transition to the new UI seamlessly.
On Sun, Mar 17, 2019, at 10:56 AM, Rémi Denis-Courmont wrote:
> Use QSharedPointer shared pointers to reference VLC objects in
> automatically generated menus. This enables using a custom callback
> for releasing the reference to object of different types.
> ---
> modules/gui/qt/menus.cpp | 100 ++++++++++++++++++++++++++-------------
> modules/gui/qt/menus.hpp | 25 +++++-----
> 2 files changed, 80 insertions(+), 45 deletions(-)
>
> diff --git a/modules/gui/qt/menus.cpp b/modules/gui/qt/menus.cpp
> index b102fa6902..1e7b34b386 100644
> --- a/modules/gui/qt/menus.cpp
> +++ b/modules/gui/qt/menus.cpp
> @@ -80,6 +80,35 @@ static QActionGroup *currentGroup;
> QMenu *VLCMenuBar::recentsMenu = NULL;
> RendererMenu *VLCMenuBar::rendererMenu = NULL;
>
> +static void releaseDummy(vlc_object_t *)
> +{
> +}
> +
> +static QVLCObject wrap(playlist_t *pl)
> +{
> + return QVLCObject(VLC_OBJECT(pl), releaseDummy);
> +}
> +
> +static QVLCObject wrap(intf_thread_t *intf)
> +{
> + return QVLCObject(VLC_OBJECT(intf), releaseDummy);
> +}
> +
> +static QVLCObject wrap(input_thread_t *input)
> +{
> + return QVLCObject(VLC_OBJECT(input), releaseDummy);
> +}
> +
> +static QVLCObject wrap(audio_output_t *aout)
> +{
> + return QVLCObject(VLC_OBJECT(aout), releaseDummy);
> +}
> +
> +static QVLCObject wrap(vout_thread_t *vout)
> +{
> + return QVLCObject(VLC_OBJECT(vout), releaseDummy);
> +}
> +
> /**
> * @brief Add static entries to DP in menus
> **/
> @@ -209,7 +238,7 @@ static QAction * FindActionWithVar( QMenu *menu,
> const char *psz_var )
> #define PUSH_OBJVAR(object,var) \
> do { \
> varnames.append(var); \
> - objects.append(VLC_OBJECT(object)); \
> + objects.append(wrap(object)); \
> } while (0)
>
> #define PUSH_VAR(var) PUSH_OBJVAR(p_object, var)
> @@ -217,7 +246,7 @@ static QAction * FindActionWithVar( QMenu *menu,
> const char *psz_var )
> #define PUSH_PLVAR(var) PUSH_OBJVAR(pl, var)
>
> static int InputAutoMenuBuilder( input_thread_t *p_input,
> - QVector<vlc_object_t *> &objects, QVector<const char *> &varnames )
> + QVector<QVLCObject> &objects, QVector<const char *> &varnames )
> {
> PUSH_INPUTVAR( "bookmark" );
> PUSH_INPUTVAR( "title" );
> @@ -228,7 +257,7 @@ static int InputAutoMenuBuilder( input_thread_t *p_input,
> }
>
> static int VideoAutoMenuBuilder( playlist_t *pl, input_thread_t *p_input,
> - QVector<vlc_object_t *> &objects, QVector<const char *> &varnames )
> + QVector<QVLCObject> &objects, QVector<const char *> &varnames )
> {
> vout_thread_t *p_object = p_input ? input_GetVout( p_input ) : NULL;
>
> @@ -249,7 +278,7 @@ static int VideoAutoMenuBuilder( playlist_t *pl,
> input_thread_t *p_input,
> }
>
> static int SubsAutoMenuBuilder( input_thread_t *p_input,
> - QVector<vlc_object_t *> &objects, QVector<const char *> &varnames )
> + QVector<QVLCObject> &objects, QVector<const char *> &varnames )
> {
> PUSH_INPUTVAR( "spu-es" );
>
> @@ -257,7 +286,7 @@ static int SubsAutoMenuBuilder( input_thread_t *p_input,
> }
>
> static int AudioAutoMenuBuilder( input_thread_t *p_input,
> - QVector<vlc_object_t *> &objects, QVector<const char *> &varnames )
> + QVector<QVLCObject> &objects, QVector<const char *> &varnames )
> {
> audio_output_t *p_object = p_input ? input_GetAout( p_input ) : NULL;
>
> @@ -553,10 +582,10 @@ QMenu *VLCMenuBar::ViewMenu( intf_thread_t
> *p_intf, QMenu *current, MainInterfac
> **/
> QMenu *VLCMenuBar::InterfacesMenu( intf_thread_t *p_intf, QMenu
> *current )
> {
> - QVector<vlc_object_t *> objects;
> + QVector<QVLCObject> objects;
> QVector<const char *> varnames;
> varnames.append( "intf-add" );
> - objects.append( VLC_OBJECT(p_intf) );
> + objects.append( wrap(p_intf) );
>
> return Populate( current, varnames, objects );
> }
> @@ -605,7 +634,7 @@ static inline void VolumeEntries( intf_thread_t
> *p_intf, QMenu *current )
> **/
> QMenu *VLCMenuBar::AudioMenu( intf_thread_t *p_intf, QMenu * current )
> {
> - QVector<vlc_object_t *> objects;
> + QVector<QVLCObject> objects;
> QVector<const char *> varnames;
> QMenu *audioDeviceMenu;
> audio_output_t *p_aout;
> @@ -646,7 +675,7 @@ QMenu *VLCMenuBar::AudioMenu( intf_thread_t
> *p_intf, QMenu * current )
> QMenu *VLCMenuBar::SubtitleMenu( intf_thread_t *p_intf, QMenu
> *current, bool b_popup )
> {
> input_thread_t *p_input;
> - QVector<vlc_object_t *> objects;
> + QVector<QVLCObject> objects;
> QVector<const char *> varnames;
>
> if( current->isEmpty() || b_popup )
> @@ -670,7 +699,7 @@ QMenu *VLCMenuBar::SubtitleMenu( intf_thread_t
> *p_intf, QMenu *current, bool b_p
> QMenu *VLCMenuBar::VideoMenu( intf_thread_t *p_intf, QMenu *current )
> {
> input_thread_t *p_input;
> - QVector<vlc_object_t *> objects;
> + QVector<QVLCObject> objects;
> QVector<const char *> varnames;
>
> if( current->isEmpty() )
> @@ -747,7 +776,7 @@ QMenu *VLCMenuBar::RebuildNavigMenu( intf_thread_t
> *p_intf, QMenu *menu, bool b_
> {
> /* */
> input_thread_t *p_object;
> - QVector<vlc_object_t *> objects;
> + QVector<QVLCObject> objects;
> QVector<const char *> varnames;
>
> /* Get the input and hold it */
> @@ -802,7 +831,7 @@ QMenu *VLCMenuBar::HelpMenu( QWidget *parent )
> *****************************************************************************/
> #define POPUP_BOILERPLATE \
> QMenu* menu; \
> - QVector<vlc_object_t *> objects; \
> + QVector<QVLCObject> objects; \
> QVector<const char *> varnames; \
> input_thread_t *p_input = THEMIM->getInput();
>
> @@ -1020,6 +1049,7 @@ QMenu* VLCMenuBar::PopupMenu( intf_thread_t
> *p_intf, bool show )
>
> if( p_input )
> {
> + QVLCObject input = wrap(p_input);
> QMenu *submenu;
> vout_thread_t *p_vout = THEMIM->getVout();
>
> @@ -1031,10 +1061,11 @@ QMenu* VLCMenuBar::PopupMenu( intf_thread_t
> *p_intf, bool show )
> b_isFullscreen = !( !val.b_bool );
> if( b_isFullscreen )
> {
> + QVLCObject playlist = wrap(THEPL);
> val.b_bool = false;
> CreateAndConnect( menu, "fullscreen",
> qtr( "Leave Fullscreen" ),"" , ITEM_NORMAL,
> - VLC_OBJECT(THEPL), val, VLC_VAR_BOOL,
> b_isFullscreen );
> + playlist, val, VLC_VAR_BOOL, b_isFullscreen );
> }
> vout_Release(p_vout);
>
> @@ -1062,7 +1093,7 @@ QMenu* VLCMenuBar::PopupMenu( intf_thread_t
> *p_intf, bool show )
> submenu = new QMenu( menu );
> action = menu->addMenu( SubtitleMenu( p_intf, submenu, true )
> );
> action->setText( qtr( "Subti&tle") );
> - UpdateItem( submenu, "spu-es", VLC_OBJECT(p_input), true );
> + UpdateItem( submenu, "spu-es", input, true );
>
> /* Playback menu for chapters */
> submenu = new QMenu( menu );
> @@ -1090,12 +1121,13 @@ QMenu* VLCMenuBar::PopupMenu( intf_thread_t
> *p_intf, bool show )
> /* In skins interface, append some items */
> if( p_intf->p_sys->b_isDialogProvider )
> {
> - vlc_object_t* p_object = vlc_object_parent(p_intf);
> + intf_thread_t *p_object =
> + (intf_thread_t *)vlc_object_parent(p_intf);
> submenu->setTitle( qtr( "Interface" ) );
>
> /* Open skin dialog box */
> objects.clear(); varnames.clear();
> - objects.append( p_object );
> + objects.append( wrap(p_object) );
> varnames.append( "intf-skins-interactive" );
> Populate( submenu, varnames, objects );
> QAction* action = submenu->actions().back();
> @@ -1103,7 +1135,7 @@ QMenu* VLCMenuBar::PopupMenu( intf_thread_t
> *p_intf, bool show )
>
> /* list of skins available */
> objects.clear(); varnames.clear();
> - objects.append( p_object );
> + objects.append( wrap(p_object) );
> varnames.append( "intf-skins" );
> Populate( submenu, varnames, objects );
>
> @@ -1206,7 +1238,7 @@ void VLCMenuBar::updateSystrayMenu( MainInterface
> *mi,
>
> *************************************************************************/
> QMenu * VLCMenuBar::Populate( QMenu *current,
> QVector< const char *> & varnames,
> - QVector<vlc_object_t *> & objects )
> + QVector<QVLCObject> & objects )
> {
> QMenu *menu = current;
> assert( menu );
> @@ -1244,7 +1276,7 @@ static bool IsMenuEmpty( const char *psz_var,
> vlc_object_t *p_object )
> #define TEXT_OR_VAR qfue ( text ? text : psz_var )
>
> void VLCMenuBar::UpdateItem( QMenu *menu,
> - const char *psz_var, vlc_object_t *p_object, bool b_submenu )
> + const char *psz_var, QVLCObject& p_object, bool b_submenu )
> {
> vlc_value_t val;
> char *text;
> @@ -1271,7 +1303,7 @@ void VLCMenuBar::UpdateItem( QMenu *menu,
> || !strcmp( psz_var, "bookmark" ) )
> i_type = VLC_VAR_INTEGER | VLC_VAR_HASCHOICE;
> else
> - i_type = var_Type( p_object, psz_var );
> + i_type = var_Type( &*p_object, psz_var );
You probably want to use p_object.data() instead, ditto below.
>
> switch( i_type & VLC_VAR_TYPE )
> {
> @@ -1289,7 +1321,7 @@ void VLCMenuBar::UpdateItem( QMenu *menu,
> }
>
> /* Make sure we want to display the variable */
> - if( menu->isEmpty() && IsMenuEmpty( psz_var, p_object ) )
> + if( menu->isEmpty() && IsMenuEmpty( psz_var, &*p_object ) )
> {
> if( action )
> action->setEnabled( false );
> @@ -1297,7 +1329,7 @@ void VLCMenuBar::UpdateItem( QMenu *menu,
> }
>
> /* Get the descriptive name of the variable */
> - if( var_Change( p_object, psz_var, VLC_VAR_GETTEXT, &text ) )
> + if( var_Change( &*p_object, psz_var, VLC_VAR_GETTEXT, &text ) )
> text = NULL;
>
> if( !action )
> @@ -1339,11 +1371,11 @@ void VLCMenuBar::UpdateItem( QMenu *menu,
> case VLC_VAR_VOID:
> val.i_int = 0; // Prevent the copy of an uninitialized value
> CreateAndConnect( menu, psz_var, TEXT_OR_VAR, "", ITEM_NORMAL,
> - p_object, val, i_type );
> + p_object, val, i_type );
> break;
>
> case VLC_VAR_BOOL:
> - var_Get( p_object, psz_var, &val );
> + var_Get( &*p_object, psz_var, &val );
> val.b_bool = !val.b_bool;
> CreateAndConnect( menu, psz_var, TEXT_OR_VAR, "",
> ITEM_CHECK,
> p_object, val, i_type, !val.b_bool );
> @@ -1369,7 +1401,7 @@ static bool CheckTitle( vlc_object_t *p_object,
> const char *psz_var )
>
>
> int VLCMenuBar::CreateChoicesMenu( QMenu *submenu, const char *psz_var,
> - vlc_object_t *p_object )
> + QVLCObject& p_object )
> {
> vlc_value_t val;
> vlc_value_t *val_list;
> @@ -1378,10 +1410,10 @@ int VLCMenuBar::CreateChoicesMenu( QMenu
> *submenu, const char *psz_var,
> int i_type;
>
> /* Check the type of the object variable */
> - i_type = var_Type( p_object, psz_var );
> + i_type = var_Type( &*p_object, psz_var );
>
> /* Make sure we want to display the variable */
> - if( submenu->isEmpty() && IsMenuEmpty( psz_var, p_object ) )
> + if( submenu->isEmpty() && IsMenuEmpty( psz_var, &*p_object ) )
> return VLC_EGENERIC;
>
> switch( i_type & VLC_VAR_TYPE )
> @@ -1397,7 +1429,7 @@ int VLCMenuBar::CreateChoicesMenu( QMenu
> *submenu, const char *psz_var,
> return VLC_EGENERIC;
> }
>
> - if( var_Change( p_object, psz_var, VLC_VAR_GETCHOICES,
> + if( var_Change( &*p_object, psz_var, VLC_VAR_GETCHOICES,
> &count, &val_list, &text_list ) < 0 )
> {
> return VLC_EGENERIC;
> @@ -1415,7 +1447,7 @@ int VLCMenuBar::CreateChoicesMenu( QMenu
> *submenu, const char *psz_var,
> switch( i_type & VLC_VAR_TYPE )
> {
> case VLC_VAR_STRING:
> - var_Get( p_object, psz_var, &val );
> + var_Get( &*p_object, psz_var, &val );
> another_val.psz_string = strdup( CURVAL.psz_string );
> menutext = qfue( CURTEXT ? CURTEXT :
> another_val.psz_string );
> CreateAndConnect( submenu, psz_var, menutext, "",
> RADIO_OR_COMMAND,
> @@ -1427,17 +1459,17 @@ int VLCMenuBar::CreateChoicesMenu( QMenu
> *submenu, const char *psz_var,
> break;
>
> case VLC_VAR_INTEGER:
> - var_Get( p_object, psz_var, &val );
> + var_Get( &*p_object, psz_var, &val );
> if( CURTEXT ) menutext = qfue( CURTEXT );
> else menutext = QString::number( CURVAL.i_int );
> CreateAndConnect( submenu, psz_var, menutext, "",
> RADIO_OR_COMMAND,
> p_object, CURVAL, i_type,
> ( CURVAL.i_int == val.i_int )
> - && CheckTitle( p_object, psz_var ) );
> + && CheckTitle( &*p_object, psz_var ) );
> break;
>
> case VLC_VAR_FLOAT:
> - var_Get( p_object, psz_var, &val );
> + var_Get( &*p_object, psz_var, &val );
> if( CURTEXT ) menutext = qfue( CURTEXT );
> else menutext.sprintf( "%.2f", CURVAL.f_float );
> CreateAndConnect( submenu, psz_var, menutext, "",
> RADIO_OR_COMMAND,
> @@ -1465,7 +1497,7 @@ int VLCMenuBar::CreateChoicesMenu( QMenu
> *submenu, const char *psz_var,
>
> void VLCMenuBar::CreateAndConnect( QMenu *menu, const char *psz_var,
> const QString& text, const QString& help,
> - int i_item_type, vlc_object_t *p_obj,
> + int i_item_type, QVLCObject& p_obj,
> vlc_value_t val, int i_val_type,
> bool checked )
> {
> @@ -1513,7 +1545,7 @@ void VLCMenuBar::CreateAndConnect( QMenu *menu,
> const char *psz_var,
> void VLCMenuBar::DoAction( QObject *data )
> {
> MenuItemData *itemData = qobject_cast<MenuItemData *>( data );
> - vlc_object_t *p_object = itemData->p_obj;
> + vlc_object_t *p_object =&*itemData->obj;
> if( p_object == NULL ) return;
> const char *var = itemData->psz_var;
> vlc_value_t val = itemData->val;
> diff --git a/modules/gui/qt/menus.hpp b/modules/gui/qt/menus.hpp
> index 536e178c79..ca24c5f8e3 100644
> --- a/modules/gui/qt/menus.hpp
> +++ b/modules/gui/qt/menus.hpp
> @@ -31,18 +31,21 @@
> #include <QObject>
> #include <QMenu>
> #include <QVector>
> +#include <QSharedPointer>
> +
> +typedef QSharedPointer<vlc_object_t> QVLCObject;
>
> class MenuItemData : public QObject
> {
> Q_OBJECT
>
> public:
> - MenuItemData( QObject* parent, vlc_object_t *_p_obj, int _i_type,
> + MenuItemData( QObject* parent, QVLCObject& obj, int _i_type,
> vlc_value_t _val, const char *_var ) : QObject( parent )
> {
> - p_obj = _p_obj;
> - if( p_obj )
> - vlc_object_hold( p_obj );
> + this->obj = obj;
> + if( obj )
> + vlc_object_hold( &*obj );
Isn't the point of the "smart" pointers to avoid this and the associated release?
> i_val_type = _i_type;
> val = _val;
> psz_var = strdup( _var );
> @@ -52,11 +55,11 @@ public:
> free( psz_var );
> if( ( i_val_type & VLC_VAR_TYPE) == VLC_VAR_STRING )
> free( val.psz_string );
> - if( p_obj )
> - vlc_object_release( p_obj );
> + if( obj )
> + vlc_object_release( &*obj );
> }
>
> - vlc_object_t *p_obj;
> + QVLCObject obj;
> vlc_value_t val;
> char *psz_var;
>
> @@ -143,13 +146,13 @@ private:
>
> /* Generic automenu methods */
> static QMenu * Populate( QMenu *current,
> - QVector<const char*>&, QVector<vlc_object_t *>& );
> + QVector<const char*>&, QVector<QVLCObject>& );
>
> static void CreateAndConnect( QMenu *, const char *, const QString&,
> - const QString&, int, vlc_object_t *,
> + const QString&, int, QVLCObject& ,
> vlc_value_t, int, bool c = false );
> - static void UpdateItem( QMenu *, const char *, vlc_object_t *, bool );
> - static int CreateChoicesMenu( QMenu *,const char *, vlc_object_t * );
> + static void UpdateItem( QMenu *, const char *, QVLCObject&, bool );
> + static int CreateChoicesMenu( QMenu *,const char *, QVLCObject& );
> static void EnableStaticEntries( QMenu *, bool );
>
> /* recentMRL menu */
> --
> 2.20.1
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
--
Hugo Beauzée-Luyssen
hugo at beauzee.fr
More information about the vlc-devel
mailing list