From 9fb288936a44fc89e9a7cbddd13219e5a5c6a95b Mon Sep 17 00:00:00 2001
From: Pierre Nicolas <pierre.nicolas@savoirfairelinux.com>
Date: Mon, 6 Jan 2025 15:23:23 -0500
Subject: [PATCH] ringbufferpool: refactor variable names + add documentation

Improve code clarity only. No logic changes.

Change-Id: I348185c79bf48248f1f407627d12fc50cbe3e1d4
---
 src/conference.cpp                 |  8 +--
 src/manager.cpp                    |  2 +-
 src/media/audio/ringbufferpool.cpp | 83 +++++++++++++-----------------
 src/media/audio/ringbufferpool.h   | 67 +++++++++++++++++-------
 src/sip/sipcall.cpp                |  4 +-
 5 files changed, 91 insertions(+), 73 deletions(-)

diff --git a/src/conference.cpp b/src/conference.cpp
index 911ba03fc..56acd18a1 100644
--- a/src/conference.cpp
+++ b/src/conference.cpp
@@ -1781,7 +1781,7 @@ Conference::bindHostAudio()
                             if (isParticipantMuted)
                                 rbPool.bindHalfDuplexOut(id, RingBufferPool::DEFAULT_ID);
                             else
-                                rbPool.bindRingbuffers(id, RingBufferPool::DEFAULT_ID);
+                                rbPool.bindRingBuffers(id, RingBufferPool::DEFAULT_ID);
                         } else {
                             auto buffer = source.sourceUri_;
                             static const std::string& sep = libjami::Media::VideoProtocolPrefix::SEPARATOR;
@@ -1789,7 +1789,7 @@ Conference::bindHostAudio()
                             if (pos != std::string::npos)
                                 buffer = source.sourceUri_.substr(pos + sep.size());
 
-                            rbPool.bindRingbuffers(id, buffer);
+                            rbPool.bindRingBuffers(id, buffer);
                         }
                     }
                 }
@@ -1846,7 +1846,7 @@ Conference::bindSubCallAudio(const std::string& callId)
                         if (isMuted(other))
                             rbPool.bindHalfDuplexOut(otherStream.first, stream.first);
                         else
-                            rbPool.bindRingbuffers(stream.first, otherStream.first);
+                            rbPool.bindRingBuffers(stream.first, otherStream.first);
 
                         rbPool.flush(otherStream.first);
                     }
@@ -1859,7 +1859,7 @@ Conference::bindSubCallAudio(const std::string& callId)
                 bool isHostMuted = isMuted("host"sv);
                 if (isMediaSourceMuted(MediaType::MEDIA_AUDIO) or isHostMuted)
                     rbPool.bindHalfDuplexOut(RingBufferPool::DEFAULT_ID, stream.first);
-                else rbPool.bindRingbuffers(stream.first, RingBufferPool::DEFAULT_ID);
+                else rbPool.bindRingBuffers(stream.first, RingBufferPool::DEFAULT_ID);
                 rbPool.flush(RingBufferPool::DEFAULT_ID);
             }
         }
diff --git a/src/manager.cpp b/src/manager.cpp
index df109330f..d2634af39 100644
--- a/src/manager.cpp
+++ b/src/manager.cpp
@@ -1704,7 +1704,7 @@ Manager::addAudio(Call& call)
     auto medias = call.getAudioStreams();
     for (const auto& media : medias) {
         JAMI_DEBUG("[call:{}] Attach audio", media.first);
-        getRingBufferPool().bindRingbuffers(media.first, RingBufferPool::DEFAULT_ID);
+        getRingBufferPool().bindRingBuffers(media.first, RingBufferPool::DEFAULT_ID);
     }
     auto oldGuard = std::move(call.audioGuard);
     call.audioGuard = startAudioStream(AudioDeviceType::PLAYBACK);
diff --git a/src/media/audio/ringbufferpool.cpp b/src/media/audio/ringbufferpool.cpp
index 9ce8f847d..1571f10ba 100644
--- a/src/media/audio/ringbufferpool.cpp
+++ b/src/media/audio/ringbufferpool.cpp
@@ -136,46 +136,40 @@ RingBufferPool::removeReadBindings(const std::string& ringbufferId)
         JAMI_ERROR("Ringbuffer {} does not exist!", ringbufferId);
 }
 
-/**
- * Make given ringbuffer a reader of given ring buffer
- */
 void
-RingBufferPool::addReaderToRingBuffer(const std::shared_ptr<RingBuffer>& rbuf,
-                                      const std::string& ringbufferId)
-{
-    if (ringbufferId != DEFAULT_ID and rbuf->getId() == ringbufferId)
+RingBufferPool::addReaderToRingBuffer(const std::shared_ptr<RingBuffer> &sourceBuffer,
+                                      const std::string &readerBufferId) {
+    if (readerBufferId != DEFAULT_ID and sourceBuffer->getId() == readerBufferId)
         JAMI_WARNING("RingBuffer has a readoffset on itself");
 
-    rbuf->createReadOffset(ringbufferId);
-    readBindingsMap_[ringbufferId].insert(rbuf); // bindings list created if not existing
-    JAMI_DEBUG("Bind rbuf '{}' to ringbuffer '{}'", rbuf->getId(), ringbufferId);
+    sourceBuffer->createReadOffset(readerBufferId);
+    readBindingsMap_[readerBufferId].insert(sourceBuffer);
 }
 
 void
-RingBufferPool::removeReaderFromRingBuffer(const std::shared_ptr<RingBuffer>& rbuf,
-                                           const std::string& ringbufferId)
-{
-    if (auto bindings = getReadBindings(ringbufferId)) {
-        bindings->erase(rbuf);
+RingBufferPool::removeReaderFromRingBuffer(const std::shared_ptr<RingBuffer> &sourceBuffer,
+                                           const std::string &readerBufferId) {
+    if (auto bindings = getReadBindings(readerBufferId)) {
+        bindings->erase(sourceBuffer);
         if (bindings->empty())
-            removeReadBindings(ringbufferId);
+            removeReadBindings(readerBufferId);
     }
 
-    rbuf->removeReadOffset(ringbufferId);
+    sourceBuffer->removeReadOffset(readerBufferId);
 }
 
 void
-RingBufferPool::bindRingbuffers(const std::string& ringbufferId1, const std::string& ringbufferId2)
-{
+RingBufferPool::bindRingBuffers(const std::string &ringbufferId1,
+                                const std::string &ringbufferId2) {
     JAMI_LOG("Bind ringbuffer {} to ringbuffer {}", ringbufferId1, ringbufferId2);
 
-    const auto& rb1 = getRingBuffer(ringbufferId1);
+    const auto &rb1 = getRingBuffer(ringbufferId1);
     if (not rb1) {
         JAMI_ERROR("No ringbuffer associated with id '{}'", ringbufferId1);
         return;
     }
 
-    const auto& rb2 = getRingBuffer(ringbufferId2);
+    const auto &rb2 = getRingBuffer(ringbufferId2);
     if (not rb2) {
         JAMI_ERROR("No ringbuffer associated to id '{}'", ringbufferId2);
         return;
@@ -188,29 +182,30 @@ RingBufferPool::bindRingbuffers(const std::string& ringbufferId1, const std::str
 }
 
 void
-RingBufferPool::bindHalfDuplexOut(const std::string& processId, const std::string& ringbufferId)
-{
+RingBufferPool::bindHalfDuplexOut(const std::string &readerBufferId,
+                                  const std::string &sourceBufferId) {
     /* This method is used only for active ringbuffers, if this ringbuffer does not exist,
      * do nothing */
-    if (const auto& rb = getRingBuffer(ringbufferId)) {
+    if (const auto &rb = getRingBuffer(sourceBufferId)) {
         std::lock_guard lk(stateLock_);
 
-        addReaderToRingBuffer(rb, processId);
+        // p1 est le binding de p2 (p2 lit le stream de p1)
+        addReaderToRingBuffer(rb, readerBufferId);
     }
 }
 
 void
-RingBufferPool::unbindRingbuffers(const std::string& ringbufferId1, const std::string& ringbufferId2)
-{
+RingBufferPool::unbindRingBuffers(const std::string &ringbufferId1,
+                                  const std::string &ringbufferId2) {
     JAMI_LOG("Unbind ringbuffers {} and {}", ringbufferId1, ringbufferId2);
 
-    const auto& rb1 = getRingBuffer(ringbufferId1);
+    const auto &rb1 = getRingBuffer(ringbufferId1);
     if (not rb1) {
         JAMI_ERROR("No ringbuffer associated to id '{}'", ringbufferId1);
         return;
     }
 
-    const auto& rb2 = getRingBuffer(ringbufferId2);
+    const auto &rb2 = getRingBuffer(ringbufferId2);
     if (not rb2) {
         JAMI_ERROR("No ringbuffer associated to id '{}'", ringbufferId2);
         return;
@@ -223,40 +218,35 @@ RingBufferPool::unbindRingbuffers(const std::string& ringbufferId1, const std::s
 }
 
 void
-RingBufferPool::unBindHalfDuplexOut(const std::string& process_id, const std::string& ringbufferId)
-{
+RingBufferPool::unBindHalfDuplexOut(const std::string &readerBufferId,
+                                    const std::string &sourceBufferId) {
     std::lock_guard lk(stateLock_);
 
-    if (const auto& rb = getRingBuffer(ringbufferId))
-        removeReaderFromRingBuffer(rb, process_id);
+    if (const auto &rb = getRingBuffer(sourceBufferId))
+        removeReaderFromRingBuffer(rb, readerBufferId);
 }
 
 void
-RingBufferPool::unBindAllHalfDuplexOut(const std::string& ringbufferId)
-{
-    const auto& rb = getRingBuffer(ringbufferId);
+RingBufferPool::unBindAllHalfDuplexOut(const std::string &ringbufferId) {
+    const auto &rb = getRingBuffer(ringbufferId);
     if (not rb) {
         JAMI_ERROR("No ringbuffer associated to id '{}'", ringbufferId);
         return;
     }
-
     std::lock_guard lk(stateLock_);
-
     auto bindings = getReadBindings(ringbufferId);
     if (not bindings)
         return;
-
     const auto bindings_copy = *bindings; // temporary copy
-    for (const auto& rbuf : bindings_copy) {
+    for (const auto &rbuf: bindings_copy) {
         removeReaderFromRingBuffer(rb, rbuf->getId());
     }
 }
 
 void
-RingBufferPool::unBindAllHalfDuplexIn(const std::string &audioToDelete) {
-
+RingBufferPool::unBindAllHalfDuplexIn(const std::string &sourceBufferId) {
     std::lock_guard lk(stateLock_);
-    const std::shared_ptr<RingBuffer> &ringBuffer = getRingBuffer(audioToDelete);
+    const std::shared_ptr<RingBuffer> &ringBuffer = getRingBuffer(sourceBufferId);
     const std::vector<std::string> &subscribers = ringBuffer->getSubscribers();
     for (const auto &subscriber: subscribers) {
         removeReaderFromRingBuffer(ringBuffer, subscriber);
@@ -264,11 +254,10 @@ RingBufferPool::unBindAllHalfDuplexIn(const std::string &audioToDelete) {
 }
 
 void
-RingBufferPool::unBindAll(const std::string& ringbufferId)
-{
+RingBufferPool::unBindAll(const std::string &ringbufferId) {
     JAMI_LOG("Unbind ringbuffer {} from all bound ringbuffers", ringbufferId);
 
-    const auto& rb = getRingBuffer(ringbufferId);
+    const auto &rb = getRingBuffer(ringbufferId);
     if (not rb) {
         JAMI_ERROR("No ringbuffer associated to id '{}'", ringbufferId);
         return;
@@ -281,7 +270,7 @@ RingBufferPool::unBindAll(const std::string& ringbufferId)
         return;
 
     const auto bindings_copy = *bindings; // temporary copy
-    for (const auto& rbuf : bindings_copy) {
+    for (const auto &rbuf: bindings_copy) {
         removeReaderFromRingBuffer(rbuf, ringbufferId);
         removeReaderFromRingBuffer(rb, rbuf->getId());
     }
diff --git a/src/media/audio/ringbufferpool.h b/src/media/audio/ringbufferpool.h
index ef3aed340..74d75da5b 100644
--- a/src/media/audio/ringbufferpool.h
+++ b/src/media/audio/ringbufferpool.h
@@ -47,29 +47,45 @@ public:
     void setInternalAudioFormat(AudioFormat format);
 
     /**
-     * Bind together two audio streams
-     */
-    void bindRingbuffers(const std::string& ringbufferId1, const std::string& ringbufferId2);
+    * Bind two RingBuffer together (full duplex).
+    * @param ringbufferId1
+    * @param ringbufferId2
+    */
+    void bindRingBuffers(const std::string &ringbufferId1, const std::string &ringbufferId2);
 
     /**
-     * Add a new ringbufferId to unidirectional outgoing stream
-     * \param ringbufferId New ringbufferId to be added for this stream
-     * \param processId Process that require this stream
-     */
-    void bindHalfDuplexOut(const std::string& processId, const std::string& ringbufferId);
+    * Unbind two RingBuffer (full duplex).
+    */
+    void unbindRingBuffers(const std::string &ringbufferId1, const std::string &ringbufferId2);
 
     /**
-     * Unbind two ringbuffers
-     */
-    void unbindRingbuffers(const std::string& ringbufferId1, const std::string& ringbufferId2);
+    * Attaches a reader the specified source.
+    * @param readerBufferId The ID of the RingBuffer that will act as the reader of the
+    * sourceBuffer.
+    * @param sourceBufferId The iID of the RingBuffer that will be the source (to be read from).
+    */
+    void bindHalfDuplexOut(const std::string &readerBufferId, const std::string &sourceBufferId);
 
     /**
-     * Unbind a unidirectional stream
-     */
-    void unBindHalfDuplexOut(const std::string& process_id, const std::string& ringbufferId);
+    * Detaches a reader from the specified source.
+    * @param readerBufferId The ID of the RingBuffer that acts as the reader to be detached from the
+    * sourceBuffer.
+    * @param sourceBufferId The RingBuffer that serves as the source (being read from).
+    */
+    void unBindHalfDuplexOut(const std::string &readerBufferId, const std::string &sourceBufferId);
+
+    /**
+    * Detaches a reader from all his sources.
+    * @param readerBufferId The ID of the RingBuffer that acts as the reader to be detached from the
+    * sources.
+    */
+    void unBindAllHalfDuplexOut(const std::string &ringbufferId);
 
-    void unBindAllHalfDuplexOut(const std::string& ringbufferId);
-    void unBindAllHalfDuplexIn(const std::string& ringbufferId);
+    /**
+     * Detaches a source from all its readers.
+     * @param sourceBufferId The ID of the RingBuffer that serves as the source (being read from).
+     */
+    void unBindAllHalfDuplexIn(const std::string &sourceBufferId);
 
     void unBindAll(const std::string& ringbufferId);
 
@@ -123,10 +139,23 @@ private:
 
     void removeReadBindings(const std::string& ringbufferId);
 
-    void addReaderToRingBuffer(const std::shared_ptr<RingBuffer>& rbuf, const std::string& ringbufferId);
+    /**
+    * Attaches a reader to the specified source.
+    * @param sourceBuffer The RingBuffer that will be the source (to be read from).
+    * @param readerBufferId The ID of the RingBuffer that will act as the reader of the
+    * sourceBuffer.
+    */
+    void addReaderToRingBuffer(const std::shared_ptr<RingBuffer> &sourceBuffer,
+                               const std::string &readerBufferId);
 
-    void removeReaderFromRingBuffer(const std::shared_ptr<RingBuffer>& rbuf,
-                                    const std::string& ringbufferId);
+    /**
+    * Detaches a reader from the specified source.
+    * @param sourceBuffer The RingBuffer that serves as the source (being read from).
+    * @param readerBufferId The ID of the RingBuffer that acts as the reader to be detached from the
+    * sourceBuffer.
+    */
+    void removeReaderFromRingBuffer(const std::shared_ptr<RingBuffer> &sourceBuffer,
+                                    const std::string &readerBufferId);
 
     // A cache of created RingBuffers listed by IDs.
     std::map<std::string, std::weak_ptr<RingBuffer>> ringBufferMap_ {};
diff --git a/src/sip/sipcall.cpp b/src/sip/sipcall.cpp
index 955b0a6da..0a20448de 100644
--- a/src/sip/sipcall.cpp
+++ b/src/sip/sipcall.cpp
@@ -3166,7 +3166,7 @@ SIPCall::enterConference(std::shared_ptr<Conference> conference)
         auto& rbPool = Manager::instance().getRingBufferPool();
         auto medias = getAudioStreams();
         for (const auto& media : medias) {
-            rbPool.unbindRingbuffers(media.first, RingBufferPool::DEFAULT_ID);
+            rbPool.unbindRingBuffers(media.first, RingBufferPool::DEFAULT_ID);
         }
         rbPool.flush(RingBufferPool::DEFAULT_ID);
     }
@@ -3194,7 +3194,7 @@ SIPCall::exitConference()
         auto medias = getAudioStreams();
         for (const auto& media : medias) {
             if (!media.second) {
-                rbPool.bindRingbuffers(media.first, RingBufferPool::DEFAULT_ID);
+                rbPool.bindRingBuffers(media.first, RingBufferPool::DEFAULT_ID);
             }
         }
         rbPool.flush(RingBufferPool::DEFAULT_ID);
-- 
GitLab