From 4e5b2050e6f8d004eb01afe55a3aa12f640357a2 Mon Sep 17 00:00:00 2001
From: Hugo Lefeuvre <hugo.lefeuvre@savoirfairelinux.com>
Date: Mon, 18 Jun 2018 10:12:01 -0400
Subject: [PATCH] recordable: add startRecording method

This commit adds a new startRecording method to the Recordable class.
This method can be used to start a recording and takes advantage of
MediaRecorder's new API (no call to setRecordingPath). Also,
startRecording is meant to be passed the path directly instead of
using initRecFilename which is removed in this commit.

In addition to these changes we modify the Recordable class to operate
on a single recorder_ object created at construction time instead of
creating/destroying it in toggleRecording and stopRecording.

Change-Id: Icf170e2a0ba5bbbb34afa37a54cf5edc76c8338d
Reviewed-by: Philippe Gorley <philippe.gorley@savoirfairelinux.com>
---
 src/conference.cpp          |  1 -
 src/media/recordable.cpp    | 57 +++++++++++++++++++++++++++++--------
 src/media/recordable.h      |  4 +--
 src/ringdht/ringaccount.cpp |  3 --
 src/sip/sipaccount.cpp      |  1 -
 src/sip/sipvoiplink.cpp     |  1 -
 6 files changed, 47 insertions(+), 20 deletions(-)

diff --git a/src/conference.cpp b/src/conference.cpp
index b45d55c519..f5d230ae3a 100644
--- a/src/conference.cpp
+++ b/src/conference.cpp
@@ -48,7 +48,6 @@ Conference::Conference()
     , videoMixer_(nullptr)
 #endif
 {
-    Recordable::initRecFilename(id_);
 }
 
 Conference::~Conference()
diff --git a/src/media/recordable.cpp b/src/media/recordable.cpp
index e0cb3dc401..7bb8d9f26a 100644
--- a/src/media/recordable.cpp
+++ b/src/media/recordable.cpp
@@ -28,15 +28,14 @@
 namespace ring {
 
 Recordable::Recordable()
-{}
+{
+    recorder_.reset();
+    recorder_ = std::make_shared<MediaRecorder>();
+}
 
 Recordable::~Recordable()
 {}
 
-void
-Recordable::initRecFilename(const std::string& /*filename*/)
-{}
-
 std::string
 Recordable::getPath() const
 {
@@ -50,9 +49,13 @@ bool
 Recordable::toggleRecording()
 {
     std::lock_guard<std::mutex> lk {apiMutex_};
-    if (!recording_ || !recorder_) {
-        recorder_.reset();
-        recorder_ = std::make_shared<MediaRecorder>();
+    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());
     }
@@ -60,16 +63,46 @@ Recordable::toggleRecording()
     return recording_;
 }
 
+bool
+Recordable::startRecording(const std::string& path)
+{
+    std::lock_guard<std::mutex> lk {apiMutex_};
+    if (!recorder_) {
+        RING_ERR("couldn't start recording, non existent recorder");
+        return false;
+    }
+
+    if (!recording_) {
+        if (path.empty()) {
+            RING_ERR("couldn't start recording, path is empty");
+            return false;
+        }
+
+        recorder_->setPath(path);
+        recorder_->audioOnly(isAudioOnly_);
+        recorder_->startRecording();
+        recording_ = recorder_->isRecording();
+    }
+
+    return recording_;
+}
+
 void
 Recordable::stopRecording()
 {
     std::lock_guard<std::mutex> lk {apiMutex_};
-    if (not recording_)
+    if (!recorder_) {
+        RING_WARN("couldn't stop recording, non existent recorder");
         return;
-    if (recorder_)
-        recorder_->stopRecording();
+    }
+
+    if (not recording_) {
+        RING_WARN("couldn't stop non-running recording");
+        return;
+    }
+
+    recorder_->stopRecording();
     recording_ = false;
-    recorder_.reset();
 }
 
 bool
diff --git a/src/media/recordable.h b/src/media/recordable.h
index e45cab80ba..1f9cc2e796 100644
--- a/src/media/recordable.h
+++ b/src/media/recordable.h
@@ -56,9 +56,9 @@ public:
     void stopRecording();
 
     /**
-     * Init the recording file name according to path specified in configuration
+     * Start recording
      */
-    void initRecFilename(const std::string& filename);
+    bool startRecording(const std::string& path);
 
     /**
      * Return the file path for this recording
diff --git a/src/ringdht/ringaccount.cpp b/src/ringdht/ringaccount.cpp
index cb27592580..0898495a67 100644
--- a/src/ringdht/ringaccount.cpp
+++ b/src/ringdht/ringaccount.cpp
@@ -347,7 +347,6 @@ RingAccount::newOutgoingCall(const std::string& toUrl,
 
     call->setIPToIP(true);
     call->setSecure(isTlsEnabled());
-    call->initRecFilename(toUrl);
 
     try {
         const std::string toUri = parseRingUri(suffix);
@@ -519,7 +518,6 @@ RingAccount::onConnectedOutgoingCall(SIPCall& call, const std::string& to_id, Ip
     call.initIceMediaTransport(true);
     call.setIPToIP(true);
     call.setPeerNumber(getToUri(to_id+"@"+target.toString(true).c_str()));
-    call.initRecFilename(to_id);
 
     const auto localAddress = ip_utils::getInterfaceAddr(getLocalInterface());
 
@@ -2496,7 +2494,6 @@ RingAccount::replyToIncomingIceMsg(const std::shared_ptr<SIPCall>& call,
     }
 
     call->setPeerNumber(from);
-    call->initRecFilename(from);
 
     // Let the call handled by the PendingCall handler loop
     {
diff --git a/src/sip/sipaccount.cpp b/src/sip/sipaccount.cpp
index 217e714252..710087defd 100644
--- a/src/sip/sipaccount.cpp
+++ b/src/sip/sipaccount.cpp
@@ -215,7 +215,6 @@ SIPAccount::newOutgoingCall(const std::string& toUrl,
     call->setIPToIP(isIP2IP());
     call->setPeerNumber(toUri);
     call->setPeerUri(toUri);
-    call->initRecFilename(to);
 
     const auto localAddress = ip_utils::getInterfaceAddr(getLocalInterface(), family);
 
diff --git a/src/sip/sipvoiplink.cpp b/src/sip/sipvoiplink.cpp
index 1a04416e8a..aed5c132db 100644
--- a/src/sip/sipvoiplink.cpp
+++ b/src/sip/sipvoiplink.cpp
@@ -335,7 +335,6 @@ transaction_request_cb(pjsip_rx_data *rdata)
     call->setPeerUri(account->getToUri(peerNumber));
     call->setPeerDisplayName(peerDisplayName);
     call->setState(Call::ConnectionState::PROGRESSING);
-    call->initRecFilename(peerNumber);
     call->getSDP().setPublishedIP(addrSdp);
 
     if (account->isStunEnabled())
-- 
GitLab