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

[Pierre Lamot]

On 2020-02-06 15:49, Alexandre Janniaux wrote:
> 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?

'it' items belongs to the 'childrens' list, I don't think they can't be 
moved directly.
Maybe we can do something like using a std::transform with move 
iterators, but this is
out of the scope of this patch

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