From 6e1f4e15dcfee240eb8e1c5f10f578b1d3cf3bc5 Mon Sep 17 00:00:00 2001
From: Mohamed Chibani <mohamed.chibani@savoirfairelinux.com>
Date: Wed, 19 Aug 2020 11:44:42 -0400
Subject: [PATCH] rendez-vous: fix participant bindings

Bind local participant to the other participants only if the local
participant is attached to the conference.

Gitlab: #279
Change-Id: I2e97affb1444302b51729e73bd9d9d399338d992
---
 src/conference.cpp                 | 25 +++++++++++++++++++++----
 src/manager.cpp                    | 26 ++++++++++++++++++++++----
 src/media/audio/ringbuffer.cpp     |  9 ++++++++-
 src/media/audio/ringbuffer.h       |  5 +++++
 src/media/audio/ringbufferpool.cpp |  6 ++++++
 5 files changed, 62 insertions(+), 9 deletions(-)

diff --git a/src/conference.cpp b/src/conference.cpp
index d39837b9d3..5cb75ecd9f 100644
--- a/src/conference.cpp
+++ b/src/conference.cpp
@@ -45,6 +45,8 @@ Conference::Conference()
     , mediaInput_(Manager::instance().getVideoManager().videoDeviceMonitor.getMRLForDefaultDevice())
 #endif
 {
+    JAMI_INFO("Create new conference %s", id_.c_str());
+
 #ifdef ENABLE_VIDEO
     getVideoMixer()->setOnSourcesUpdated([this](const std::vector<video::SourceInfo>&& infos) {
         runOnMainThread([w = weak(), infos = std::move(infos)] {
@@ -93,6 +95,8 @@ Conference::Conference()
 
 Conference::~Conference()
 {
+    JAMI_INFO("Destroy conference %s", id_.c_str());
+
 #ifdef ENABLE_VIDEO
     for (const auto& participant_id : participants_) {
         if (auto call = Manager::instance().callFactory.getCall<SIPCall>(participant_id)) {
@@ -222,6 +226,8 @@ Conference::remove(const std::string& participant_id)
 void
 Conference::attach()
 {
+    JAMI_INFO("Attach local participant to conference %s", id_.c_str());
+
     if (getState() == State::ACTIVE_DETACHED) {
         auto& rbPool = Manager::instance().getRingBufferPool();
         for (const auto& participant : getParticipantList()) {
@@ -238,13 +244,16 @@ Conference::attach()
 #endif
         setState(State::ACTIVE_ATTACHED);
     } else {
-        JAMI_WARN("Invalid conference state in attach participant");
+        JAMI_WARN("Invalid conference state in attach participant: current \"%s\" - expected \"%s\"",
+            getStateStr(), "ACTIVE_DETACHED");
     }
 }
 
 void
 Conference::detach()
 {
+    JAMI_INFO("Detach local participant from conference %s", id_.c_str());
+
     if (getState() == State::ACTIVE_ATTACHED) {
         Manager::instance().getRingBufferPool().unBindAll(RingBufferPool::DEFAULT_ID);
 #ifdef ENABLE_VIDEO
@@ -254,13 +263,17 @@ Conference::detach()
 #endif
         setState(State::ACTIVE_DETACHED);
     } else {
-        JAMI_WARN("Invalid conference state in detach participant");
+        JAMI_WARN("Invalid conference state in detach participant: current \"%s\" - expected \"%s\"",
+            getStateStr(), "ACTIVE_ATTACHED");
     }
 }
 
 void
 Conference::bindParticipant(const std::string& participant_id)
 {
+    JAMI_INFO("Bind participant %s to conference %s",
+        participant_id.c_str(), id_.c_str());
+
     auto& rbPool = Manager::instance().getRingBufferPool();
 
     for (const auto& item : participants_) {
@@ -269,8 +282,12 @@ Conference::bindParticipant(const std::string& participant_id)
         rbPool.flush(item);
     }
 
-    rbPool.bindCallID(participant_id, RingBufferPool::DEFAULT_ID);
-    rbPool.flush(RingBufferPool::DEFAULT_ID);
+    // Bind local participant to other participants only if the
+    // local is attached to the conference.
+    if (getState() == State::ACTIVE_ATTACHED) {
+        rbPool.bindCallID(participant_id, RingBufferPool::DEFAULT_ID);
+        rbPool.flush(RingBufferPool::DEFAULT_ID);
+    }
 }
 
 const ParticipantSet&
diff --git a/src/manager.cpp b/src/manager.cpp
index fc07166e97..0ea8a7bb20 100644
--- a/src/manager.cpp
+++ b/src/manager.cpp
@@ -977,6 +977,8 @@ Manager::outgoingCall(const std::string& account_id,
 bool
 Manager::answerCall(const std::string& call_id)
 {
+    JAMI_INFO("Answer call %s", call_id.c_str());
+
     bool result = true;
 
     auto call = getCallFromCallID(call_id);
@@ -1284,6 +1286,8 @@ Manager::getConferenceFromID(const std::string& confID) const
 bool
 Manager::holdConference(const std::string& id)
 {
+    JAMI_INFO("Hold conference %s", id.c_str());
+
     if (auto conf = getConferenceFromID(id)) {
         conf->detach();
         emitSignal<DRing::CallSignal::ConferenceChanged>(conf->getConfID(), conf->getStateStr());
@@ -1347,7 +1351,7 @@ Manager::addParticipant(const std::string& callId, const std::string& conference
         return true;
     }
 
-    JAMI_DBG("Add participant %s to %s", callId.c_str(), conferenceId.c_str());
+    JAMI_DBG("Add participant %s to conference %s", callId.c_str(), conferenceId.c_str());
 
     // store the current call id (it will change in offHoldCall or in answerCall)
     auto current_call_id = getCurrentCallId();
@@ -1360,7 +1364,7 @@ Manager::addParticipant(const std::string& callId, const std::string& conference
 
     // TODO: remove this ugly hack => There should be different calls when double clicking
     // a conference to add main participant to it, or (in this case) adding a participant
-    // toconference
+    // to conference
     pimpl_->unsetCurrentCall();
 
     pimpl_->addMainParticipant(*conf);
@@ -1384,10 +1388,16 @@ Manager::ManagerPimpl::addMainParticipant(Conference& conf)
 bool
 Manager::addMainParticipant(const std::string& conference_id)
 {
+    JAMI_INFO("Add main participant to conference %s", conference_id.c_str());
+
     if (auto conf = getConferenceFromID(conference_id)) {
         pimpl_->addMainParticipant(*conf);
+        JAMI_DBG("Successfully added main participant to conference %s",
+            conference_id.c_str());
         return true;
     } else
+        JAMI_WARN("Failed to add main participant to conference %s",
+            conference_id.c_str());
         return false;
 }
 
@@ -1400,6 +1410,8 @@ Manager::getCallFromCallID(const std::string& callID) const
 bool
 Manager::joinParticipant(const std::string& callId1, const std::string& callId2, bool attached)
 {
+    JAMI_INFO("JoinParticipant(%s, %s, %i)", callId1.c_str(), callId2.c_str(), attached);
+
     if (callId1 == callId2) {
         JAMI_ERR("Cannot join participant %s to itself", callId1.c_str());
         return false;
@@ -1412,7 +1424,7 @@ Manager::joinParticipant(const std::string& callId1, const std::string& callId2,
         return false;
     }
 
-    // Set corresponding conderence details
+    // Set corresponding conference details
     auto call2 = getCallFromCallID(callId2);
     if (!call2) {
         JAMI_ERR("Could not find call %s", callId2.c_str());
@@ -1506,7 +1518,8 @@ Manager::setActiveParticipant(const std::string& confId, const std::string& call
 bool
 Manager::detachLocalParticipant(const std::string& conf_id)
 {
-    JAMI_DBG("Unbind local participant from conference");
+    JAMI_INFO("Detach local participant from conference %s", conf_id.c_str());
+
     if (auto conf = getConferenceFromID(conf_id.empty() ? getCurrentCallId() : conf_id)) {
         conf->detach();
         emitSignal<DRing::CallSignal::ConferenceChanged>(conf->getConfID(), conf->getStateStr());
@@ -1617,6 +1630,8 @@ Manager::joinConference(const std::string& conf_id1, const std::string& conf_id2
 void
 Manager::addAudio(Call& call)
 {
+    JAMI_INFO("Add audio to call %s", call.getCallId().c_str());
+
     const auto& call_id = call.getCallId();
 
     if (isConferenceParticipant(call_id)) {
@@ -1819,6 +1834,9 @@ Manager::incomingCallsWaiting()
 void
 Manager::incomingCall(Call& call, const std::string& accountId)
 {
+    JAMI_INFO("Incoming call %s on account %s)",
+        call.getCallId().c_str(), accountId.c_str());
+
     stopTone();
     const std::string callID(call.getCallId());
 
diff --git a/src/media/audio/ringbuffer.cpp b/src/media/audio/ringbuffer.cpp
index d8f792247a..bfef4767fc 100644
--- a/src/media/audio/ringbuffer.cpp
+++ b/src/media/audio/ringbuffer.cpp
@@ -51,7 +51,14 @@ RingBuffer::RingBuffer(const std::string& rbuf_id, size_t /*size*/, AudioFormat
     , resizer_(format_, format_.sample_rate / 50, [this](std::shared_ptr<AudioFrame>&& frame) {
         putToBuffer(std::move(frame));
     })
-{}
+{
+    JAMI_INFO("Create new RingBuffer %s", id.c_str());
+}
+
+RingBuffer::~RingBuffer()
+{
+    JAMI_INFO("Destroy RingBuffer %s", id.c_str());
+}
 
 void
 RingBuffer::flush(const std::string& call_id)
diff --git a/src/media/audio/ringbuffer.h b/src/media/audio/ringbuffer.h
index 7ebbc4b656..11cdc3fe00 100644
--- a/src/media/audio/ringbuffer.h
+++ b/src/media/audio/ringbuffer.h
@@ -54,6 +54,11 @@ public:
      */
     RingBuffer(const std::string& id, size_t size, AudioFormat format = AudioFormat::MONO());
 
+    /**
+     * Destructor
+     */
+    ~RingBuffer();
+
     const std::string& getId() const { return id; }
 
     /**
diff --git a/src/media/audio/ringbufferpool.cpp b/src/media/audio/ringbufferpool.cpp
index 88a16bfbe0..21bcd8e94c 100644
--- a/src/media/audio/ringbufferpool.cpp
+++ b/src/media/audio/ringbufferpool.cpp
@@ -172,6 +172,8 @@ RingBufferPool::removeReaderFromRingBuffer(const std::shared_ptr<RingBuffer>& rb
 void
 RingBufferPool::bindCallID(const std::string& call_id1, const std::string& call_id2)
 {
+    JAMI_INFO("Bind call %s to call %s", call_id1.c_str(), call_id2.c_str());
+
     const auto& rb_call1 = getRingBuffer(call_id1);
     if (not rb_call1) {
         JAMI_ERR("No ringbuffer associated to call '%s'", call_id1.c_str());
@@ -205,6 +207,8 @@ RingBufferPool::bindHalfDuplexOut(const std::string& process_id, const std::stri
 void
 RingBufferPool::unBindCallID(const std::string& call_id1, const std::string& call_id2)
 {
+    JAMI_INFO("Unbind calls %s and %s", call_id1.c_str(), call_id2.c_str());
+
     const auto& rb_call1 = getRingBuffer(call_id1);
     if (not rb_call1) {
         JAMI_ERR("No ringbuffer associated to call '%s'", call_id1.c_str());
@@ -235,6 +239,8 @@ RingBufferPool::unBindHalfDuplexOut(const std::string& process_id, const std::st
 void
 RingBufferPool::unBindAll(const std::string& call_id)
 {
+    JAMI_INFO("Unbind call %s from all bound calls", call_id.c_str());
+
     const auto& rb_call = getRingBuffer(call_id);
     if (not rb_call) {
         JAMI_ERR("No ringbuffer associated to call '%s'", call_id.c_str());
-- 
GitLab