Commit 6a93bcdc authored by Guillaume Roguez's avatar Guillaume Roguez Committed by Aline Bonnet

call: fix multidevice implementation

Following patch was not reviewed (bussiness pressure):
1dccd81b [multi-device: initial implementation]

This patch repays it partially by fixing Call::addSubCall() and
Call::merge() (and overloading) methods.

Many weird and hard-to-debug errors are fixed, as example:
when a device call is in ICE connection step and fails
due to OpenDHT error, the device call is terminated but not the
master call and client is not aware of the situation.

It's a complex patch due to the interlacements of some changes.
Consider it as a new implementation of multi-device inside Call/IM
components.

This is a lists of changes made:

- add more documentation to indicate the intent.
- use reference (&) rather than const shared_ptr<>& type
  as it's identical, if we know that we'll not keep a reference
  (like in merge).
- add Call::getPtr() and SIPCall::getPtr() helper functions to
  simplify code when we need smart pointer on such objects.
  This also permits to make SIPCall::merge() private and
  more safe-to-bad-usage.
  This also removes the RTTI usage due to a dynamic cast and reduce API.
- fix std::set() to use correct Compare method when type is a smart ptr.
- use parent (Call&) rather than the "quiet" member, not always used
  as its name indicates and can reduce the code.
- remove TRYING state set in addSubcall() as this call is already
  in this state.
- lock callMutex at right places to ensure multi-thread safety.
- replace "fat-lambda" by member method (lambda is an unnamed function,
  it's hard to debug in gdb, so keep it thin and simple to make sure
  we don't need to debug that).
- better pending IM handling.
- use "override" when we override a virtual method.
- return immediately when instance maybe deleted as we relaxed smrt_ptr.
- fix the incorrect state of the parent call when subcall is over.
- do not send handle IM when subcall is being answered (it's the job
  of the parent call).
- log pollution: replace RING_WARN by RING_DBG when we don't want to warn
- remove/re-write log output to have consistency, useful and
  non-redundant information.
- code cleanup, coding rules, logic blocks spacing, etc.

Change-Id: I8da9747db6ca01d6a50dcde2378ecf8499cf6685
Reviewed-by: Andreas Traczyk's avatarAndreas Traczyk <andreas.traczyk@savoirfairelinux.com>
Reviewed-by: default avatarAline Bonnet <aline.bonnet@savoirfairelinux.com>
parent 3466a4a9
This diff is collapsed.
......@@ -41,6 +41,7 @@
#include <condition_variable>
#include <set>
#include <list>
#include <atomic>
namespace ring {
......@@ -57,6 +58,8 @@ template <class T> using CallMap = std::map<std::string, std::shared_ptr<T> >;
class Call : public Recordable, public std::enable_shared_from_this<Call> {
public:
using SubcallSet = std::set<std::shared_ptr<Call>, std::owner_less<std::shared_ptr<Call>>>;
static const char * const DEFAULT_ID;
/**
......@@ -323,11 +326,16 @@ class Call : public Recordable, public std::enable_shared_from_this<Call> {
void addStateListener(T&& list) {
stateChangedListeners_.emplace_back(std::forward<T>(list));
}
void addSubCall(const std::shared_ptr<Call>& call);
virtual void merge(const std::shared_ptr<Call>& scall);
/**
* Attach subcall to this instance.
* If this subcall is answered, this subcall and this instance will be merged using merge().
*/
void addSubCall(Call& call);
protected:
virtual void merge(Call& scall);
/**
* Constructor of a call
* @param id Unique identifier of the call
......@@ -341,12 +349,20 @@ class Call : public Recordable, public std::enable_shared_from_this<Call> {
bool isAudioMuted_{false};
bool isVideoMuted_{false};
bool quiet {false};
std::set<std::shared_ptr<Call>> subcalls {};
std::list<std::pair<std::map<std::string, std::string>, std::string>> pendingInMessages_ {};
std::list<std::pair<std::map<std::string, std::string>, std::string>> pendingOutMessages_ {};
///< MultiDevice: parent call, nullptr otherwise.
std::atomic<Call*> parent_ {nullptr};
///< MultiDevice: list of attached subcall
SubcallSet subcalls_;
using MsgList = std::list<std::pair<std::map<std::string, std::string>, std::string>>;
///< MultiDevice: message waiting to be sent (need a valid subcall)
MsgList pendingOutMessages_;
private:
friend void hangupCallsIf(Call::SubcallSet, int, const std::function<bool(Call*)>&);
bool validStateTransition(CallState newState);
......@@ -354,6 +370,10 @@ class Call : public Recordable, public std::enable_shared_from_this<Call> {
void checkAudio();
void subcallStateChanged(Call&, Call::CallState, Call::ConnectionState);
SubcallSet safePopSubcalls();
/** Protect every attribute that can be changed by two threads */
mutable std::recursive_mutex callMutex_ {};
......@@ -395,6 +415,19 @@ class Call : public Recordable, public std::enable_shared_from_this<Call> {
std::string peerDisplayName_ {};
time_t timestamp_start_ {0};
///< MultiDevice: message received by subcall to merged yet
MsgList pendingInMessages_;
};
// Helpers
/**
* Obtain a shared smart pointer of instance
*/
inline std::shared_ptr<Call> getPtr(Call& call)
{
return call.shared_from_this();
}
} // namespace ring
......@@ -453,7 +453,7 @@ RingAccount::startOutgoingCall(const std::shared_ptr<SIPCall>& call, const std::
return;
}
call->addSubCall(dev_call);
call->addSubCall(*dev_call);
manager.addTask([sthis, weak_dev_call, ice, dev] {
auto call = weak_dev_call.lock();
......
......@@ -663,9 +663,9 @@ SIPCall::sendTextMessage(const std::map<std::string, std::string>& messages,
//TODO: for now we ignore the "from" (the previous implementation for sending this info was
// buggy and verbose), another way to send the original message sender will be implemented
// in the future
if (not subcalls.empty()) {
if (not subcalls_.empty()) {
pendingOutMessages_.emplace_back(messages, from);
for (auto& c : subcalls)
for (auto& c : subcalls_)
c->sendTextMessage(messages, from);
} else {
if (inv) {
......@@ -710,7 +710,7 @@ SIPCall::onAnswered()
RING_WARN("[call:%s] onAnswered()", getCallId().c_str());
if (getConnectionState() != ConnectionState::CONNECTED) {
setState(CallState::ACTIVE, ConnectionState::CONNECTED);
if (not quiet)
if (not parent_.load())
Manager::instance().peerAnsweredCall(*this);
}
}
......@@ -888,7 +888,7 @@ SIPCall::startAllMedia()
}
}
if (not quiet and peerHolding_ != peer_holding) {
if (not parent_.load() and peerHolding_ != peer_holding) {
peerHolding_ = peer_holding;
emitSignal<DRing::CallSignal::PeerHold>(getCallId(), peerHolding_);
}
......@@ -934,7 +934,7 @@ SIPCall::muteMedia(const std::string& mediaType, bool mute)
isVideoMuted_ = mute;
videoInput_ = isVideoMuted_ ? "" : Manager::instance().getVideoManager().videoDeviceMonitor.getMRLForDefaultDevice();
DRing::switchInput(getCallId(), videoInput_);
if (not quiet)
if (not parent_.load())
emitSignal<DRing::CallSignal::VideoMuted>(getCallId(), isVideoMuted_);
#endif
} else if (mediaType.compare(DRing::Media::Details::MEDIA_TYPE_AUDIO) == 0) {
......@@ -942,7 +942,7 @@ SIPCall::muteMedia(const std::string& mediaType, bool mute)
RING_WARN("[call:%s] audio muting %s", getCallId().c_str(), bool_to_str(mute));
isAudioMuted_ = mute;
avformatrtp_->setMuted(isAudioMuted_);
if (not quiet)
if (not parent_.load())
emitSignal<DRing::CallSignal::AudioMuted>(getCallId(), isAudioMuted_);
}
}
......@@ -954,7 +954,7 @@ SIPCall::onMediaUpdate()
stopAllMedia();
openPortsUPnP();
if (startIce()) {
if (not quiet)
if (not parent_.load())
waitForIceAndStartMedia();
} else {
RING_WARN("[call:%s] ICE not used for media", getCallId().c_str());
......@@ -1113,18 +1113,24 @@ SIPCall::initIceTransport(bool master, unsigned channel_num)
}
void
SIPCall::merge(const std::shared_ptr<SIPCall>& scall)
SIPCall::merge(Call& call)
{
RING_WARN("SIPCall::merge %s -> %s", scall->getCallId().c_str(), getCallId().c_str());
inv = std::move(scall->inv);
RING_DBG("[sipcall:%s] merge subcall %s", getCallId().c_str(), call.getCallId().c_str());
// This static cast is safe as this method is private and overload Call::merge
auto& subcall = static_cast<SIPCall&>(call);
inv = std::move(subcall.inv);
inv->mod_data[getSIPVoIPLink()->getModId()] = this;
setTransport(scall->transport_);
sdp_ = std::move(scall->sdp_);
peerHolding_ = scall->peerHolding_;
upnp_ = std::move(scall->upnp_);
std::copy_n(scall->contactBuffer_, PJSIP_MAX_URL_SIZE, contactBuffer_);
pj_strcpy(&contactHeader_, &scall->contactHeader_);
Call::merge(scall);
setTransport(subcall.transport_);
sdp_ = std::move(subcall.sdp_);
peerHolding_ = subcall.peerHolding_;
upnp_ = std::move(subcall.upnp_);
std::copy_n(subcall.contactBuffer_, PJSIP_MAX_URL_SIZE, contactBuffer_);
pj_strcpy(&contactHeader_, &subcall.contactHeader_);
Call::merge(subcall);
if (iceTransport_->isStarted())
waitForIceAndStartMedia();
}
......
......@@ -215,11 +215,6 @@ class SIPCall : public Call
void terminateSipSession(int status);
virtual void merge(const std::shared_ptr<Call>& scall) {
merge(std::dynamic_pointer_cast<SIPCall>(scall));
}
virtual void merge(const std::shared_ptr<SIPCall>& scall);
void setPeerRegistredName(const std::string& name) {
peerRegistredName_ = name;
}
......@@ -246,6 +241,8 @@ class SIPCall : public Call
int SIPSessionReinvite();
void merge(Call& call) override; // only called by Call
std::vector<IceCandidate> getAllRemoteCandidates();
std::unique_ptr<AudioRtpSession> avformatrtp_;
......@@ -282,6 +279,16 @@ class SIPCall : public Call
std::unique_ptr<ring::upnp::Controller> upnp_;
};
// Helpers
/**
* Obtain a shared smart pointer of instance
*/
inline std::shared_ptr<SIPCall> getPtr(SIPCall& call)
{
return std::static_pointer_cast<SIPCall>(call.shared_from_this());
}
} // namespace ring
#endif // __SIPCALL_H__
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment