Add on_rejected_incoming_call() callback #3683

trengginas · 2023-09-04T16:33:58Z

Consider the scenario: (on iOS)

User agent received push notification of a new call
the call is registered to iOS CallKit, and show to user notification about incoming call. wait for SIP INVITE
SIP INVITE is received, however is rejected by the library due to invalid message
Notification to the incoming call still shown since the application is not aware of the call rejection

This PR will add a new callback on_rejected_incoming_call() to notifiy the application in case incoming call is rejected.

wosrediinanatour · 2023-09-05T05:01:18Z

Does this handle INVITE crossover, i.e. SIP 491?

Having the SIP message, that has been rejected, would/could be useful. Maybe by using sip_event.

Isn't it implemented intentionally in PJSUA2?

pjsip/include/pjsua-lib/pjsua_internal.h

pjsip/include/pjsua-lib/pjsua.h

pjsip/src/pjsua-lib/pjsua_call.c

- save incoming rdata to be passed as callback parameter

sauwming · 2023-09-13T09:07:59Z

This approach seems to be more complicated than expected.

Some alternative ideas:
a. Add on_incoming_invite(). We simply call this new callback in the beginning of pjsua_call_on_incoming().
Pro: very simple. Con: Even though app can know if an incoming INVITE is eventually rejected by the library, it will not know the exact reason why.
b. Invoke on_rejected_incoming_call() from pjsua_call_on_incoming() instead.
Pro: allow more customization (app can change the status code/reason, perhaps even override the rejection, etc).

Personally, I like option b.

pjsip/src/pjsua-lib/pjsua_call.c

sauwming · 2023-09-15T00:50:48Z

A couple of spec questions to consider:

Is there any need for the user to get the actual rejection message in the form of tdata? So far, pjsua callback usually only provides the status code (and st_text) of the answer. If we don't need the tdata, it will be much simpler, you don't need to copy the implementation of pjsip_endpt_respond/respond_stateless() into pjsua_call.
Do we want to call the callback before, or after, rejecting the call?
If before, the callback will allow room for customisation, such as permitting user to modify the status code and text. But we may need to change the callback name since the call has not actually been rejected yet.
If after, the callback will serve as informational purpose only.

trengginas · 2023-09-15T08:07:30Z

A couple of spec questions to consider:

Is there any need for the user to get the actual rejection message in the form of tdata? So far, pjsua callback usually only provides the status code (and st_text) of the answer. If we don't need the tdata, it will be much simpler, you don't need to copy the implementation of pjsip_endpt_respond/respond_stateless() into pjsua_call.

Do we want to call the callback before, or after, rejecting the call?
If before, the callback will allow room for customisation, such as permitting user to modify the status code and text. But we may need to change the callback name since the call has not actually been rejected yet.
If after, the callback will serve as informational purpose only.

I agree to not include tdata on the callback param. It will simplify the solution. Adding st_text as parameter should also be useful.
I think we should not allow application to override the/change the status code and it should be informational only.

wosrediinanatour · 2023-09-15T08:43:21Z

A couple of spec questions to consider:

Is there any need for the user to get the actual rejection message in the form of tdata? So far, pjsua callback usually only provides the status code (and st_text) of the answer. If we don't need the tdata, it will be much simpler, you don't need to copy the implementation of pjsip_endpt_respond/respond_stateless() into pjsua_call.

Do we want to call the callback before, or after, rejecting the call?
If before, the callback will allow room for customisation, such as permitting user to modify the status code and text. But we may need to change the callback name since the call has not actually been rejected yet.
If after, the callback will serve as informational purpose only.
* I agree to not include `tdata` on the callback param. It will simplify the solution. Adding `st_text` as parameter should also be useful.

* I think we should not allow application to override the/change the status code and it should be informational only.

The transmitted message is very useful for tracing, when onCallTsxState(...) is not called.

sauwming · 2023-09-18T03:46:58Z

pjsip/include/pjsua-lib/pjsua.h

@@ -1967,6 +2016,20 @@ typedef struct pjsua_callback
     */
    void (*on_media_event)(pjmedia_event *event);

+    /**
+     * This callback is called when an incoming call is rejected.


Fix the doc.

The callback is now called only when it's internally rejected by the library, right? Will using pjsua_call_answer() also trigger the callback?

Minor:
make it clear in the doc that the callback is only called for calls that are internally rejected by the library. And the callback won't be called if user rejects the call using pjsua_call_answer/answer2().

No, the callback will not trigger when pjsua_call_answer() is used to reject the call

pjsip/include/pjsua2/endpoint.hpp

pjsip/src/pjsua-lib/pjsua_call.c

sauwming · 2023-09-18T04:04:11Z

The transmitted message is very useful for tracing, when onCallTsxState(...) is not called.

By "tracing", you mean for info only, correct?
Since it doesn't have any practical purpose, it's not strong enough reason to include tdata within the callback, considering the complexity to do so.

wosrediinanatour · 2023-09-18T06:11:04Z

The transmitted message is very useful for tracing, when onCallTsxState(...) is not called.

By "tracing", you mean for info only, correct? Since it doesn't have any practical purpose, it's not strong enough reason to include tdata within the callback, considering the complexity to do so.

Yes.

nanangizz · 2023-09-19T04:35:31Z

pjsip/src/pjsua-lib/pjsua_call.c

@@ -1475,6 +1516,8 @@ pj_bool_t pjsua_call_on_incoming(pjsip_rx_data *rdata)
    pjmedia_sdp_session *offer=NULL;
    pj_bool_t should_dec_dlg = PJ_FALSE;
    pjsip_tpselector tp_sel;
+    pj_str_t st_reason = pj_str("");
+    int st_code = 200;


Why not initialize st_code with zero?

It's following the original code on

pjproject/pjsip/src/pjsua-lib/pjsua_call.c

Line 1561 in 05d03ad

int st_code = 200;

It's following the original code on

pjproject/pjsip/src/pjsua-lib/pjsua_call.c

Line 1561 in 05d03ad

int st_code = 200;

Oh, "the original code" is actually the default answer code for replaced dialog (may be overriden by app from the replaced dlg callback). IMO better not share the same var with the replaced dialog, it serves different purpose, it may cause confusion in the future.

nanangizz · 2023-09-19T04:42:01Z

pjsip/src/pjsua-lib/pjsua_call.c

@@ -1965,6 +2023,7 @@ pj_bool_t pjsua_call_on_incoming(pjsip_rx_data *rdata)
            pjsua_perror(THIS_FILE, "Error initializing media channel", status);

            pjsip_dlg_inc_lock(dlg);
+            st_code = sip_err_code;


When pjsua_media_channel_init() is done async-ly or returning EPENDING, e.g: due to waiting for TURN allocations, the library may reject the call if the media init fails, should we invoke the callback too?

This is an interesting scenario indeed. And note that on_incoming_call() has been called for such case, whereas for other cases, it hasn't.

Since the on_incoming_call() has been called, the new callback (on_rejected_incoming_call()) should not be called again. Another case would be if the called is answered with 200OK, however there's an issue (e.g: media init) and the call is rejected. The callback would not be called, however app still be informed from on_call_state().

Make sure the behavior is consistent on pjsua2 as well, since on pjsua2 onIncomingCall() is called earlier.

As it is async, I guess there can be race between on_incoming_call() and on_incoming_call_med_tp_complete(), in other word, we cannot be sure whether on_incoming_call() has been called?

Moreover, the purpose of the callback is to notify app about the call rejection by lib (e.g: so app can stop the incoming call UI), I guess it is better to still call the callback even when on_incoming_call() has been called. Btw, if rx_data is the problem (add complexity for example), IMHO we can set it to NULL for now for this scenario (notifying app is the priority).

pjsip/src/pjsua-lib/pjsua_call.c

…ode to using defined/enum error for uniformity

sauwming · 2023-09-27T07:47:51Z

Sorry for rather late comment. But maybe we should consider adding call_id to the callback? So app can easily tell: - which call is getting internally rejected by the library, and - whether on_incoming_call() has ever been called (I suppose if call_id == PJSUA_INVALID_ID it means that app has not been notified of such call before).

sauwming · 2023-10-03T03:39:30Z

looks okay.

sauwming · 2023-10-03T04:02:06Z

Just a few additional notes:
The last commit won't be able to help app know if the callback on_incoming_call() has been called.
And I still have some reservations about whether there can be any issue caused by the overlap between on_call_state(DISCONNECTED) and on_rejected_incoming_call().

But I think the PR can be merged first for now.

If there's any feedback @anikitin-intermedia, please feel free to inform us.

Add on_rejected_incoming_call() callback

2fa315e

trengginas added the component: pjsua-lib label Sep 4, 2023

trengginas added this to the release-2.14 milestone Sep 4, 2023

trengginas requested review from sauwming and nanangizz September 4, 2023 16:33

trengginas self-assigned this Sep 4, 2023

anikitin-intermedia reviewed Sep 5, 2023

View reviewed changes

pjsip/include/pjsua-lib/pjsua_internal.h Outdated Show resolved Hide resolved

nanangizz reviewed Sep 5, 2023

View reviewed changes

pjsip/include/pjsua-lib/pjsua.h Outdated Show resolved Hide resolved

pjsip/src/pjsua-lib/pjsua_call.c Outdated Show resolved Hide resolved

trengginas added 2 commits September 9, 2023 16:47

Modification based on comments

c780fe7

- save incoming rdata to be passed as callback parameter

Fix failed unit test

dc72374

Call the callback from pjsua_call_on_incoming()

c8bde05

wosrediinanatour reviewed Sep 14, 2023

View reviewed changes

pjsip/src/pjsua-lib/pjsua_call.c Outdated Show resolved Hide resolved

Remove tdata from callback param

e74e036

sauwming reviewed Sep 18, 2023

View reviewed changes

Modification based on comments

b4565c3

nanangizz reviewed Sep 19, 2023

View reviewed changes

Modification based on comments

c2ff154

nanangizz approved these changes Sep 25, 2023

View reviewed changes

Add doc

f7e28fc

sauwming approved these changes Sep 25, 2023

View reviewed changes

wosrediinanatour reviewed Sep 25, 2023

View reviewed changes

pjsip/src/pjsua-lib/pjsua_call.c Show resolved Hide resolved

Fix build error on C89 compiler (e.g: VS2005) and change the status c…

eb77f1f

…ode to using defined/enum error for uniformity

Add call_id to the callback param

70a3e2e

trengginas merged commit be0c6c4 into master Oct 3, 2023
34 checks passed

sauwming mentioned this pull request Dec 26, 2023

[PJSUA] onIncomingCall never gets called, call hangs #3807

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add on_rejected_incoming_call() callback #3683

Add on_rejected_incoming_call() callback #3683

trengginas commented Sep 4, 2023

wosrediinanatour commented Sep 5, 2023

sauwming commented Sep 13, 2023

sauwming commented Sep 15, 2023

trengginas commented Sep 15, 2023 •

edited

Loading

wosrediinanatour commented Sep 15, 2023

sauwming Sep 18, 2023

sauwming Sep 18, 2023

sauwming Sep 25, 2023

trengginas Sep 25, 2023

sauwming commented Sep 18, 2023

wosrediinanatour commented Sep 18, 2023

nanangizz Sep 19, 2023

trengginas Sep 21, 2023

nanangizz Sep 22, 2023

nanangizz Sep 19, 2023

sauwming Sep 19, 2023

trengginas Sep 21, 2023

sauwming Sep 21, 2023

nanangizz Sep 22, 2023 •

edited

Loading

sauwming commented Sep 27, 2023 •

edited

Loading

sauwming commented Oct 3, 2023 •

edited

Loading

sauwming commented Oct 3, 2023

Add on_rejected_incoming_call() callback #3683

Add on_rejected_incoming_call() callback #3683

Conversation

trengginas commented Sep 4, 2023

wosrediinanatour commented Sep 5, 2023

sauwming commented Sep 13, 2023

sauwming commented Sep 15, 2023

trengginas commented Sep 15, 2023 • edited Loading

wosrediinanatour commented Sep 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sauwming commented Sep 18, 2023

wosrediinanatour commented Sep 18, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nanangizz Sep 22, 2023 • edited Loading

Choose a reason for hiding this comment

sauwming commented Sep 27, 2023 • edited Loading

sauwming commented Oct 3, 2023 • edited Loading

sauwming commented Oct 3, 2023

trengginas commented Sep 15, 2023 •

edited

Loading

nanangizz Sep 22, 2023 •

edited

Loading

sauwming commented Sep 27, 2023 •

edited

Loading

sauwming commented Oct 3, 2023 •

edited

Loading