Skip to content
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 26 additions & 2 deletions src/plugins/janus_sip.c
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@
"force_tcp" : <true|false; if true, forces TCP for the SIP messaging; optional>,
"sips" : <true|false; if true, configures a SIPS URI too when registering; optional>,
"rfc2543_cancel" : <true|false; if true, configures sip client to CANCEL pending INVITEs without having received a provisional response first; optional>,
"automatic_ringing" : <true|false; if false, don't generate ringing automatically as soon as an INVITE; optional>,
"username" : "<SIP URI to register; mandatory>",
"secret" : "<password to use to register; optional>",
"ha1_secret" : "<prehashed password to use to register; optional>",
Expand Down Expand Up @@ -808,6 +809,7 @@ static struct janus_json_parameter register_parameters[] = {
{"force_tcp", JANUS_JSON_BOOL, 0},
{"sips", JANUS_JSON_BOOL, 0},
{"rfc2543_cancel", JANUS_JSON_BOOL, 0},
{"automatic_ringing", JANUS_JSON_BOOL, 0},
{"username", JSON_STRING, JANUS_JSON_PARAM_REQUIRED},
{"secret", JSON_STRING, 0},
{"ha1_secret", JSON_STRING, 0},
Expand Down Expand Up @@ -1047,6 +1049,7 @@ typedef struct janus_sip_account {
gboolean force_tcp;
gboolean sips;
gboolean rfc2543_cancel;
gboolean automatic_ringing;
char *username;
char *display_name; /* Used for outgoing calls in the From header */
char *authuser; /**< username to use for authentication */
Expand Down Expand Up @@ -2323,6 +2326,7 @@ void janus_sip_create_session(janus_plugin_session *handle, int *error) {
session->account.force_tcp = FALSE;
session->account.sips = FALSE;
session->account.rfc2543_cancel = FALSE;
session->account.automatic_ringing = TRUE;
session->account.username = NULL;
session->account.display_name = NULL;
session->account.user_agent = NULL;
Expand Down Expand Up @@ -3133,6 +3137,11 @@ static void *janus_sip_handler(void *data) {
if(do_rfc2543_cancel != NULL) {
rfc2543_cancel = json_is_true(do_rfc2543_cancel);
}
gboolean automatic_ringing = TRUE;
json_t *do_automatic_ringing = json_object_get(root, "automatic_ringing");
if(do_automatic_ringing != NULL) {
automatic_ringing = json_is_true(do_automatic_ringing);
}

/* Parse addresses */
json_t *proxy = json_object_get(root, "proxy");
Expand Down Expand Up @@ -3290,6 +3299,7 @@ static void *janus_sip_handler(void *data) {
session->account.force_tcp = FALSE;
session->account.sips = FALSE;
session->account.rfc2543_cancel = FALSE;
session->account.automatic_ringing = TRUE;
if(session->account.username != NULL)
g_free(session->account.username);
session->account.username = NULL;
Expand Down Expand Up @@ -3322,6 +3332,7 @@ static void *janus_sip_handler(void *data) {
session->account.force_tcp = force_tcp;
session->account.sips = sips;
session->account.rfc2543_cancel = rfc2543_cancel;
session->account.automatic_ringing = automatic_ringing;
session->account.username = g_strdup(user_id);
session->account.authuser = g_strdup(authuser_text ? authuser_text : user_id);
session->account.secret = secret_text ? g_strdup(secret_text) : NULL;
Expand Down Expand Up @@ -4947,6 +4958,16 @@ static void *janus_sip_handler(void *data) {
/* Notify the operation */
result = json_object();
json_object_set_new(result, "event", json_string("infosent"));
} else if(!strcasecmp(request_text, "send_ringing")) {
if(session->status != janus_sip_call_status_invited
&& session->status != janus_sip_call_status_progress) {
Comment thread
adnanel marked this conversation as resolved.
Outdated
JANUS_LOG(LOG_ERR, "Wrong state (not invited or progress? status=%s)\n", janus_sip_call_status_string(session->status));
g_snprintf(error_cause, 512, "Wrong state (not in a call?)");
goto error;
}
if (session->stack->s_nh_i) {
Comment thread
adnanel marked this conversation as resolved.
Outdated
nua_respond(session->stack->s_nh_i, 180, sip_status_phrase(180), TAG_END());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have one small doubt: shouldn't this part also check whether automatic ringing is enabled, and/or whether or not we already sent a 180 back? Not sure what happens in the SIP world if you send a 180 twice or more.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I had the same concern about state corruption if you do this multiple times. Frankly I'm no SIP expert but to my knowledge (based on quick google searching RFCs), it's specified that multiple progress messages may be sent freely:

A UAS MAY
send as many provisional responses as it likes. Each of these MUST
indicate the same dialog ID. However, these will not be delivered
reliably.

https://www.rfc-editor.org/rfc/rfc3261.html#section-13.3.1.1

For the first question - the same rationale as above, having automatic ringing doesn't mean someone wouldn't want to send multiple progresses later on - for what purpose I do not know, but I figured I Janus shouldn't be too opinionated in this regard.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also regarding your previous comment

The only doubt I have is related to the send_ringing request: I was wondering it it may make sense to have a generic method for sending generic error codes back, but thinking about it it would probably be a mess

tbh the main reason why I didn't add it is because I see no use for it. And again going back to me being a SIP novice, I wouldn't want to miss any validation that a generic API would require that I'm completely oblivious about lol.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, thanks for the feedback! I think this is good to merge, so I'll go ahead, and see if this is easy to backport to 0.x too. Thanks again for your contribution!

As a side and unrelated note, not sure if you had the chance to look at #3514: as a regular user of the SIP plugin, I'd be interested in feedback on that part (even though it may not be needed in your scenarios).

}
} else if(!strcasecmp(request_text, "message")) {
/* Send a SIP MESSAGE request: we'll only need the content and optional payload type */
JANUS_VALIDATE_JSON_OBJECT(root, sipmessage_parameters,
Expand Down Expand Up @@ -5644,8 +5665,10 @@ void janus_sip_sofia_callback(nua_event_t event, int status, char const *phrase,
su_free(session->stack->s_home, callee_text);
g_free(referred_by);
if(!reinvite) {
/* Send a Ringing back */
nua_respond(nh, 180, sip_status_phrase(180), TAG_END());
if (session->account.automatic_ringing) {
Comment thread
adnanel marked this conversation as resolved.
Outdated
/* Send a Ringing back */
nua_respond(nh, 180, sip_status_phrase(180), TAG_END());
}
session->stack->s_nh_i = nh;
}
break;
Expand Down Expand Up @@ -6404,6 +6427,7 @@ void janus_sip_sofia_callback(nua_event_t event, int status, char const *phrase,
session->account.force_tcp = FALSE;
session->account.sips = FALSE;
session->account.rfc2543_cancel = FALSE;
session->account.automatic_ringing = TRUE;
if(session->account.username != NULL)
g_free(session->account.username);
session->account.username = NULL;
Expand Down