recorder: add doc and reorganize code

Change-Id: I68c9b2b2ea3863653f86fcabd17ad4128f0ebcfd

src/media/media_recorder.cpp

@@ -39,6 +39,7 @@
 
 namespace ring {
 
+// Replaces every occurrence of @from with @to in @str
 static std::string
 replaceAll(const std::string& str, const std::string& from, const std::string& to)
 {
@@ -61,10 +62,16 @@ MediaRecorder::MediaRecorder()
 
 MediaRecorder::~MediaRecorder()
 {
-    if (isRecording_)
-        flush();
     if (loop_.isRunning())
         loop_.join();
+    if (isRecording_)
+        flush();
+}
+
+bool
+MediaRecorder::isRecording() const
+{
+    return isRecording_;
 }
 
 std::string
@@ -82,24 +89,17 @@ MediaRecorder::audioOnly(bool audioOnly)
     audioOnly_ = audioOnly;
 }
 
-void
-MediaRecorder::setMetadata(const std::string& title, const std::string& desc)
-{
-    title_ = title;
-    description_ = desc;
-}
-
 void
 MediaRecorder::setPath(const std::string& path)
 {
-    if (!path.empty())
-        path_ = path;
+    path_ = path;
 }
 
-bool
-MediaRecorder::isRecording() const
+void
+MediaRecorder::setMetadata(const std::string& title, const std::string& desc)
 {
-    return isRecording_;
+    title_ = title;
+    description_ = desc;
 }
 
 int
@@ -136,28 +136,13 @@ MediaRecorder::stopRecording()
         flush();
         emitSignal<DRing::CallSignal::RecordPlaybackStopped>(getPath());
     }
-    resetToDefaults();
-}
-
-void
-MediaRecorder::update(Observable<std::shared_ptr<AudioFrame>>* ob, const std::shared_ptr<AudioFrame>& a)
-{
-    std::string name;
-    if (dynamic_cast<AudioReceiveThread*>(ob))
-        name = "a:remote";
-    else // ob is of type AudioInput*
-        name = "a:local";
-    recordData(a->pointer(), streams_[name]);
-}
-
-void MediaRecorder::update(Observable<std::shared_ptr<VideoFrame>>* ob, const std::shared_ptr<VideoFrame>& v)
-{
-    std::string name;
-    if (dynamic_cast<video::VideoReceiveThread*>(ob))
-        name = "v:remote";
-    else // ob is of type VideoInput*
-        name = "v:local";
-    recordData(v->pointer(), streams_[name]);
+    streams_.clear();
+    videoIdx_ = audioIdx_ = -1;
+    isRecording_ = false;
+    audioOnly_ = false;
+    videoFilter_.reset();
+    audioFilter_.reset();
+    encoder_.reset();
 }
 
 int
@@ -181,18 +166,32 @@ MediaRecorder::addStream(const MediaStream& ms)
     }
 }
 
+void
+MediaRecorder::update(Observable<std::shared_ptr<AudioFrame>>* ob, const std::shared_ptr<AudioFrame>& a)
+{
+    std::string name;
+    if (dynamic_cast<AudioReceiveThread*>(ob))
+        name = "a:remote";
+    else // ob is of type AudioInput*
+        name = "a:local";
+    recordData(a->pointer(), streams_[name]);
+}
+
+void MediaRecorder::update(Observable<std::shared_ptr<VideoFrame>>* ob, const std::shared_ptr<VideoFrame>& v)
+{
+    std::string name;
+    if (dynamic_cast<video::VideoReceiveThread*>(ob))
+        name = "v:remote";
+    else // ob is of type VideoInput*
+        name = "v:local";
+    recordData(v->pointer(), streams_[name]);
+}
+
 int
 MediaRecorder::recordData(AVFrame* frame, const MediaStream& ms)
 {
     // recorder may be recording, but not ready for the first frames
-    if (!isRecording_)
-        return 0;
-
-    if (!isReady_ && streams_.find(ms.name) == streams_.end())
-        if (addStream(ms) < 0)
-            return -1;
-
-    if (!isReady_ || !loop_.isRunning()) // check again in case initRecord was called
+    if (!isRecording_ || !loop_.isRunning())
         return 0;
 
     const auto& params = streams_.at(ms.name);
@@ -288,29 +287,18 @@ MediaRecorder::initRecord()
         }
     }
 
-    // ready to start recording if audio stream index and video stream index are valid
-    bool audioIsReady = hasAudio_ && audioIdx_ >= 0;
-    bool videoIsReady = !audioOnly_ && hasVideo_ && videoIdx_ >= 0;
-    isReady_ = audioIsReady && videoIsReady;
-
-    if (isReady_) {
-        if (!loop_.isRunning())
-            loop_.start();
-
+    try {
         std::unique_ptr<MediaIOHandle> ioHandle;
-        try {
-            encoder_->setIOContext(ioHandle);
-            encoder_->startIO();
-        } catch (const MediaEncoderException& e) {
-            RING_ERR() << "Could not start recorder: " << e.what();
-            return -1;
-        }
-        RING_DBG() << "Recording initialized";
-        return 0;
-    } else {
-        RING_ERR() << "Failed to initialize recorder";
+        encoder_->setIOContext(ioHandle);
+        encoder_->startIO();
+    } catch (const MediaEncoderException& e) {
+        RING_ERR() << "Could not start recorder: " << e.what();
         return -1;
     }
+
+    RING_DBG() << "Recording initialized";
+    loop_.start();
+    return 0;
 }
 
 MediaStream
@@ -508,19 +496,6 @@ MediaRecorder::flush()
     return 0;
 }
 
-void
-MediaRecorder::resetToDefaults()
-{
-    streams_.clear();
-    videoIdx_ = audioIdx_ = -1;
-    isRecording_ = false;
-    isReady_ = false;
-    audioOnly_ = false;
-    videoFilter_.reset();
-    audioFilter_.reset();
-    encoder_.reset();
-}
-
 void
 MediaRecorder::process()
 {

src/media/media_recorder.h

@@ -53,69 +53,92 @@ public:
     MediaRecorder();
     ~MediaRecorder();
 
-    std::string getPath() const;
+    /**
+     * Gets whether or not the recorder is active.
+     */
+    bool isRecording() const;
 
-    void setPath(const std::string& path);
+    /**
+     * Get file path of file to be recorded. Same path as sent to @setPath, but with
+     * file extension appended.
+     *
+     * NOTE @audioOnly must be called to have the right extension.
+     */
+    std::string getPath() const;
 
+    /**
+     * Sets whether or not output file will have audio. Determines the extension of the
+     * output file (.ogg or .webm).
+     */
     void audioOnly(bool audioOnly);
 
-    // replaces %TIMESTAMP with time at start of recording
-    // default title: "Conversation at %Y-%m-%d %H:%M:%S"
-    // default description: "Recorded with Jami https://jami.net"
+    /**
+     * Sets output file path.
+     *
+     * NOTE An empty path will put the output file in the working directory.
+     */
+    void setPath(const std::string& path);
+
+    /**
+     * Sets title and description metadata for the file. Uses default if either is empty.
+     * Default title is "Conversation at %Y-%m-%d %H:%M:%S".
+     * Default description is "Recorded with Jami https://jami.net".
+     *
+     * NOTE replaces %TIMESTAMP with time at start of recording
+     */
     void setMetadata(const std::string& title, const std::string& desc);
 
-    bool isRecording() const;
+    /**
+     */
+    int addStream(const MediaStream& ms);
 
+    /**
+     * Starts the record. Streams must have been added using Observable::attach and
+     * @addStream.
+     */
     int startRecording();
 
+    /**
+     * Stops the record. Streams must be removed using Observable::detach afterwards.
+     */
     void stopRecording();
 
-    int addStream(const MediaStream& ms);
-
-    /* Observer methods*/
+    /**
+     * Updates the recorder with an audio or video frame.
+     */
     void update(Observable<std::shared_ptr<AudioFrame>>* ob, const std::shared_ptr<AudioFrame>& a) override;
-
     void update(Observable<std::shared_ptr<VideoFrame>>* ob, const std::shared_ptr<VideoFrame>& v) override;
 
 private:
     NON_COPYABLE(MediaRecorder);
 
     int recordData(AVFrame* frame, const MediaStream& ms);
+    int flush();
 
     int initRecord();
     MediaStream setupVideoOutput();
     std::string buildVideoFilter(const std::vector<MediaStream>& peers, const MediaStream& local) const;
     MediaStream setupAudioOutput();
     std::string buildAudioFilter(const std::vector<MediaStream>& peers, const MediaStream& local) const;
-    void emptyFilterGraph();
-    int sendToEncoder(AVFrame* frame, int streamIdx);
-    int flush();
-    void resetToDefaults(); // clear saved data for next recording
-
-    std::unique_ptr<MediaEncoder> encoder_;
-    std::unique_ptr<MediaFilter> videoFilter_;
-    std::unique_ptr<MediaFilter> audioFilter_;
 
     std::mutex mutex_; // protect against concurrent file writes
 
     std::map<std::string, const MediaStream> streams_;
 
+    std::string path_;
     std::tm startTime_;
     std::string title_;
     std::string description_;
 
-    std::string path_;
-
-    // NOTE do not use dir_ or filename_, use path_ instead
-    std::string dir_;
-    std::string filename_;
+    std::unique_ptr<MediaEncoder> encoder_;
+    std::unique_ptr<MediaFilter> videoFilter_;
+    std::unique_ptr<MediaFilter> audioFilter_;
 
     bool hasAudio_ {false};
     bool hasVideo_ {false};
     int videoIdx_ = -1;
     int audioIdx_ = -1;
     bool isRecording_ = false;
-    bool isReady_ = false;
     bool audioOnly_ = false;
 
     struct RecordFrame {
@@ -131,6 +154,8 @@ private:
     };
     InterruptedThreadLoop loop_;
     void process();
+    void emptyFilterGraph();
+    int sendToEncoder(AVFrame* frame, int streamIdx);
     std::mutex qLock_;
     std::deque<RecordFrame> frames_;
 };