[vlc-devel] [PATCH 03/14] qt: refcount children passed to onItemAdded in NetworkMediaModel

Alexandre Janniaux ajanni at videolabs.io
Thu Feb 6 15:49:18 CET 2020


Hi,

Comment inline

On Thu, Feb 06, 2020 at 01:56:40PM +0100, Pierre Lamot wrote:
> ---
>  modules/gui/qt/network/networkmediamodel.cpp | 23 ++++++++++++++------
>  modules/gui/qt/network/networkmediamodel.hpp |  2 +-
>  2 files changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/modules/gui/qt/network/networkmediamodel.cpp b/modules/gui/qt/network/networkmediamodel.cpp
> index dca7534438..5d27ab8eb6 100644
> --- a/modules/gui/qt/network/networkmediamodel.cpp
> +++ b/modules/gui/qt/network/networkmediamodel.cpp
> @@ -296,7 +296,12 @@ void NetworkMediaModel::onItemCleared( MediaSourcePtr mediaSource, input_item_no
>          if (!found)
>              return;
>
> -        refreshMediaList( std::move( mediaSource ), res->pp_children, res->i_children, true );
> +        std::vector<InputItemPtr> itemList;
> +        itemList.reserve( static_cast<size_t>(res->i_children) );
> +        for (int i = 0; i < res->i_children; i++)
> +            itemList.emplace_back(res->pp_children[i]->p_item);
> +
> +        refreshMediaList( std::move( mediaSource ), std::move( itemList ), true );
>      }, Qt::QueuedConnection);
>  }
>
> @@ -305,9 +310,14 @@ void NetworkMediaModel::onItemAdded( MediaSourcePtr mediaSource, input_item_node
>                                    size_t count )
>  {
>      InputItemPtr p_parent { parent->p_item };
> -    QMetaObject::invokeMethod(this, [this, p_parent = std::move(p_parent), mediaSource = std::move(mediaSource), children, count]() {
> +    std::vector<InputItemPtr> itemList;
> +    itemList.reserve( count );
> +    for (size_t i = 0; i < count; i++)
> +        itemList.emplace_back(children[i]->p_item);
> +
> +    QMetaObject::invokeMethod(this, [this, p_parent = std::move(p_parent), mediaSource = std::move(mediaSource), itemList=std::move(itemList)]() {
>          if ( p_parent == m_treeItem.media )
> -            refreshMediaList( std::move( mediaSource ), children, count, false );
> +            refreshMediaList( std::move( mediaSource ), std::move( itemList ), false );
>      }, Qt::QueuedConnection);
>  }
>
> @@ -369,13 +379,12 @@ void NetworkMediaModel::onItemPreparseEnded(MediaSourcePtr, input_item_node_t* n
>  }
>
>  void NetworkMediaModel::refreshMediaList( MediaSourcePtr mediaSource,
> -                                       input_item_node_t* const children[], size_t count,
> +                                       std::vector<InputItemPtr> childrens,
>                                         bool clear )
>  {
>      std::vector<Item> items;
> -    for ( auto i = 0u; i < count; ++i )
> +    for ( auto it: childrens)
>      {
> -        auto it = children[i]->p_item;
>          Item item;
>          item.name = it->psz_name;
>          item.protocol = "";
> @@ -394,7 +403,7 @@ void NetworkMediaModel::refreshMediaList( MediaSourcePtr mediaSource,
>                                      &item.indexed ) != VLC_SUCCESS )
>                  item.indexed = false;
>          }
> -        item.tree = NetworkTreeItem( mediaSource, it );
> +        item.tree = NetworkTreeItem( mediaSource, it.get() );

It can be done later, but shouldn't we use std::move on `it`
here to avoid a useless Hold/Release?

>          items.push_back( std::move( item ) );
>      }
>      if ( clear == true )
> diff --git a/modules/gui/qt/network/networkmediamodel.hpp b/modules/gui/qt/network/networkmediamodel.hpp
> index 969d085dc4..59dae326e6 100644
> --- a/modules/gui/qt/network/networkmediamodel.hpp
> +++ b/modules/gui/qt/network/networkmediamodel.hpp
> @@ -162,7 +162,7 @@ private:
>      void onItemRemoved( MediaSourcePtr mediaSource, input_item_node_t * node, input_item_node_t *const children[], size_t count ) override;
>      void onItemPreparseEnded( MediaSourcePtr mediaSource, input_item_node_t* node, enum input_item_preparse_status status ) override;
>
> -    void refreshMediaList(MediaSourcePtr s, input_item_node_t* const children[], size_t count , bool clear);
> +    void refreshMediaList(MediaSourcePtr s, std::vector<InputItemPtr> childrens , bool clear);
>
>      static bool canBeIndexed(const QUrl& url , ItemType itemType );
>
> --
> 2.17.1
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel


More information about the vlc-devel mailing list