File: Improve parsing status granularity

src/File.cpp

@@ -45,7 +45,7 @@ File::File( MediaLibraryPtr ml, sqlite::Row& row )
         >> m_type
         >> m_lastModificationDate
         >> m_size
-        >> m_isParsed
+        >> m_parserSteps
         >> m_folderId
         >> m_isPresent
         >> m_isRemovable;
@@ -59,7 +59,7 @@ File::File( MediaLibraryPtr ml, int64_t mediaId, Type type, const fs::IFile& fil
     , m_type( type )
     , m_lastModificationDate( file.lastModificationDate() )
     , m_size( file.size() )
-    , m_isParsed( false )
+    , m_parserSteps( ParserStep::None )
     , m_folderId( folderId )
     , m_isPresent( true )
     , m_isRemovable( isRemovable )
@@ -101,9 +101,23 @@ unsigned int File::size() const
     return m_size;
 }
 
-bool File::isParsed() const
+void File::markStepCompleted( ParserStep step )
 {
-    return m_isParsed;
+    m_parserSteps = static_cast<ParserStep>( static_cast<uint8_t>( m_parserSteps ) |
+                                             static_cast<uint8_t>( step ) );
+}
+
+bool File::saveParserStep()
+{
+    static const std::string req = "UPDATE " + policy::FileTable::Name + " SET parser_step = ? WHERE id_file = ?";
+    if ( sqlite::Tools::executeUpdate( m_ml->getConn(), req, m_parserSteps, m_id ) == false )
+        return false;
+    return true;
+}
+
+File::ParserStep File::parserStep() const
+{
+    return m_parserSteps;
 }
 
 std::shared_ptr<Media> File::media() const
@@ -121,16 +135,6 @@ bool File::destroy()
     return DatabaseHelpers::destroy( m_ml, m_id );
 }
 
-void File::markParsed()
-{
-    if ( m_isParsed == true )
-        return;
-    static const std::string req = "UPDATE " + policy::FileTable::Name + " SET parsed = 1 WHERE id_file = ?";
-    if ( sqlite::Tools::executeUpdate( m_ml->getConn(), req, m_id ) == false )
-        return;
-    m_isParsed = true;
-}
-
 bool File::createTable( DBConnection dbConnection )
 {
     std::string req = "CREATE TABLE IF NOT EXISTS " + policy::FileTable::Name + "("
@@ -140,7 +144,7 @@ bool File::createTable( DBConnection dbConnection )
             "type UNSIGNED INTEGER,"
             "last_modification_date UNSIGNED INT,"
             "size UNSIGNED INT,"
-            "parsed BOOLEAN NOT NULL DEFAULT 0,"
+            "parser_step INTEGER NOT NULL DEFAULT 0,"
             "folder_id UNSIGNED INTEGER,"
             "is_present BOOLEAN NOT NULL DEFAULT 1,"
             "is_removable BOOLEAN NOT NULL,"

src/File.h

@@ -27,6 +27,7 @@
 #include "database/SqliteConnection.h"
 #include "filesystem/IFile.h"
 #include "utils/Cache.h"
+#include "parser/Parser.h"
 
 namespace medialibrary
 {
@@ -47,6 +48,16 @@ struct FileTable
 class File : public IFile, public DatabaseHelpers<File, policy::FileTable>
 {
 public:
+    enum class ParserStep : uint8_t
+    {
+        None = 0,
+        MetadataExtraction = 1,
+        MetadataAnalysis = 2,
+        Thumbnailer = 4,
+
+        Completed = 1 | 2 | 4,
+    };
+
     File( MediaLibraryPtr ml, sqlite::Row& row );
     File( MediaLibraryPtr ml, int64_t mediaId, Type type, const fs::IFile& file, int64_t folderId, bool isRemovable );
     virtual int64_t id() const override;
@@ -54,11 +65,16 @@ public:
     virtual Type type() const override;
     virtual unsigned int lastModificationDate() const override;
     virtual unsigned int size() const override;
-    /// Explicitely mark a media as fully parsed, meaning no metadata service needs to run anymore.
-    //FIXME: This lacks granularity as we don't have a straight forward way to know which service
-    //needs to run or not.
-    void markParsed();
-    bool isParsed() const;
+    /*
+     * We need to decouple the current parser state and the saved one.
+     * For instance, metadata extraction won't save anything in DB, so while
+     * we might want to know that it's been processed and metadata have been
+     * extracted, in case we were to restart the parsing, we would need to
+     * extract the same information again
+     */
+    void markStepCompleted( ParserStep step );
+    bool saveParserStep();
+    ParserStep parserStep() const;
     std::shared_ptr<Media> media() const;
     bool destroy();
 
@@ -90,7 +106,7 @@ private:
     Type m_type;
     unsigned int m_lastModificationDate;
     unsigned int m_size;
-    bool m_isParsed;
+    ParserStep m_parserSteps;
     int64_t m_folderId;
     bool m_isPresent;
     bool m_isRemovable;

src/metadata_services/MetadataParser.cpp

@@ -51,19 +51,21 @@ parser::Task::Status MetadataParser::run( parser::Task& task )
 {
     auto& media = task.media;
 
-    auto t = m_ml->getConn()->newTransaction();
-    // Some media (ogg/ts, most likely) won't have visible tracks, but shouldn't be considered audio files.
     bool isAudio = task.videoTracks.empty() && task.audioTracks.empty() == false;
-    for ( const auto& t : task.videoTracks )
-    {
-        media->addVideoTrack( t.fcc, t.width, t.height, t.fps, t.language, t.description );
-    }
-    for ( const auto& t : task.audioTracks )
     {
-        media->addAudioTrack( t.fcc, t.bitrate, t.samplerate, t.nbChannels,
-                              t.language, t.description );
+        auto t = m_ml->getConn()->newTransaction();
+        // Some media (ogg/ts, most likely) won't have visible tracks, but shouldn't be considered audio files.
+        for ( const auto& t : task.videoTracks )
+        {
+            media->addVideoTrack( t.fcc, t.width, t.height, t.fps, t.language, t.description );
+        }
+        for ( const auto& t : task.audioTracks )
+        {
+            media->addAudioTrack( t.fcc, t.bitrate, t.samplerate, t.nbChannels,
+                                  t.language, t.description );
+        }
+        t->commit();
     }
-    t->commit();
     if ( isAudio == true )
     {
         if ( parseAudioFile( task ) == false )
@@ -76,8 +78,14 @@ parser::Task::Status MetadataParser::run( parser::Task& task )
     }
     auto duration = task.duration;
     media->setDuration( duration );
+
+    auto t = m_ml->getConn()->newTransaction();
     if ( media->save() == false )
         return parser::Task::Status::Error;
+    task.file->markStepCompleted( File::ParserStep::MetadataAnalysis );
+    if ( task.file->saveParserStep() == false )
+        return parser::Task::Status::Error;
+    t->commit();
     m_notifier->notifyMediaCreation( media );
     return parser::Task::Status::Success;
 }
@@ -428,4 +436,9 @@ uint8_t MetadataParser::nbThreads() const
     return 1;
 }
 
+File::ParserStep MetadataParser::step() const
+{
+    return File::ParserStep::MetadataAnalysis;
+}
+
 }

src/metadata_services/MetadataParser.h

@@ -37,6 +37,7 @@ protected:
     virtual parser::Task::Status run( parser::Task& task ) override;
     virtual const char* name() const override;
     virtual uint8_t nbThreads() const override;
+    virtual File::ParserStep step() const override;
 
     bool parseAudioFile(parser::Task& task) const;
     bool parseVideoFile(parser::Task& task) const;

src/metadata_services/vlc/VLCMetadataService.cpp

@@ -45,12 +45,6 @@ parser::Task::Status VLCMetadataService::run( parser::Task& task )
 {
     auto media = task.media;
     auto file = task.file;
-    // FIXME: This is now becomming an invalid predicate
-    if ( media->duration() != -1 )
-    {
-        LOG_INFO( file->mrl(), " was already parsed" );
-        return parser::Task::Status::Success;
-    }
 
     LOG_INFO( "Parsing ", file->mrl() );
     auto chrono = std::chrono::steady_clock::now();
@@ -99,6 +93,9 @@ parser::Task::Status VLCMetadataService::run( parser::Task& task )
     storeMeta( task, vlcMedia );
     auto duration = std::chrono::steady_clock::now() - chrono;
     LOG_DEBUG("VLC parsing done in ", std::chrono::duration_cast<std::chrono::microseconds>( duration ).count(), "µs" );
+    // Don't save the file parsing step yet, since all data are just in memory. Just mark
+    // the extraction as done.
+    task.file->markStepCompleted( File::ParserStep::MetadataExtraction );
     return parser::Task::Status::Success;
 }
 
@@ -112,6 +109,11 @@ uint8_t VLCMetadataService::nbThreads() const
     return 1;
 }
 
+File::ParserStep VLCMetadataService::step() const
+{
+    return File::ParserStep::MetadataExtraction;
+}
+
 void VLCMetadataService::storeMeta( parser::Task& task, VLC::Media& vlcMedia )
 {
 #if LIBVLC_VERSION_INT >= LIBVLC_VERSION(3, 0, 0, 0)

src/metadata_services/vlc/VLCMetadataService.h

@@ -28,6 +28,7 @@
 #include <mutex>
 
 #include "parser/ParserService.h"
+#include "parser/Parser.h"
 #include "AlbumTrack.h"
 
 namespace medialibrary
@@ -42,6 +43,7 @@ private:
         virtual parser::Task::Status run( parser::Task& task ) override;
         virtual const char* name() const override;
         virtual uint8_t nbThreads() const override;
+        virtual File::ParserStep step() const override;
         void storeMeta(parser::Task& task, VLC::Media& vlcMedia );
         int toInt(VLC::Media& vlcMedia, libvlc_meta_t meta, const char* name );
 

src/metadata_services/vlc/VLCThumbnailer.cpp

@@ -63,6 +63,11 @@ bool VLCThumbnailer::initialize()
     return true;
 }
 
+File::ParserStep VLCThumbnailer::step() const
+{
+    return File::ParserStep::Thumbnailer;
+}
+
 parser::Task::Status VLCThumbnailer::run( parser::Task& task )
 {
     auto media = task.media;
@@ -78,11 +83,8 @@ parser::Task::Status VLCThumbnailer::run( parser::Task& task )
     else if ( media->type() != IMedia::Type::VideoType )
     {
         // There's no point in generating a thumbnail for a non-video media.
-        return parser::Task::Status::Success;
-    }
-    else if ( media->thumbnail().empty() == false )
-    {
-        LOG_INFO(media->thumbnail(), " already has a thumbnail" );
+        task.file->markStepCompleted( File::ParserStep::Thumbnailer );
+        task.file->saveParserStep();
         return parser::Task::Status::Success;
     }
 
@@ -248,8 +250,13 @@ parser::Task::Status VLCThumbnailer::compress( std::shared_ptr<Media> media, std
 
     media->setThumbnail( path );
     LOG_INFO( "Done generating ", file->mrl(), " thumbnail" );
+    auto t = m_ml->getConn()->newTransaction();
     if ( media->save() == false )
         return parser::Task::Status::Error;
+    file->markStepCompleted( File::ParserStep::Thumbnailer );
+    if ( file->saveParserStep() == false )
+        return parser::Task::Status::Error;
+    t->commit();
     m_notifier->notifyMediaModification( media );
     return parser::Task::Status::Success;
 }

src/metadata_services/vlc/VLCThumbnailer.h

@@ -39,6 +39,7 @@ public:
     virtual ~VLCThumbnailer() = default;
     virtual parser::Task::Status run( parser::Task& task ) override;
     virtual bool initialize() override;
+    virtual File::ParserStep step() const override;
 
 private:
     parser::Task::Status startPlayback( VLC::MediaPlayer& mp );

src/parser/Parser.cpp

@@ -102,8 +102,8 @@ void Parser::restore()
         return;
 
     static const std::string req = "SELECT * FROM " + policy::FileTable::Name
-            + " WHERE parsed = 0 AND is_present = 1";
-    auto files = File::fetchAll<File>( m_ml, req );
+            + " WHERE parser_step != ? AND is_present = 1";
+    auto files = File::fetchAll<File>( m_ml, req, File::ParserStep::Completed );
 
     for ( auto& f : files )
     {
@@ -141,11 +141,8 @@ void Parser::done( std::unique_ptr<parser::Task> t, parser::Task::Status status
     }
     updateStats();
 
-    if ( serviceIdx == m_services.size() )
-    {
-        t->file->markParsed();
+    if ( t->file->parserStep() == File::ParserStep::Completed )
         return;
-    }
     m_services[serviceIdx]->parse( std::move( t ) );
 }
 

src/parser/ParserService.cpp

@@ -143,6 +143,12 @@ void ParserService::mainloop()
             task = std::move( m_tasks.front() );
             m_tasks.pop();
         }
+        if ( ( static_cast<uint8_t>( task->file->parserStep() ) &
+               static_cast<uint8_t>( step() ) ) != 0 )
+        {
+            m_parserCb->done( std::move( task ), parser::Task::Status::Success );
+            continue;
+        }
         parser::Task::Status status;
         try
         {

src/parser/ParserService.h

@@ -30,6 +30,7 @@
 #include "medialibrary/Types.h"
 #include "compat/Mutex.h"
 #include "compat/Thread.h"
+#include "File.h"
 
 namespace medialibrary
 {
@@ -69,6 +70,7 @@ protected:
     virtual parser::Task::Status run( parser::Task& task ) = 0;
     virtual const char* name() const = 0;
     virtual uint8_t nbThreads() const = 0;
+    virtual File::ParserStep step() const = 0;
 
 private:
     // Thread(s) entry point

test/unittest/FileTests.cpp

@@ -47,7 +47,6 @@ protected:
 TEST_F( Files, Create )
 {
     ASSERT_NE( 0u, f->id() );
-    ASSERT_FALSE( f->isParsed() );
     ASSERT_EQ( "media.mkv", f->mrl() );
     ASSERT_NE( 0u, f->lastModificationDate() );
     ASSERT_NE( 0u, f->size() );