From d7e524e6bd0e72fbacaecdfd64061bdef76fe475 Mon Sep 17 00:00:00 2001
From: philippegorley <philippe.gorley@savoirfairelinux.com>
Date: Thu, 29 Nov 2018 17:06:36 -0500
Subject: [PATCH] recorder: remove toggle and recording path

It should not be up to the recorder to manage this state, it should be
up to the caller.

Removes deprecated setRecordingPath in order to only use setPath.

Change-Id: Ib920c041d61c8fd49607ad15c69801144fc165c8
---
 src/media/media_recorder.cpp | 52 +++++-------------------------------
 src/media/media_recorder.h   |  5 ----
 src/media/recordable.cpp     | 27 +++++++++++++------
 3 files changed, 25 insertions(+), 59 deletions(-)

diff --git a/src/media/media_recorder.cpp b/src/media/media_recorder.cpp
index 54c1550e79..2fcd94d94b 100644
--- a/src/media/media_recorder.cpp
+++ b/src/media/media_recorder.cpp
@@ -70,18 +70,10 @@ MediaRecorder::~MediaRecorder()
 std::string
 MediaRecorder::getPath() const
 {
-    if (path_.empty()) {
-        // FIXME deprecated code, will be removed once all clients transitioned to startRecording(path).
-        if (audioOnly_)
-            return dir_ + filename_ + ".ogg";
-        else
-            return dir_ + filename_ + ".webm";
-    } else {
-        if (audioOnly_)
-            return path_ + ".ogg";
-        else
-            return path_ + ".webm";
-    }
+    if (audioOnly_)
+        return path_ + ".ogg";
+    else
+        return path_ + ".webm";
 }
 
 void
@@ -97,25 +89,11 @@ MediaRecorder::setMetadata(const std::string& title, const std::string& desc)
     description_ = desc;
 }
 
-void
-MediaRecorder::setRecordingPath(const std::string& dir)
-{
-    if (!dir.empty() && fileutils::isDirectory(dir))
-        dir_ = dir;
-    else
-        dir_ = fileutils::get_home_dir();
-    if (dir_.back() != DIR_SEPARATOR_CH)
-        dir_ = dir_ + DIR_SEPARATOR_CH;
-    RING_DBG() << "Recording will be saved in '" << dir_ << "'";
-}
-
 void
 MediaRecorder::setPath(const std::string& path)
 {
-    if (!path.empty()) {
+    if (!path.empty())
         path_ = path;
-    }
-    RING_DBG() << "Recording will be saved as '" << getPath() << "'";
 }
 
 bool
@@ -124,30 +102,12 @@ MediaRecorder::isRecording() const
     return isRecording_;
 }
 
-bool
-MediaRecorder::toggleRecording()
-{
-    if (isRecording_) {
-        stopRecording();
-    } else {
-        startRecording();
-    }
-    return isRecording_;
-}
-
 int
 MediaRecorder::startRecording()
 {
     std::time_t t = std::time(nullptr);
     startTime_ = *std::localtime(&t);
 
-    if (path_.empty()) {
-        // FIXME deprecated code, will be removed once all clients transitioned to startRecording(path).
-        std::stringstream ss;
-        ss << std::put_time(&startTime_, "%Y%m%d-%H%M%S");
-        filename_ = ss.str();
-    }
-
     if (!frames_.empty()) {
         RING_WARN() << "Frame queue not empty at beginning of recording, frames will be lost";
         std::lock_guard<std::mutex> q(qLock_);
@@ -184,7 +144,7 @@ MediaRecorder::update(Observable<std::shared_ptr<AudioFrame>>* ob, const std::sh
     MediaStream ms;
     if (dynamic_cast<AudioReceiveThread*>(ob))
         ms.name = "a:remote";
-    else // if (dynamic_cast<AudioSender*>(ob) || dynamic_cast<AudioInput*>(ob))
+    else // ob is of type AudioInput*
         ms.name = "a:local";
     ms.isVideo = false;
     ms.update(a->pointer());
diff --git a/src/media/media_recorder.h b/src/media/media_recorder.h
index 829b052450..b3fbba002b 100644
--- a/src/media/media_recorder.h
+++ b/src/media/media_recorder.h
@@ -64,13 +64,8 @@ public:
     // default description: "Recorded with Jami https://jami.net"
     void setMetadata(const std::string& title, const std::string& desc);
 
-    [[deprecated("use setPath to set full recording path")]]
-    void setRecordingPath(const std::string& dir);
-
     bool isRecording() const;
 
-    bool toggleRecording();
-
     int startRecording();
 
     void stopRecording();
diff --git a/src/media/recordable.cpp b/src/media/recordable.cpp
index 5fc2569b09..cf4d92ce4c 100644
--- a/src/media/recordable.cpp
+++ b/src/media/recordable.cpp
@@ -19,10 +19,13 @@
  *  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA.
  */
 
-#include "recordable.h"
 #include "audio/ringbufferpool.h"
-#include "manager.h"
+#include "fileutils.h"
 #include "logger.h"
+#include "manager.h"
+#include "recordable.h"
+
+#include <iomanip>
 
 namespace ring {
 
@@ -47,18 +50,26 @@ Recordable::getPath() const
 bool
 Recordable::toggleRecording()
 {
-    std::lock_guard<std::mutex> lk {apiMutex_};
     if (!recorder_) {
         RING_ERR("couldn't toggle recording, non existent recorder");
         return false;
     }
 
     if (!recording_) {
-        // FIXME uses old way of setting recording path in MediaRecorder
-        recorder_->audioOnly(isAudioOnly_);
-        recorder_->setRecordingPath(Manager::instance().audioPreference.getRecordPath());
+        std::time_t t = std::time(nullptr);
+        auto startTime = *std::localtime(&t);
+        std::stringstream ss;
+        auto dir = Manager::instance().audioPreference.getRecordPath();
+        if (dir.empty())
+            dir = fileutils::get_home_dir();
+        ss << dir;
+        if (dir.back() != DIR_SEPARATOR_CH)
+            ss << DIR_SEPARATOR_CH;
+        ss << std::put_time(&startTime, "%Y%m%d-%H%M%S");
+        startRecording(ss.str());
+    } else {
+        stopRecording();
     }
-    recording_ = recorder_->toggleRecording();
     return recording_;
 }
 
@@ -77,8 +88,8 @@ Recordable::startRecording(const std::string& path)
             return false;
         }
 
-        recorder_->setPath(path);
         recorder_->audioOnly(isAudioOnly_);
+        recorder_->setPath(path);
         recorder_->startRecording();
         recording_ = recorder_->isRecording();
     }
-- 
GitLab