<div id="geary-body" dir="auto">Hi,<div>Thanks for the review, i'll split-up the commits and enhance the description!</div><div><br></div></div><div id="geary-quote" dir="auto"><br>On Fri, Feb 5, 2021 at 10:07, Thomas Guillem <thomas@gllm.fr> wrote:<br><blockquote type="cite"><div class="plaintext" style="white-space: pre-wrap;">Hello Alaric,

Thanks for the patches!

Nit (nitpicking): the commit title should be short, add more description (the why?) in the commit message. Cf. <a href="https://chris.beams.io/posts/git-commit/">https://chris.beams.io/posts/git-commit/</a>

It seems this patches is adding 4 different. It is possible to split them in 4 patches?


On Thu, Feb 4, 2021, at 17:15, Alaric Senat wrote:
<blockquote> ---
  include/vlc_media_library.h                |  2 +
  modules/misc/medialibrary/entities.cpp     |  2 +
  modules/misc/medialibrary/fs/directory.cpp | 51 +++++++++++++++++++++-
  modules/misc/medialibrary/fs/directory.h   |  1 +
  modules/misc/medialibrary/fs/file.cpp      | 45 ++++++++++++++++---
  modules/misc/medialibrary/fs/file.h        | 20 ++++++---
  modules/misc/medialibrary/fs/fs.cpp        |  2 +-
  modules/misc/medialibrary/fs/util.cpp      |  8 ++++
  modules/misc/medialibrary/fs/util.h        |  1 +
  9 files changed, 118 insertions(+), 14 deletions(-)
 
 diff --git a/include/vlc_media_library.h b/include/vlc_media_library.h
 index fde638692d..b9df2e8ca8 100644
 --- a/include/vlc_media_library.h
 +++ b/include/vlc_media_library.h
 @@ -150,6 +150,8 @@ typedef struct vlc_ml_label_list_t
  typedef struct vlc_ml_file_t
  {
      char* psz_mrl;
 +    int64_t i_size;
 +    time_t i_last_modification_date;
      vlc_ml_file_type_t i_type;
      bool b_external;
      bool b_removable;
 diff --git a/modules/misc/medialibrary/entities.cpp 
 b/modules/misc/medialibrary/entities.cpp
 index 02c106e4e1..1618c4969f 100644
 --- a/modules/misc/medialibrary/entities.cpp
 +++ b/modules/misc/medialibrary/entities.cpp
 @@ -295,6 +295,8 @@ bool Convert( const medialibrary::IFile* input, 
 vlc_ml_file_t& output )
              vlc_assert_unreachable();
      }
  
 +    output.i_size = input->size();
 +    output.i_last_modification_date = input->lastModificationDate();
      output.b_removable = input->isRemovable();
      output.b_present = true;
      try
 diff --git a/modules/misc/medialibrary/fs/directory.cpp 
 b/modules/misc/medialibrary/fs/directory.cpp
 index 50ef287217..256f7e87a8 100644
 --- a/modules/misc/medialibrary/fs/directory.cpp
 +++ b/modules/misc/medialibrary/fs/directory.cpp
 @@ -26,12 +26,15 @@
  #include "file.h"
  #include "util.h"
  
 +#include <sys/stat.h>
  #include <algorithm>
  #include <assert.h>
  #include <vector>
  #include <system_error>
  #include <vlc_common.h>
 +#include <vlc_url.h>
  #include <vlc_input_item.h>
 +#include <vlc_fs.h>
  #include <vlc_input.h>
  #include <vlc_threads.h>
  #include <vlc_cxx_helpers.hpp>
 @@ -188,6 +191,7 @@ SDDirectory::read() const
  
      input_item_AddOption( media.get(), "show-hiddenfiles", 
 VLC_INPUT_OPTION_TRUSTED );
      input_item_AddOption( media.get(), "ignore-filetypes=''", 
 VLC_INPUT_OPTION_TRUSTED );
 +    input_item_AddOption( media.get(), "sub-autodetect-fuzzy=2", 
 VLC_INPUT_OPTION_TRUSTED );
      auto status = request_metadata_sync( m_fs.libvlc(), media.get(), 
 &children);
  
      if ( status == false )
 @@ -199,13 +203,58 @@ SDDirectory::read() const
          const char *mrl = m.get()->psz_uri;
          enum input_item_type_e type = m->i_type;
          if (type == ITEM_TYPE_DIRECTORY)
 +        {
              m_dirs.push_back(std::make_shared<SDDirectory>(mrl, m_fs));
 +        }
          else if (type == ITEM_TYPE_FILE)
 -            m_files.push_back(std::make_shared<SDFile>(mrl));
 +        {
 +            addFile( mrl, IFile::LinkedFileType::None, {} );
 +            for ( auto i = 0; i < m->i_slaves; ++i )
 +            {
 +                const auto* slave = m->pp_slaves[i];
 +                const auto linked_type = slave->i_type == 
 SLAVE_TYPE_AUDIO
 +                                             ? 
 IFile::LinkedFileType::SoundTrack
 +                                             : 
 IFile::LinkedFileType::Subtitles;
 +
 +                addFile( slave->psz_uri, linked_type, mrl );
 +            }
 +        }
      }
  
      m_read_done = true;
  }
  
 +void
 +SDDirectory::addFile(std::string mrl, IFile::LinkedFileType fType, 
 std::string linkedFile) const
 +{
 +    time_t lastModificationDate = 0;
 +    int64_t fileSize = 0;
 +
 +    if ( m_fs.isNetworkFileSystem() == false )
 +    {
 +        const auto path = vlc::wrap_cptr( vlc_uri2path(mrl.c_str()) );
 +        struct stat stat;
 +
 +        if ( vlc_stat( path.get(), &stat ) != 0 )
 +        {
 +            if ( errno == EACCES )
 +                return;
 +            throw errors::System{ errno, "Failed to get file info" };
 +        }
 +        lastModificationDate = stat.st_mtime;
 +        fileSize = stat.st_size;
 +    }
 +
 +    if (fType == IFile::LinkedFileType::None)
 +    {
 +        m_files.push_back(
 +            std::make_shared<SDFile>( std::move( mrl ), fileSize, 
 lastModificationDate ) );
 +    }
 +    else
 +    {
 +        m_files.push_back( std::make_shared<SDFile>(
 +            std::move( mrl ), fType, std::move( linkedFile ), 
 fileSize, lastModificationDate ) );
 +    }
 +}
    } /* namespace medialibrary */
  } /* namespace vlc */
 diff --git a/modules/misc/medialibrary/fs/directory.h 
 b/modules/misc/medialibrary/fs/directory.h
 index c73ef1b1a3..1409cd0d95 100644
 --- a/modules/misc/medialibrary/fs/directory.h
 +++ b/modules/misc/medialibrary/fs/directory.h
 @@ -43,6 +43,7 @@ public:
  
  private:
      void read() const;
 +    void addFile( std::string mrl, fs::IFile::LinkedFileType, 
 std::string linkedWith ) const;
  
      std::string m_mrl;
      SDFileSystemFactory &m_fs;
 diff --git a/modules/misc/medialibrary/fs/file.cpp 
 b/modules/misc/medialibrary/fs/file.cpp
 index 77b48329e5..645ebb4597 100644
 --- a/modules/misc/medialibrary/fs/file.cpp
 +++ b/modules/misc/medialibrary/fs/file.cpp
 @@ -25,13 +25,38 @@
  #include "file.h"
  #include "util.h"
  
 +#include <vlc_common.h>
 +#include <vlc_url.h>
 +
 +#include <sys/stat.h>
 +#include <iostream>
 +
  namespace vlc {
    namespace medialibrary {
  
 -SDFile::SDFile(const std::string &mrl)
 -    : m_mrl(mrl)
 -    , m_name(utils::fileName(mrl))
 -    , m_extension(utils::extension(mrl))
 +SDFile::SDFile( const std::string mrl, const int64_t size, const 
 time_t lastModificationDate )
 +    : m_mrl( std::move( mrl ) )
 +    , m_name( utils::fileName( m_mrl ) )
 +    , m_extension( utils::extension( m_mrl ) )
 +    , m_isNetwork( m_mrl.rfind( "<a href="file://" "="">file://"</a> ) != 0 )
 +    , m_size( size )
 +    , m_lastModificationTime( lastModificationDate )
 +{
 +}
 +
 +SDFile::SDFile( const std::string mrl,
 +                const LinkedFileType fType,
 +                const std::string linkedFile,
 +                const int64_t size,
 +                const time_t lastModificationDate )
 +    : m_mrl( std::move( mrl ) )
 +    , m_name( utils::fileName( m_mrl ) )
 +    , m_extension( utils::extension( m_mrl ) )
 +    , m_linkedFile( std::move( linkedFile ) )
 +    , m_linkedType( fType )
 +    , m_isNetwork( m_mrl.rfind( "<a href="file://" "="">file://"</a> ) != 0 )
 +    , m_size( size )
 +    , m_lastModificationTime( lastModificationDate )
  {
  }
  
 @@ -56,18 +81,24 @@ SDFile::extension() const
  time_t
  SDFile::lastModificationDate() const
  {
 -    return 0;
 +    return m_lastModificationTime;
 +}
 +
 +bool
 +SDFile::isNetwork() const
 +{
 +  return m_isNetwork;
  }
  
  int64_t
  SDFile::size() const
  {
 -    return 0;
 +    return m_size;
  }
  
  IFile::LinkedFileType SDFile::linkedType() const
  {
 -    return IFile::LinkedFileType::None;
 +    return m_linkedType;
  }
  
  const std::string &SDFile::linkedWith() const
 diff --git a/modules/misc/medialibrary/fs/file.h 
 b/modules/misc/medialibrary/fs/file.h
 index 05730f6bda..c6ce6d43d5 100644
 --- a/modules/misc/medialibrary/fs/file.h
 +++ b/modules/misc/medialibrary/fs/file.h
 @@ -31,22 +31,32 @@ using namespace ::medialibrary::fs;
  class SDFile : public IFile
  {
  public:
 -    explicit SDFile(const std::string &mrl);
 +    explicit SDFile( std::string mrl, int64_t, time_t );
 +    explicit SDFile( std::string mrl,
 +                     LinkedFileType,
 +                     std::string linkedFile,
 +                     int64_t,
 +                     time_t );
 +
      virtual ~SDFile() = default;
      const std::string& mrl() const override;
      const std::string& name() const override;
      const std::string& extension() const override;
 -    time_t lastModificationDate() const override;
 -    int64_t size() const override;
 -    inline bool isNetwork() const override { return true; }
 +    const std::string& linkedWith() const override;
      LinkedFileType linkedType() const override;
 -    const std::string &linkedWith() const override;
 +    bool isNetwork() const override;
 +    int64_t size() const override;
 +    time_t lastModificationDate() const override;
  
  private:
      std::string m_mrl;
      std::string m_name;
      std::string m_extension;
      std::string m_linkedFile;
 +    LinkedFileType m_linkedType = LinkedFileType::None;
 +    bool m_isNetwork;
 +    int64_t m_size = 0;
 +    time_t m_lastModificationTime = 0;
  };
  
    } /* namespace medialibrary */
 diff --git a/modules/misc/medialibrary/fs/fs.cpp 
 b/modules/misc/medialibrary/fs/fs.cpp
 index 85323cb231..c81439b0f9 100644
 --- a/modules/misc/medialibrary/fs/fs.cpp
 +++ b/modules/misc/medialibrary/fs/fs.cpp
 @@ -60,7 +60,7 @@ SDFileSystemFactory::createDirectory(const 
 std::string &mrl)
  std::shared_ptr<fs::IFile>
  SDFileSystemFactory::createFile(const std::string& mrl)
  {
 -    auto dir = createDirectory(mrl);
 +    auto dir = createDirectory( utils::directory( mrl ) );
      assert(dir != nullptr);
      return dir->file(mrl);
  }
 diff --git a/modules/misc/medialibrary/fs/util.cpp 
 b/modules/misc/medialibrary/fs/util.cpp
 index f25f96d161..7f22425c07 100644
 --- a/modules/misc/medialibrary/fs/util.cpp
 +++ b/modules/misc/medialibrary/fs/util.cpp
 @@ -52,6 +52,14 @@ fileName(const std::string &filePath)
      return filePath.substr(pos + 1);
  }
  
 +std::string
 +directory(const std::string &filePath)
 +{
 +    auto pos = filePath.find_last_of(DIR_SEPARATORS);
 +    if (pos == std::string::npos)
 +        return filePath;
 +    return filePath.substr(0, pos);
 +}
      } /* namespace utils */
    } /* namespace medialibrary */
  } /* namespace vlc */
 diff --git a/modules/misc/medialibrary/fs/util.h 
 b/modules/misc/medialibrary/fs/util.h
 index 6cbff37114..10862477ad 100644
 --- a/modules/misc/medialibrary/fs/util.h
 +++ b/modules/misc/medialibrary/fs/util.h
 @@ -29,6 +29,7 @@ namespace vlc {
  
  std::string fileName(const std::string& filePath);
  std::string extension(const std::string& fileName);
 +std::string directory(const std::string& fileName);
  
      } /* namespace utils */
    } /* namespace medialibrary */
 -- 
 2.28.0
 
 _______________________________________________
 vlc-devel mailing list
 To unsubscribe or modify your subscription options:
 <a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a>
</blockquote>_______________________________________________
vlc-devel mailing list
To unsubscribe or modify your subscription options:
<a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a></div></blockquote></div>