Skip to content
Snippets Groups Projects

Playlist rc3

Closed Alexandre Fernandez requested to merge Nerf/medialibrary:playlist-rc3 into master

Mieux

Edited by Hugo Beauzée-Luyssen

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • added 23 commits

    • 0a41f53d...3ca56d9b - 6 commits from branch videolan:master
    • ab1636d4 - Task: Allow parser to give optional Playlist context to a task
    • 307afa91 - [ARCHIVED] utils: directory: Added platform specific ways to check if a path is a directory
    • ed793d4d - VLCMetadata: Check if file already preparsed by libVLC playlist importer
    • 867c6147 - utils: file: return scheme with correct length
    • a434e66f - utils: file: Add helper to get parent directory name only
    • 07c0251d - utils: file: Add helper to strip arbitrary scheme from mrl
    • b3e780df - utils: file: Added helper to split a path
    • f6920b43 - Prober: Add interface to abstract different discoverer behavior
    • 0bbe23e4 - FsDiscoverer: Use new Probe decisions
    • f24f3c44 - [RFC] MetadataParser: Import playlist files
    • a3b8f435 - Folder: Add a table containing root points to ignore
    • 50d3d58d - MetadataParser: Exclude created entry folder from refresh list
    • 7b8a642e - Paylist: Add a table associating folders and playlists
    • a3831152 - MediaLibrary: Add info log when a folder is being deleted
    • 18312fa8 - FsDiscoverer: Add playlist deletion case
    • be572ec0 - MediaLibrary: migration: Delete already parsed playlist file
    • 188ece0b - IMediaLibrary: Add a function to destroy a MediaLibrary object

    Compare with previous version

157 157
158 158 void MediaLibraryTester::addDiscoveredFile(std::shared_ptr<fs::IFile> fileFs,
159 159 std::shared_ptr<Folder> parentFolder,
160 std::shared_ptr<fs::IDirectory> parentFolderFs)
160 std::shared_ptr<fs::IDirectory> parentFolderFs,
161 std::shared_ptr<Playlist> parentPlaylist)
161 162 {
163 (void)parentPlaylist;
  • Hugo Beauzée-Luyssen unmarked as a Work In Progress

    unmarked as a Work In Progress

  • 39 #endif
    40
    41 namespace medialibrary
    42 {
    43
    44 namespace utils
    45 {
    46
    47 namespace directory
    48 {
    49
    50 bool isDirectory( const std::string& path )
    51 {
    52 #ifdef _WIN32
    53 auto attr = GetFileAttributes( path.c_str() );
    54 return attr & FILE_ATTRIBUTE_DIRECTORY;
  • 43
    44 namespace utils
    45 {
    46
    47 namespace directory
    48 {
    49
    50 bool isDirectory( const std::string& path )
    51 {
    52 #ifdef _WIN32
    53 auto attr = GetFileAttributes( path.c_str() );
    54 return attr & FILE_ATTRIBUTE_DIRECTORY;
    55 #else
    56 struct stat s;
    57 if ( lstat( path.c_str(), &s ) != 0 )
    58 throw std::runtime_error( strerror( errno ) );
  • 68 68 return filePath.substr( 0, pos + 1 );
    69 69 }
    70 70
    71 std::string directoryName( const std::string& filePath )
    72 {
    73 auto pos = filePath.find_last_of( DIR_SEPARATOR );
    74 if ( pos == std::string::npos )
    75 return filePath;
    76 if ( pos == 0 )
    77 return filePath.substr( 1 );
    78 const std::string dir = directory( filePath );
    • I don't really understand why you start with finding the first separator, and then fallback to other helpers. Couldn't you:

      • Find the last 2 separators if any, and return the substring in between them
      • If there's only 1 separator, return filePath.substr(1);
      • If the string is empty or doesn't contain any separator, give up ?
      Edited by Hugo Beauzée-Luyssen
    • Author Contributor

      I guess it would work and be more efficient, I just used another helper because it was already multiplatform-n-stuff

    • Please register or sign in to reply
  • 28 namespace medialibrary
    29 {
    30
    31 class Playlist;
    32 class Folder;
    33
    34 namespace fs
    35 {
    36 class IDirectory;
    37 class IFile;
    38 }
    39
    40 namespace prober
    41 {
    42
    43 class Probe
  • 31 class Playlist;
    32 class Folder;
    33
    34 namespace fs
    35 {
    36 class IDirectory;
    37 class IFile;
    38 }
    39
    40 namespace prober
    41 {
    42
    43 class Probe
    44 {
    45 public:
    46 Probe() = default;
  • 52 virtual bool proceedOnDirectory( const fs::IDirectory& directory ) = 0;
    53
    54 /**
    55 * @brief proceedOnFile Decide if the FsDiscoverer should check a file or ignore it
    56 */
    57 virtual bool proceedOnFile( const fs::IFile& file ) = 0;
    58
    59 /**
    60 * @brief stopFileDiscovery Tell the FsDiscoverer if whe should stop the scan for optimisation purpose
    61 */
    62 virtual bool stopFileDiscovery() = 0;
    63
    64 /**
    65 * @brief proceedIfDotNoMedia Tell the FsDiscoverer if we want to scan folders containing a .nomedia file
    66 */
    67 virtual bool proceedIfDotNoMedia() = 0;
    • Please merge this with proceedOnDirectory

    • Oops my bad, I understood this as "hasNoMedia". However I think it makes sense to probe .nomedia from the prober now, as part of the proceedOnDirectory method

    • Author Contributor

      The catch here is that PathProbe::proceedOnDirectory() perform side effects on its object, we cannot call it as frequently as hasDotNoMediaFile, the cached path will be out of sync, and return false on a valid IDirectory object.

    • Author Contributor

      I also think these two behaviors diverge too much to share the same method name, for example in FsDiscoverer::checkFolder() the folder is deleted if hasNoMedia() is true, we don't want that if proceedOnDirectory() is false. I would rather add another shared implementation for IProbe::isHidden or something.

    • That seems like a very valid point. I'm fine with keeping the 2 methods separated for now then!

    • Please register or sign in to reply
  • 47 virtual ~Probe() = default;
    48
    49 /**
    50 * @brief proceedOnDirectory Decide whether or not the FsDiscoverer should scan a directory
    51 */
    52 virtual bool proceedOnDirectory( const fs::IDirectory& directory ) = 0;
    53
    54 /**
    55 * @brief proceedOnFile Decide if the FsDiscoverer should check a file or ignore it
    56 */
    57 virtual bool proceedOnFile( const fs::IFile& file ) = 0;
    58
    59 /**
    60 * @brief stopFileDiscovery Tell the FsDiscoverer if whe should stop the scan for optimisation purpose
    61 */
    62 virtual bool stopFileDiscovery() = 0;
  • 65 * @brief proceedIfDotNoMedia Tell the FsDiscoverer if we want to scan folders containing a .nomedia file
    66 */
    67 virtual bool proceedIfDotNoMedia() = 0;
    68
    69 /**
    70 * @brief proceedOnDirectory Decide if the FsDiscoverer should delete folders not found on the file system
    71 */
    72 virtual bool deleteUnseenFolders() = 0;
    73
    74 /**
    75 * @brief deleteUnseenFiles Decide if the FsDiscoverer should delete files not found on the file system
    76 */
    77 virtual bool deleteUnseenFiles() = 0;
    78
    79 /**
    80 * @brief forceFileRefresh Decide if we force-add discovered files to filesToAdd, like if it was new ones
  • 31
    32 namespace medialibrary
    33 {
    34
    35 namespace prober
    36 {
    37
    38 PathProbe::PathProbe( const std::string& path, bool isDirectory, std::shared_ptr<Playlist> parentPlaylist,
    39 std::shared_ptr<Folder> parentFolder, const std::string& parentFolderPath, bool reload )
    40 : m_isDirectory( isDirectory )
    41 , m_isDiscoveryEnded( false )
    42 , m_parentPlaylist( std::move( parentPlaylist ) )
    43 , m_parentFolder( std::move( parentFolder ) )
    44 , m_path( path )
    45 { // FIXME: Is that constructor dangerous? Can weird path triggers infinite loop?
    46 m_splittedPath = utils::file::splitPath( path, isDirectory );
  • 211 211 return mrl.substr( 0, pos + 3 );
    212 212 }
    213 213
    214 std::stack<std::string> splitPath( const std::string& path, bool isDirectory )
  • 36 {
    37
    38 PathProbe::PathProbe( const std::string& path, bool isDirectory, std::shared_ptr<Playlist> parentPlaylist,
    39 std::shared_ptr<Folder> parentFolder, const std::string& parentFolderPath, bool reload )
    40 : m_isDirectory( isDirectory )
    41 , m_isDiscoveryEnded( false )
    42 , m_parentPlaylist( std::move( parentPlaylist ) )
    43 , m_parentFolder( std::move( parentFolder ) )
    44 , m_path( path )
    45 { // FIXME: Is that constructor dangerous? Can weird path triggers infinite loop?
    46 m_splittedPath = utils::file::splitPath( path, isDirectory );
    47 if ( this->m_parentFolder != nullptr && m_splittedPath.empty() == false )
    48 {
    49 auto parentSplitted = utils::file::splitPath( parentFolderPath, true );
    50 while ( parentSplitted.empty() == false &&
    51 parentSplitted.top() == m_splittedPath.top() ) // Safety check
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading