Refactoring Session saving

use Session::save static method to save a session from a thread (same mechanism as Session::load). It calls Action::takeSnapshot if saving version is required. Mixer is busy during saving, pops up info when done.

ActionManager.cpp

@@ -45,40 +45,6 @@
 
 using namespace tinyxml2;
 
-void captureMixerSession(tinyxml2::XMLDocument *doc, std::string node, std::string label)
-{
-    // create node
-    XMLElement *sessionNode = doc->NewElement( node.c_str() );
-    doc->InsertEndChild(sessionNode);
-    // label describes the action
-    sessionNode->SetAttribute("label", label.c_str() );
-    // label describes the action
-    sessionNode->SetAttribute("date", SystemToolkit::date_time_string().c_str() );
-    // view indicates the view when this action occured
-    sessionNode->SetAttribute("view", (int) Mixer::manager().view()->mode());
-
-    // get session to operate on
-    Session *se = Mixer::manager().session();
-
-    // get the thumbnail (requires one opengl update to render)
-    FrameBufferImage *thumbnail = se->renderThumbnail();
-    if (thumbnail) {
-        XMLElement *imageelement = SessionVisitor::ImageToXML(thumbnail, doc);
-        if (imageelement)
-            sessionNode->InsertEndChild(imageelement);
-        delete thumbnail;
-    }
-
-    // save session attributes
-    sessionNode->SetAttribute("activationThreshold", se->activationThreshold());
-
-    // save all sources using source visitor
-    SessionVisitor sv(doc, sessionNode);
-    for (auto iter = se->begin(); iter != se->end(); ++iter, sv.setRoot(sessionNode) )
-        (*iter)->accept(sv);
-
-}
-
 
 Action::Action(): history_step_(0), history_max_step_(0), locked_(false),
     snapshot_id_(0), snapshot_node_(nullptr), interpolator_(nullptr), interpolator_node_(nullptr)
@@ -100,6 +66,40 @@ void Action::init()
     store("Session start");
 }
 
+void captureMixerSession(Session *se, tinyxml2::XMLDocument *doc, std::string node, std::string label)
+{
+    if (se != nullptr) {
+
+        // create node
+        XMLElement *sessionNode = doc->NewElement( node.c_str() );
+        doc->InsertEndChild(sessionNode);
+        // label describes the action
+        sessionNode->SetAttribute("label", label.c_str() );
+        // label describes the action
+        sessionNode->SetAttribute("date", SystemToolkit::date_time_string().c_str() );
+        // view indicates the view when this action occured
+        sessionNode->SetAttribute("view", (int) Mixer::manager().view()->mode());
+
+        // get the thumbnail (requires one opengl update to render)
+        FrameBufferImage *thumbnail = se->renderThumbnail();
+        if (thumbnail) {
+            XMLElement *imageelement = SessionVisitor::ImageToXML(thumbnail, doc);
+            if (imageelement)
+                sessionNode->InsertEndChild(imageelement);
+            delete thumbnail;
+        }
+
+        // save session attributes
+        sessionNode->SetAttribute("activationThreshold", se->activationThreshold());
+
+        // save all sources using source visitor
+        SessionVisitor sv(doc, sessionNode);
+        for (auto iter = se->begin(); iter != se->end(); ++iter, sv.setRoot(sessionNode) )
+            (*iter)->accept(sv);
+
+    }
+}
+
 void Action::store(const std::string &label)
 {
     // ignore if locked or if no label is given
@@ -118,7 +118,7 @@ void Action::store(const std::string &label)
     history_max_step_ = history_step_;
 
     // threaded capturing state of current session
-    std::thread(captureMixerSession, &history_doc_, HISTORY_NODE(history_step_), label).detach();
+    std::thread(captureMixerSession, Mixer::manager().session(), &history_doc_, HISTORY_NODE(history_step_), label).detach();
 
 #ifdef ACTION_DEBUG
     Log::Info("Action stored %d '%s'", history_step_, label.c_str());
@@ -208,31 +208,40 @@ void Action::restore(uint target)
 }
 
 
+void Action::takeSnapshot(Session *se, const std::string &label, bool create_thread)
+{
+    if (se !=nullptr) {
 
-void Action::snapshot(const std::string &label, bool threaded)
+        // create snapshot id
+        u_int64_t id = BaseToolkit::uniqueId();
+
+        // inform session of its new snapshot
+        se->snapshots()->keys_.push_back(id);
+
+        if (create_thread)
+            // threaded capture state of current session
+            std::thread(captureMixerSession, se, se->snapshots()->xmlDoc_, SNAPSHOT_NODE(id), label).detach();
+        else
+            captureMixerSession(se, se->snapshots()->xmlDoc_, SNAPSHOT_NODE(id), label);
+
+#ifdef ACTION_DEBUG
+        Log::Info("Snapshot stored %d '%s'", id, label.c_str());
+#endif
+    }
+}
+
+void Action::snapshot(const std::string &label)
 {
     // ignore if locked
     if (locked_)
         return;
 
+    // ensure label is unique
     std::string snap_label = BaseToolkit::uniqueName(label, labels());
 
-    // create snapshot id
+    // take the snapshot on current session
-    u_int64_t id = BaseToolkit::uniqueId();
+    takeSnapshot(Mixer::manager().session(), snap_label, true);
 
-    // get session to operate on
-    Session *se = Mixer::manager().session();
-    se->snapshots()->keys_.push_back(id);
-
-    if (threaded)
-        // threaded capture state of current session
-        std::thread(captureMixerSession, se->snapshots()->xmlDoc_, SNAPSHOT_NODE(id), snap_label).detach();
-    else
-        captureMixerSession(se->snapshots()->xmlDoc_, SNAPSHOT_NODE(id), snap_label);
-
-#ifdef ACTION_DEBUG
-    Log::Info("Snapshot stored %d '%s'", id, snap_label.c_str());
-#endif
 }
 
 void Action::open(uint64_t snapshotid)
@@ -267,16 +276,18 @@ void Action::replace(uint64_t snapshotid)
 
         // remove previous node
         Session *se = Mixer::manager().session();
+        if (se) {
             se->snapshots()->xmlDoc_->DeleteChild( snapshot_node_ );
 
             // threaded capture state of current session
-        std::thread(captureMixerSession, se->snapshots()->xmlDoc_, SNAPSHOT_NODE(snapshot_id_), label).detach();
+            std::thread(captureMixerSession, se, se->snapshots()->xmlDoc_, SNAPSHOT_NODE(snapshot_id_), label).detach();
 
 #ifdef ACTION_DEBUG
             Log::Info("Snapshot replaced %d '%s'", snapshot_id_, label.c_str());
 #endif
         }
     }
+}
 
 std::list<uint64_t> Action::snapshots() const
 {
@@ -329,8 +340,12 @@ void Action::setLabel (uint64_t snapshotid, const std::string &label)
 {
     open(snapshotid);
 
-    if (snapshot_node_)
+    if (snapshot_node_){
-        snapshot_node_->SetAttribute("label", label.c_str());
+        // ensure unique snapshot label
+        std::string snap_label = BaseToolkit::uniqueName(label, labels());
+        // change attribute
+        snapshot_node_->SetAttribute("label", snap_label.c_str());
+    }
 }
 
 FrameBufferImage *Action::thumbnail(uint64_t snapshotid) const

ActionManager.h

@@ -7,6 +7,7 @@
 
 #include <tinyxml2.h>
 
+class Session;
 class Interpolator;
 class FrameBufferImage;
 
@@ -39,7 +40,8 @@ public:
     FrameBufferImage *thumbnail (uint s) const;
 
     // Snapshots
-    void snapshot (const std::string &label = "", bool threaded = false);
+    static void takeSnapshot (Session *se, const std::string &label, bool create_thread);
+    void snapshot (const std::string &label = "");
     void clearSnapshots ();
     std::list<uint64_t> snapshots () const;
     uint64_t currentSnapshot () const { return snapshot_id_; }

Mixer.cpp

@@ -57,43 +57,15 @@
 #include "Mixer.h"
 
 #define THREADED_LOADING
+std::vector< std::future<std::string> > sessionSavers_;
 std::vector< std::future<Session *> > sessionLoaders_;
 std::vector< std::future<Session *> > sessionImporters_;
 std::vector< SessionSource * > sessionSourceToImport_;
 const std::chrono::milliseconds timeout_ = std::chrono::milliseconds(4);
 
 
-// static multithreaded session saving
-static void saveSession(const std::string& filename, Session *session, bool with_version)
-{
-    // lock access while saving
-    session->lock();
-
-    // capture a snapshot of current version if requested
-    if (with_version)
-        Action::manager().snapshot( SystemToolkit::date_time_string());
-
-    // save file to disk
-    if ( SessionVisitor::saveSession(filename, session) ) {
-        // all ok
-        // set session filename
-        session->setFilename(filename);
-        // cosmetics saved ok
-        Rendering::manager().setMainWindowTitle(SystemToolkit::filename(filename));
-        Settings::application.recentSessions.push(filename);
-        Log::Notify("Session %s saved.", filename.c_str());
-
-    }
-    else {
-        // error
-        Log::Warning("Failed to save Session file %s.", filename.c_str());
-    }
-
-    session->unlock();
-}
-
 Mixer::Mixer() : session_(nullptr), back_session_(nullptr), sessionSwapRequested_(false),
-    current_view_(nullptr), dt_(16.f), dt__(16.f)
+    current_view_(nullptr), busy_(false), dt_(16.f), dt__(16.f)
 {
     // unsused initial empty session
     session_ = new Session;
@@ -129,6 +101,7 @@ void Mixer::update()
     if (!sessionImporters_.empty()) {
         // check status of loader: did it finish ?
         if (sessionImporters_.back().wait_for(timeout_) == std::future_status::ready ) {
+            if (sessionImporters_.back().valid()) {
                 // get the session loaded by this loader
                 merge( sessionImporters_.back().get() );
                 // FIXME: shouldn't we delete the imported session?
@@ -136,20 +109,50 @@ void Mixer::update()
                 sessionImporters_.pop_back();
             }
         }
+    }
 
     // if there is a session loader pending
     if (!sessionLoaders_.empty()) {
         // check status of loader: did it finish ?
         if (sessionLoaders_.back().wait_for(timeout_) == std::future_status::ready ) {
             // get the session loaded by this loader
-            if (sessionLoaders_.back().valid())
+            if (sessionLoaders_.back().valid()) {
+                // get the session loaded by this loader
                 set( sessionLoaders_.back().get() );
                 // done with this session loader
                 sessionLoaders_.pop_back();
             }
+            busy_ = false;
+        }
     }
 #endif
 
+    // if there is a session saving pending
+    if (!sessionSavers_.empty()) {
+        // check status of saver: did it finish ?
+        if (sessionSavers_.back().wait_for(timeout_) == std::future_status::ready ) {
+            std::string filename;
+            // did we get a filename in return?
+            if (sessionSavers_.back().valid()) {
+                filename = sessionSavers_.back().get();
+                // done with this session saver
+                sessionSavers_.pop_back();
+            }
+            if (filename.empty())
+                Log::Warning("Failed to save Session.");
+            // all ok
+            else {
+                // set session filename
+                session_->setFilename(filename);
+                // cosmetics saved ok
+                Rendering::manager().setMainWindowTitle(SystemToolkit::filename(filename));
+                Settings::application.recentSessions.push(filename);
+                Log::Notify("Session %s saved.", filename.c_str());
+            }
+            busy_ = false;
+        }
+    }
+
     // if there is a session source to import
     if (!sessionSourceToImport_.empty()) {
         // get the session source to be imported
@@ -1006,8 +1009,18 @@ void Mixer::saveas(const std::string& filename, bool with_version)
     session_->config(View::LAYER)->copyTransform( layer_.scene.root() );
     session_->config(View::TEXTURE)->copyTransform( appearance_.scene.root() );
 
+    // save only one at a time
+    if (sessionSavers_.empty()) {
+        // will be busy for saving
+        busy_ = true;
+        // prepare argument for saving a version (if requested)
+        std::string versionname;
+        if (with_version)
+            versionname = SystemToolkit::date_time_string();
         // launch a thread to save the session
-    std::thread (saveSession, filename, session_, with_version).detach();
+        // Will be captured in the future in update()
+        sessionSavers_.emplace_back( std::async(std::launch::async, Session::save, filename, session_, versionname) );
+    }
 }
 
 void Mixer::load(const std::string& filename)
@@ -1018,6 +1031,7 @@ void Mixer::load(const std::string& filename)
 #ifdef THREADED_LOADING
     // load only one at a time
     if (sessionLoaders_.empty()) {
+        busy_ = true;
         // Start async thread for loading the session
         // Will be obtained in the future in update()
         sessionLoaders_.emplace_back( std::async(std::launch::async, Session::load, filename, 0) );

Mixer.h

@@ -110,6 +110,7 @@ public:
     void merge  (SessionSource *source);
     void set    (Session *session);
     void setResolution(glm::vec3 res);
+    bool busy   () const { return busy_; }
 
     // operations depending on transition mode
     void close  (bool smooth = false);
@@ -146,6 +147,7 @@ protected:
     TextureView appearance_;
     TransitionView transition_;
 
+    bool busy_;
     float dt_;
     float dt__;
 };

Session.cpp

@@ -26,6 +26,8 @@
 #include "FrameBuffer.h"
 #include "FrameGrabber.h"
 #include "SessionCreator.h"
+#include "SessionVisitor.h"
+#include "ActionManager.h"
 #include "SessionSource.h"
 #include "RenderSource.h"
 #include "MixingGroup.h"
@@ -592,24 +594,6 @@ SourceList Session::playGroup(size_t i) const
     return list;
 }
 
-void Session::lock()
-{
-    access_.lock();
-}
-
-void Session::unlock()
-{
-    access_.unlock();
-}
-
-bool Session::locked()
-{
-    bool l = access_.try_lock();
-    if (l)
-        access_.unlock();
-    return !l;
-}
-
 void Session::validate (SourceList &sources)
 {
     // verify that all sources given are valid in the sesion
@@ -633,3 +617,26 @@ Session *Session::load(const std::string& filename, uint recursion)
     return creator.session();
 }
 
+std::string Session::save(const std::string& filename, Session *session, const std::string& snapshot_name)
+{
+    std::string ret;
+
+    if (session) {
+        // lock access while saving
+        session->access_.lock();
+
+        // capture a snapshot of current version if requested (do not create thread)
+        if (!snapshot_name.empty())
+            Action::takeSnapshot(session, snapshot_name, false );
+
+        // save file to disk
+        if (SessionVisitor::saveSession(filename, session))
+            // return filename string on success
+            ret = filename;
+
+        // unlock access after saving
+        session->access_.unlock();
+    }
+
+    return ret;
+}

Session.h

@@ -39,6 +39,7 @@ public:
     ~Session();
 
     static Session *load(const std::string& filename, uint recursion = 0);
+    static std::string save(const std::string& filename, Session *session, const std::string& snapshot_name = "");
 
     // add given source into the session
     SourceList::iterator addSource (Source *s);
@@ -148,11 +149,6 @@ public:
     void removeFromPlayGroup(size_t i, Source *s);
     std::vector<SourceIdList> getPlayGroups() { return play_groups_; }
 
-    // lock and unlock access (e.g. while saving)
-    void lock ();
-    void unlock ();
-    bool locked ();
-
 protected:
     bool active_;
     float activation_threshold_;

UserInterfaceManager.cpp

@@ -760,8 +760,7 @@ void UserInterface::NewFrame()
     if (DialogToolkit::FileDialog::busy()){
         if (!ImGui::IsPopupOpen("Busy"))
             ImGui::OpenPopup("Busy");
-        if (ImGui::BeginPopupModal("Busy", NULL, ImGuiWindowFlags_AlwaysAutoResize))
+        if (ImGui::BeginPopupModal("Busy", NULL, ImGuiWindowFlags_AlwaysAutoResize)) {
-        {
             ImGui::Text("Close file dialog box to resume.");
             ImGui::EndPopup();
         }
@@ -798,17 +797,13 @@ void UserInterface::NewFrame()
     if (close_and_exit){
         if (!ImGui::IsPopupOpen("Closing"))
             ImGui::OpenPopup("Closing");
-        if (ImGui::BeginPopupModal("Closing", NULL, ImGuiWindowFlags_AlwaysAutoResize))
+        if (ImGui::BeginPopupModal("Closing", NULL, ImGuiWindowFlags_AlwaysAutoResize)) {
-        {
             ImGui::Text("Please wait...");
-            if (FrameGrabbing::manager().busy())
-                ImGui::Text(" - Stop recordings.");
-            if (Settings::application.recentSessions.save_on_exit)
-                ImGui::Text(" - Save session.");
             ImGui::EndPopup();
         }
+
         // exit only after everything is closed
-        if (!FrameGrabbing::manager().busy() && !Mixer::manager().session()->locked()) {
+        if (!FrameGrabbing::manager().busy() && !Mixer::manager().busy()) {
             Rendering::manager().close();
         }
     }