From faf99abf603678abef2ae8f20ae715755d141b7c Mon Sep 17 00:00:00 2001 From: Donat Zenichev Date: Thu, 27 Jun 2024 18:03:01 +0200 Subject: [PATCH] MT#59851 CallLeg: Refactor hold related things - remove `hm` type, which used to be a crutch - only use the `holdMethod` type for hold method detection (so instead of `hm` used) - move `holdMethod` from .cpp to .h - make `CallLeg::hold` to a `enum holdAction` data type (so this enum can be reused) - make `CallLeg::hold_method_requested` to a `enum holdType` data type (so this enum can be reused) - move the `isHoldRequest()` helper to class methods instead of being static - rename `hold_method_requested` to `hold_type_requested` to have less confusion in naming (with `enum holdMethod` for example) Change-Id: Ib40a9e1c40419d6ffd444b59f92958c893c7f268 --- apps/sbc/CallLeg.cpp | 164 ++++++++++++++++++++----------------------- apps/sbc/CallLeg.h | 38 +++++++--- 2 files changed, 102 insertions(+), 100 deletions(-) diff --git a/apps/sbc/CallLeg.cpp b/apps/sbc/CallLeg.cpp index 7479fc6d..94917909 100644 --- a/apps/sbc/CallLeg.cpp +++ b/apps/sbc/CallLeg.cpp @@ -75,18 +75,15 @@ ReliableB2BEvent::~ReliableB2BEvent() } } -//////////////////////////////////////////////////////////////////////////////// -// helper functions - -enum HoldMethod { SendonlyStream, InactiveStream, ZeroedConnection, RecvonlyStream, None }; - static const string sendonly("sendonly"); static const string recvonly("recvonly"); static const string sendrecv("sendrecv"); static const string inactive("inactive"); - static const string zero_connection("0.0.0.0"); +//////////////////////////////////////////////////////////////////////////////// +// helper functions + /** returns true if connection is avtive. * Returns given default_value if the connection address is empty to cope with * connection address set globaly and not per media stream */ @@ -163,69 +160,6 @@ static bool isSDPBodyHold(const AmSdp &sdp) return false; } -static bool isHoldRequest(const AmSdp &sdp, HoldMethod &method) -{ - /* set defaults from session parameters and attributes - * inactive/sendonly/sendrecv/recvonly may be given as session attributes, - * connection can be given for session as well */ - bool connection_active = connectionActive(sdp.conn, false /* empty connection like inactive? */); - MediaActivity session_activity = getMediaActivity(sdp.attributes, Sendrecv); - - for (std::vector::const_iterator m = sdp.media.begin(); - m != sdp.media.end(); ++m) - { - if (m->port == 0) continue; /* this stream is disabled, handle like inactive (?) */ - if (!connectionActive(m->conn, connection_active)) { - method = ZeroedConnection; - continue; - } - switch (getMediaActivity(*m, session_activity)) { - case Sendonly: - method = SendonlyStream; - continue; - - case Inactive: - method = InactiveStream; - continue; - - case Recvonly: - method = RecvonlyStream; - continue; - - case Sendrecv: - return false; /* media stream is active */ - } - } - - if (sdp.media.empty()) { - /* no streams in the SDP, needed to set the method somehow */ - if (!connection_active) method = ZeroedConnection; - else { - switch (session_activity) { - case Sendonly: - method = SendonlyStream; - break; - - case Inactive: - method = InactiveStream; - break; - - case Recvonly: - method = RecvonlyStream; - break; - - /* well, no stream is something like InactiveStream, isn't it? */ - case Sendrecv: - method = InactiveStream; - break; - - } - } - } - - return true; -} - static bool isDSMEarlyAnnounceForced(const std::string &hdrs) { string announce = getHeader(hdrs, SIP_HDR_P_DSM_APP); @@ -241,7 +175,7 @@ CallLeg::CallLeg(const CallLeg* caller, AmSipDialog* p_dlg, AmSipSubscription* p call_status(Disconnected), on_hold(false), hold(PreserveHoldStatus), - hold_method_requested(NonHold) + hold_type_requested(NonHold) { a_leg = !caller->a_leg; // we have to be the complement @@ -302,7 +236,7 @@ CallLeg::CallLeg(AmSipDialog* p_dlg, AmSipSubscription* p_subs) call_status(Disconnected), on_hold(false), hold(PreserveHoldStatus), - hold_method_requested(NonHold) + hold_type_requested(NonHold) { a_leg = true; @@ -338,6 +272,59 @@ CallLeg::~CallLeg() SBCCallRegistry::removeCall(getLocalTag()); } +bool CallLeg::isHoldRequest(const AmSdp &sdp, holdMethod &method) +{ + /* set defaults from session parameters and attributes + * inactive/sendonly/sendrecv/recvonly may be given as session attributes, + * connection can be given for session as well */ + bool connection_active = connectionActive(sdp.conn, false /* empty connection like inactive? */); + MediaActivity session_activity = getMediaActivity(sdp.attributes, Sendrecv); + for (std::vector::const_iterator m = sdp.media.begin(); + m != sdp.media.end(); ++m) + { + if (m->port == 0) continue; /* this stream is disabled, handle like inactive (?) */ + if (!connectionActive(m->conn, connection_active)) { + method = ZeroedConnection; + continue; + } + switch (getMediaActivity(*m)) { + case Sendonly: + method = SendonlyStream; + continue; + case Inactive: + method = InactiveStream; + continue; + case Recvonly: + method = RecvonlyStream; + continue; + case Sendrecv: + return false; /* media stream is active */ + } + } + if (sdp.media.empty()) { + /* no streams in the SDP, needed to set the method somehow */ + if (!connection_active) method = ZeroedConnection; + else { + switch (session_activity) { + case Sendonly: + method = SendonlyStream; + break; + case Inactive: + method = InactiveStream; + break; + case Recvonly: + method = RecvonlyStream; + break; + /* well, no stream is something like InactiveStream, isn't it? */ + case Sendrecv: + method = InactiveStream; + break; + } + } + } + return true; +} + void CallLeg::terminateOtherLeg() { if (call_status != Connected) { @@ -1128,21 +1115,19 @@ void CallLeg::onSipRequest(const AmSipRequest& req) * to avoid other confusions... */ dlg->reply(req, 200, "OK"); - } - else - { + } else { /** only for requests which put the call on hold. * Remember that we have to answer to the one, who puts on hold, * with the 'inactive' back (as soon as the on hold is accepted with the 200OK * by the other side of the call) in case the on hold was requested using 'inactive' */ AmSdp sdp; - hm hold_method; + holdMethod hold_method; if (req.method == SIP_METH_INVITE && retrieveAmSdp(req.body, sdp)) { /* case when remote side puts us on hold */ if (isOnHoldRequested(sdp, hold_method)) updateHoldMethod(hold_method); - /* it's likely sendrecv - then make sure 'hold_method_requested' is kept updated */ - else hold_method_requested = NonHold; + /* it's likely sendrecv - then make sure 'hold_type_requested' is kept updated */ + else hold_type_requested = NonHold; } AmB2BSession::onSipRequest(req); } @@ -1414,21 +1399,22 @@ void CallLeg::updateCallStatus(CallStatus new_status, const StatusChangeCause &c onCallStatusChange(cause); } -void CallLeg::updateHoldMethod(const hm &hm) +void CallLeg::updateHoldMethod(const holdMethod &hm) { switch (hm) { - case SendonlyStream: hold_method_requested = SendonlyHold; break; - case InactiveStream: hold_method_requested = InactiveHold; break; - case ZeroedConnection: hold_method_requested = ZeroedHold; break; - default: hold_method_requested = NonHold; + case SendonlyStream: hold_type_requested = SendonlyHold; break; + case InactiveStream: hold_type_requested = InactiveHold; break; + case ZeroedConnection: hold_type_requested = ZeroedHold; break; + default: hold_type_requested = NonHold; } - DBG("hold_method_requested is set to: <%d> for LT <%s>\n", - hold_method_requested, getLocalTag().c_str()); + DBG("hold_type_requested is set to: <%d> for LT <%s>\n", + hold_type_requested, getLocalTag().c_str()); } -bool CallLeg::isOnHoldRequested(const AmSdp &sdp, hm &method) + +bool CallLeg::isOnHoldRequested(const AmSdp &sdp, holdMethod &method) { - if (isHoldRequest(sdp, (HoldMethod&)method)) { + if (isHoldRequest(sdp, (holdMethod&)method)) { DBG("This request puts the call on hold\n"); return true; } @@ -1808,7 +1794,7 @@ void CallLeg::adjustOffer(AmSdp &sdp) } else { /* handling B2B SDP, check for hold/unhold */ - HoldMethod hm = None; + holdMethod hm = None; /* if hold request, transform to requested kind of hold and remember that hold * was requested with this offer */ @@ -1852,13 +1838,13 @@ void CallLeg::updateLocalSdp(AmSdp &sdp) * - 'inactive' must be faced with the 'inactive' sent back as an answer * This block is only needed for cases, when MoH is emulated on the SEMS directly. */ - else if (hold == PreserveHoldStatus && hold_method_requested != NonHold) + else if (hold == PreserveHoldStatus && hold_type_requested != NonHold) { for (std::vector::iterator m = sdp.media.begin(); m != sdp.media.end(); ++m) { if (m->isAudio()) { - switch(hold_method_requested) + switch(hold_type_requested) { case SendonlyHold: m->send = false; /* make sure to answer with the recvonly, if the on hold */ diff --git a/apps/sbc/CallLeg.h b/apps/sbc/CallLeg.h index aa101d17..07992c85 100644 --- a/apps/sbc/CallLeg.h +++ b/apps/sbc/CallLeg.h @@ -141,8 +141,30 @@ class CallLeg: public AmB2BSession bool on_hold; // remote is on hold AmSdp non_hold_sdp; - enum { HoldRequested, ResumeRequested, PreserveHoldStatus } hold; - enum { NonHold, InactiveHold, SendonlyHold, ZeroedHold } hold_method_requested; + + /** + * TODO: clear thing around hold out. + * what is the difference between holdType and HoldMethod? + * This can probably be merged into one type later. + */ + enum holdAction { + HoldRequested, + ResumeRequested, + PreserveHoldStatus + } hold; + enum holdType { + NonHold, + InactiveHold, + SendonlyHold, + ZeroedHold + } hold_type_requested; + enum holdMethod { + SendonlyStream, + InactiveStream, + ZeroedConnection, + RecvonlyStream, + None + }; // queue of session update operations, first element is possibly the one // being in progress @@ -190,21 +212,15 @@ class CallLeg: public AmB2BSession void updateCallStatus(CallStatus new_status, const StatusChangeCause &cause = StatusChangeCause()); - /** TODO: bring 'enum HoldMethod' from the .cpp file inside the CallLeg class definition. - * Currently this is a rude implementation which creates obstacles, - * and forces us to have a duplicate (hm) here in the class, when we want to have it as param. - * Additionally we have to constantly cast HoldMethod -> hm while working with them. - */ - enum hm { SendonlyStream, InactiveStream, ZeroedConnection }; + bool isHoldRequest(const AmSdp &sdp, holdMethod &method); /** keep the method, which was used to put the call * on hold, in the SDP offer received (most likely in re-INVITE) */ - void updateHoldMethod(const hm &hm); + void updateHoldMethod(const holdMethod &hm); /* check if this request assumes call to be put on hold, * this in internal class'es implementation. - * hm - hold method (SendonlyStream, InactiveStream, ZeroedConnection) */ - bool isOnHoldRequested(const AmSdp &sdp, hm &hm); + bool isOnHoldRequested(const AmSdp &sdp, holdMethod &hm); /* set AmSdp based on AmMimeBody */ bool retrieveAmSdp(const AmMimeBody &mSdp, AmSdp &sdp);