Move the rest of the frame destructions off the video pivot.

Author: Mysticial

SerialPrograms/Source/CommonFramework/VideoPipeline/Backends/SnapshotManager.cpp

@@ -55,33 +55,35 @@ void SnapshotManager::convert(uint64_t seqnum, QVideoFrame frame, WallClock time
         }catch (...){}
     }
 
-    std::lock_guard<std::mutex> lg(m_lock);
-//    cout << "SnapshotManager::convert() - post convert: " << seqnum << endl;
-
-    if (m_converted_seqnum < seqnum){
-        m_converted_seqnum = seqnum;
-        m_converted_snapshot = std::move(snapshot);
-    }
+    {
+        std::lock_guard<std::mutex> lg(m_lock);
+//        cout << "SnapshotManager::convert() - post convert: " << seqnum << endl;
 
-    m_active_conversions--;
+        if (m_converted_seqnum < seqnum){
+            push_new_screenshot(seqnum, std::move(snapshot));
+        }
 
-    if (m_queued_convert){
-        m_queued_convert = false;
-        seqnum = m_cache.get_latest(frame, timestamp);
-        if (m_converting_seqnum < seqnum){
-            dispatch_conversion(seqnum, std::move(frame), timestamp);
+        if (m_queued_convert){
+            m_queued_convert = false;
+            seqnum = m_cache.get_latest(frame, timestamp);
+            if (m_converting_seqnum < seqnum){
+                dispatch_conversion(seqnum, std::move(frame), timestamp);
+            }
         }
     }
 
     cleanup();
 
-    //  Warning: The moment we release the lock with (m_active_conversions == 0),
-    //  this class can be immediately destructed.
+    {
+        std::lock_guard<std::mutex> lg(m_lock);
+        m_active_conversions--;
 
-    //  Therefore it is not safe to notify after releasing the lock.
-    m_cv.notify_all();
+        //  Warning: The moment we release the lock with (m_active_conversions == 0),
+        //  this class can be immediately destructed.
 
-    //  Lock is implicitly released here.
+        //  Therefore it is not safe to notify after releasing the lock.
+        m_cv.notify_all();
+    }
 }
 bool SnapshotManager::try_dispatch_conversion(uint64_t seqnum, QVideoFrame frame, WallClock timestamp) noexcept{
     //  Must call under the lock.
@@ -126,18 +128,49 @@ void SnapshotManager::dispatch_conversion(uint64_t seqnum, QVideoFrame frame, Wa
         m_active_conversions--;
     }
 }
+
+void SnapshotManager::push_new_screenshot(uint64_t seqnum, VideoSnapshot snapshot){
+    m_converted_snapshot_archive[seqnum] = snapshot;
+    m_converted_snapshot = std::move(snapshot);
+    m_converted_seqnum = seqnum;
+}
 void SnapshotManager::cleanup(){
-    //  Must call under the lock.
+    //  We do this in 2 passes. First we walk through both sets and move all the
+    //  stale objects out. Then we release the lock and destroy all the stale
+    //  objects. This minimizes the amount of time the lock is held.
+
+    std::vector<std::unique_ptr<AsyncTask>> tasks_to_free;
+    std::vector<VideoSnapshot> snapshots_to_free;
+
+    //  Pass 1: Move all stale objects out.
+    {
+        std::lock_guard<std::mutex> lg(m_lock);
+
+        //  Cleanup finished tasks.
+        while (!m_pending_conversions.empty()){
+            auto iter = m_pending_conversions.begin();
+            if (iter->second->is_finished()){
+                tasks_to_free.emplace_back(std::move(iter->second));
+                m_pending_conversions.erase(iter);
+            }else{
+                break;
+            }
+        }
 
-    //  Cleanup finished tasks.
-    while (!m_pending_conversions.empty()){
-        auto iter = m_pending_conversions.begin();
-        if (iter->second->is_finished()){
-            m_pending_conversions.erase(iter);
-        }else{
-            break;
+        //  Walk through the snapshot archive and clear out everything with only
+        //  one reference.
+        for (auto iter = m_converted_snapshot_archive.begin(); iter != m_converted_snapshot_archive.end();){
+            if (iter->second.frame.use_count() <= 1){
+                snapshots_to_free.emplace_back(std::move(iter->second));
+                iter = m_converted_snapshot_archive.erase(iter);
+            }else{
+                ++iter;
+            }
         }
     }
+
+    //  Pass 2: Destroy the stale objects.
+    //  Implicitly destroyed here.
 }
 
 
@@ -169,7 +202,6 @@ VideoSnapshot SnapshotManager::snapshot_latest_blocking(){
     WallClock timestamp;
     seqnum = m_cache.get_latest(frame, timestamp);
 
-//    m_active_conversions++;
     m_converting_seqnum = seqnum;
 
     VideoSnapshot snapshot;
@@ -183,17 +215,14 @@ VideoSnapshot SnapshotManager::snapshot_latest_blocking(){
             microseconds = (uint32_t)std::chrono::duration_cast<std::chrono::microseconds>(time1 - time0).count();
         }
         m_stats_conversion.report_data(m_logger, microseconds);
-//        m_active_conversions--;
     }catch (...){
-//        m_active_conversions--;
         m_logger.log("Exception thrown while converting QVideoFrame -> QImage.", COLOR_RED);
         throw;
     }
 
-    m_converted_seqnum = seqnum;
 
     if (timestamp > m_converted_snapshot.timestamp){
-        m_converted_snapshot = std::move(snapshot);
+        push_new_screenshot(seqnum, std::move(snapshot));
         m_cv.notify_all();
     }
 
@@ -232,22 +261,6 @@ VideoSnapshot SnapshotManager::snapshot_recent_nonblocking(WallClock min_time){
 //        cout << "snapshot_recent_nonblocking(): Too old..." << endl;
         return VideoSnapshot();
     }
-
-#if 0
-    std::chrono::microseconds window(m_stats_conversion.max());
-//    cout << "window = " << window.count() << endl;
-
-    WallClock oldest_allowed = now - 2 * window;
-//    cout << "snapshot_recent_nonblocking(): " << m_converted_seqnum << endl;
-
-    if (m_converted_snapshot.timestamp >= oldest_allowed){
-//        cout << "snapshot_recent_nonblocking(): Good..." << endl;
-        return m_converted_snapshot;
-    }
-
-//    cout << "snapshot_recent_nonblocking(): Too old..." << endl;
-    return VideoSnapshot();
-#endif
 }
 
 

SerialPrograms/Source/CommonFramework/VideoPipeline/Backends/SnapshotManager.h

@@ -7,6 +7,7 @@
 #ifndef PokemonAutomation_VideoPipeline_SnapshotManager_H
 #define PokemonAutomation_VideoPipeline_SnapshotManager_H
 
+#include <map>
 #include <mutex>
 #include <condition_variable>
 #include "Common/Cpp/AbstractLogger.h"
@@ -32,6 +33,8 @@ class SnapshotManager{
     void convert(uint64_t seqnum, QVideoFrame frame, WallClock timestamp) noexcept;
     bool try_dispatch_conversion(uint64_t seqnum, QVideoFrame frame, WallClock timestamp) noexcept;
     void dispatch_conversion(uint64_t seqnum, QVideoFrame frame, WallClock timestamp) noexcept;
+
+    void push_new_screenshot(uint64_t seqnum, VideoSnapshot snapshot);
     void cleanup();
 
 private:
@@ -47,9 +50,16 @@ class SnapshotManager{
 
     uint64_t m_converting_seqnum;
 
+    //  Store the latest converted snapshot + seqnum.
     uint64_t m_converted_seqnum;
     VideoSnapshot m_converted_snapshot;
 
+    //  Keep an archive of older snapshots. The idea here is that we don't want
+    //  snapshots to be destroyed in other places (such as the video pivot)
+    //  because it's expensive. So instead, we keep a reference here which we
+    //  will periodically clear out on the conversion threads.
+    std::map<uint64_t, VideoSnapshot> m_converted_snapshot_archive;
+
     PeriodicStatsReporterI32 m_stats_conversion;
 };
 

SerialPrograms/Source/PokemonLZA/Programs/Farming/PokemonLZA_InPlaceCatcher.cpp

@@ -240,7 +240,6 @@ void InPlaceCatcher::program(SingleSwitchProgramEnvironment& env, ProControllerC
         }
     );
 
-
     pbf_wait(context, 5 * TICKS_PER_SECOND);
     go_home(env.console, context);
     send_program_finished_notification(env, NOTIFICATION_PROGRAM_FINISH);