Some callbacks not invoked in call replace in PJSUA2 (#3059)

Warning: potential backward incompatibility issue, previously the replacing call can use any account (selected using pjsua_acc_find_for_incoming() and app may override via callback), now it is forced to use the same account.
pjsip · Apr 26, 2022 · 41023da · 41023da
1 parent 6713e02
commit 41023da
Show file tree

Hide file tree

Showing 5 changed files with 128 additions and 24 deletions.
diff --git a/pjsip-apps/src/samples/pjsua2_demo.cpp b/pjsip-apps/src/samples/pjsua2_demo.cpp
@@ -72,7 +72,7 @@ class MyCall : public Call
 
     virtual void onCallState(OnCallStateParam &prm);
     virtual void onCallTransferRequest(OnCallTransferRequestParam &prm);
-    virtual void onCallReplaced(OnCallReplacedParam &prm);
+    virtual void onCallReplaceRequest(OnCallReplaceRequestParam &prm);
     virtual void onCallMediaState(OnCallMediaStateParam &prm);
 };
 
@@ -190,10 +190,10 @@ void MyCall::onCallTransferRequest(OnCallTransferRequestParam &prm)
     prm.newCall = new MyCall(*myAcc);
 }
 
-void MyCall::onCallReplaced(OnCallReplacedParam &prm)
+void MyCall::onCallReplaceRequest(OnCallReplaceRequestParam &prm)
 {
     /* Create new Call for call replace */
-    prm.newCall = new MyCall(*myAcc, prm.newCallId);
+    prm.newCall = new MyCall(*myAcc);
 }
 
 

diff --git a/pjsip/include/pjsua2/call.hpp b/pjsip/include/pjsua2/call.hpp
@@ -965,6 +965,11 @@ struct OnCallReplaceRequestParam
      * the call being replaced.
      */
     CallSetting         opt;
+
+    /**
+     * New Call derived object instantiated by application.
+     */
+    Call		*newCall;
 };
 
 /**
@@ -1900,7 +1905,9 @@ class Call
      *
      * If application decides to accept the transfer request, it must also
      * instantiate the new Call object for the transfer operation and return
-     * this new Call object to prm.newCall.
+     * this new Call object to prm.newCall. For the new Call instance,
+     * the account should use the same account as this call and the call ID
+     * must be set to PJSUA_INVALID_ID.
      * 
      * If application does not specify new Call object, library will reuse the
      * existing Call object for initiating the new call (to the transfer
@@ -1930,6 +1937,19 @@ class Call
      * Notify application about incoming INVITE with Replaces header.
      * Application may reject the request by setting non-2xx code.
      *
+     * In this callback, application should create a new Call instance and
+     * return the Call object via prm.newCall. In creating the new Call
+     * instance, the account should use the same account as this call and
+     * the call ID must be set to PJSUA_INVALID_ID.
+     *
+     * If application does not specify new Call object, library will reuse the
+     * existing Call object for callbacks. In this case, any events from
+     * both calls (replaced and new) will be delivered to the same Call object,
+     * where the call ID will be switched back and forth between callbacks.
+     * Application must be careful to not destroy the Call object when
+     * receiving disconnection event of the replaced call after the transfer
+     * process is completed.
+     *
      * @param prm	Callback parameter.
      */
     virtual void onCallReplaceRequest(OnCallReplaceRequestParam &prm)
@@ -1941,11 +1961,24 @@ class Call
      * request with Replaces header.
      *
      * After this callback is called, normally PJSUA-API will disconnect
-     * this call and establish a new call. To be able to control the call,
-     * e.g: hold, transfer, change media parameters, application must
-     * instantiate a new Call object for the new call using call ID
-     * specified in prm.newCallId, and return the Call object via
-     * prm.newCall.
+     * this call and establish a new call.
+     *
+     * If not yet done in onCallReplaceRequest(), application can create
+     * the new Call instance and return the Call object via prm.newCall.
+     * In creating the new Call instance, the account should use the same
+     * account as this call and the call ID must be set to prm.newCallId.
+     *
+     * If the new Call instance has been setup in onCallReplaceRequest(),
+     * the prm.newCall should contain the new Call instance and application
+     * MUST not change it.
+     *
+     * If application does not specify new Call object, library will reuse the
+     * existing Call object for callbacks. In this case, any events from
+     * both calls (replaced and new) will be delivered to the same Call object,
+     * where the call ID will be switched back and forth between callbacks.
+     * Application must be careful to not destroy the Call object when
+     * receiving disconnection event of the replaced call after the transfer
+     * process is completed.
      *
      * @param prm	Callback parameter.
      */

diff --git a/pjsip/src/pjsua-lib/pjsua_call.c b/pjsip/src/pjsua-lib/pjsua_call.c
@@ -1599,6 +1599,12 @@ pj_bool_t pjsua_call_on_incoming(pjsip_rx_data *rdata)
 				st_code, &st_text, NULL, NULL, NULL);
 	    goto on_return;
 	}
+
+	/* Set the user_data of the new call to the existing/parent call,
+	 * it is needed by PJSUA2 to update its states. While PJSUA app can
+	 * always override it anytime.
+	 */
+	pjsua_call_set_user_data(call_id, replaced_call->user_data);
     }
 
     if (!replaced_dlg) {
@@ -1611,16 +1617,23 @@ pj_bool_t pjsua_call_on_incoming(pjsip_rx_data *rdata)
      * call. We need the account to find which contact URI to put for
      * the call.
      */
-    acc_id = call->acc_id = pjsua_acc_find_for_incoming(rdata);
-    if (acc_id == PJSUA_INVALID_ID) {
-	pjsip_endpt_respond_stateless(pjsua_var.endpt, rdata,
-				      PJSIP_SC_TEMPORARILY_UNAVAILABLE, NULL,
-				      NULL, NULL);
+    if (replaced_dlg) {
+	/* For call replace, use the same account as the replaced call */
+	pjsua_call *replaced_call;
+	replaced_call = (pjsua_call*)replaced_dlg->mod_data[pjsua_var.mod.id];
+	acc_id = call->acc_id = replaced_call->acc_id;
+    } else {
+	acc_id = call->acc_id = pjsua_acc_find_for_incoming(rdata);
+	if (acc_id == PJSUA_INVALID_ID) {
+	    pjsip_endpt_respond_stateless(pjsua_var.endpt, rdata,
+					  PJSIP_SC_TEMPORARILY_UNAVAILABLE,
+					  NULL, NULL, NULL);
 
-	PJ_LOG(2,(THIS_FILE,
-		  "Unable to accept incoming call (no available account)"));
+	    PJ_LOG(2,(THIS_FILE,
+		      "Unable to accept incoming call (no available account)"));
 
-	goto on_return;
+	    goto on_return;
+	}
     }
     call->call_hold_type = pjsua_var.acc[acc_id].cfg.call_hold_type;
 
@@ -6071,7 +6084,11 @@ static void on_call_transferred( pjsip_inv_session *inv,
 	pj_list_push_back(&msg_data.hdr_list, dup);
     }
 
-    /* Now make the outgoing call. */
+    /* Now make the outgoing call.
+     * Note that the user_data of the new call is initialized to the
+     * original call, it is needed by PJSUA2 to update its states.
+     * While PJSUA app can always override it anytime.
+     */
     tmp = pj_str(uri);
     status = pjsua_call_make_call(existing_call->acc_id, &tmp, &call_opt,
 				  existing_call->user_data, &msg_data,

diff --git a/pjsip/src/pjsua2/call.cpp b/pjsip/src/pjsua2/call.cpp
@@ -515,7 +515,11 @@ Call *Call::lookup(int call_id)
     Call *call = (Call*)pjsua_call_get_user_data(call_id);
     if (call && call_id != call->id) {
 	if (call->child && call->child->id == PJSUA_INVALID_ID) {
-	    /* This must be a new call from call transfer */
+	    /* This must be a new call from call transfer or call replace
+	     * which initially shares user_data with its parent (so the
+	     * user_data points to its parent's Call instance).
+	     * Let's update its user_data to its own Call instance.
+	     */
 	    call = call->child;
 	    pjsua_call_set_user_data(call_id, call);
 	}

diff --git a/pjsip/src/pjsua2/endpoint.cpp b/pjsip/src/pjsua2/endpoint.cpp
@@ -1398,13 +1398,22 @@ void Endpoint::on_call_transfer_request2(pjsua_call_id call_id,
     *opt = prm.opt.toPj();
     if (*code/100 <= 2) {
 	if (prm.newCall) {
+	    /* Sanity checks */
+	    pj_assert(prm.newCall->id == PJSUA_INVALID_ID);
+	    pj_assert(prm.newCall->acc.getId() == call->acc.getId());
+
 	    /* We don't manage (e.g: create, delete) the call child,
 	     * so let's just override any existing child.
 	     */
 	    call->child = prm.newCall;
 	    call->child->id = PJSUA_INVALID_ID;
+
+	    /* The newCall shares the same user_data as the parent call,
+	     * the next Call::lookup(new_call_id) will assign the call ID
+	     * and update user_data for the newCall.
+	     */
 	} else {
-	    PJ_LOG(4,(THIS_FILE,
+	    PJ_LOG(3,(THIS_FILE,
 		      "Warning: application reuses Call instance in "
 		      "call transfer (call ID:%d)", call_id));
 	}
@@ -1449,37 +1458,78 @@ void Endpoint::on_call_replace_request2(pjsua_call_id call_id,
     prm.statusCode = (pjsip_status_code)*st_code;
     prm.reason = pj2Str(*st_text);
     prm.opt.fromPj(*opt);
+    prm.newCall = NULL;
 
     call->onCallReplaceRequest(prm);
 
     *st_code = prm.statusCode;
     *st_text = str2Pj(prm.reason);
     *opt = prm.opt.toPj();
+    if (prm.newCall && prm.newCall != call) {
+	/* Sanity checks */
+	pj_assert(prm.newCall->id == PJSUA_INVALID_ID);
+	pj_assert(prm.newCall->acc.getId() == call->acc.getId());
+
+	/* We don't manage (e.g: create, delete) the call child,
+	 * so let's just override any existing child.
+	 */
+	call->child = prm.newCall;
+	call->child->id = PJSUA_INVALID_ID;
+
+	/* The newCall shares the same user_data as the parent call,
+	 * the next Call::lookup(new_call_id) will assign the call ID
+	 * and update user_data for the newCall.
+	 */
+    } else {
+	PJ_LOG(3,(THIS_FILE,
+		  "Warning: application has not created new Call instance "
+		  "for call replace request (call ID:%d)", call_id));
+    }
 }
 
 void Endpoint::on_call_replaced(pjsua_call_id old_call_id,
                                 pjsua_call_id new_call_id)
 {
+    /* Lookup the new call first, to avoid Call::lookup() overwriting
+     * Call.id (to the new Call).
+     */
+    Call *new_call = Call::lookup(new_call_id);
+
     Call *call = Call::lookup(old_call_id);
     if (!call) {
 	return;
     }
+
+    /* Check if new call object has not been created in
+     * onCallReplaceRequest().
+     */
+    if (new_call == call)
+	new_call = NULL;
 
     OnCallReplacedParam prm;
     prm.newCallId = new_call_id;
-    prm.newCall = NULL;
+    prm.newCall = new_call;
 
     call->onCallReplaced(prm);
 
-    if (prm.newCall) {
+    if (prm.newCall && prm.newCall != call) {
 	/* Sanity checks */
 	pj_assert(prm.newCall->id == new_call_id);
 	pj_assert(prm.newCall->acc.getId() == call->acc.getId());
 	pj_assert(pjsua_call_get_user_data(new_call_id) == prm.newCall);
+
+	/* Warn if new_call created in onCallReplaceRequest() is changed */
+	if (new_call && new_call != prm.newCall) {
+	    PJ_LOG(3,(THIS_FILE,
+		      "Warning: application has created a new Call instance "
+		      "in onCallReplaceRequest, but created another in "
+		      "onCallReplaced (call ID:%d)",
+		      new_call_id));
+	}
     } else {
-	PJ_LOG(4,(THIS_FILE,
+	PJ_LOG(3,(THIS_FILE,
 		  "Warning: application has not created new Call instance "
-		  "for call replace (old call ID:%d, new call ID: %d)",
+		  "for call replace (old call ID:%d, new call ID:%d)",
 		  old_call_id, new_call_id));
     }
 }