Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for "progress" request on SIP Plugin #3466

Merged
merged 33 commits into from
Nov 25, 2024
Merged

Add support for "progress" request on SIP Plugin #3466

Changes from 30 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
56a7968
Add pre_accept status
adnan-mujagic Oct 24, 2024
7caf8d2
Print what's left in the buffer when destroying the logger (see #3462)
lminiero Oct 24, 2024
17aee9c
Change the way call-IDs are tracked in the SIP plugin (fixes #3404) (…
lminiero Oct 24, 2024
3874d57
Fix if
adnan-mujagic Oct 24, 2024
8d12e89
Set update to true if previously pre-accepted
adnan-mujagic Oct 24, 2024
0f58984
Rename status and request
adnan-mujagic Oct 25, 2024
937bde9
Merge branch 'meetecho:master' into pre_accept_PoC
adnan-mujagic Oct 25, 2024
7881cbc
Change comments and logs
adnan-mujagic Oct 25, 2024
0199f56
Merge branch 'pre_accept_PoC' of https://github.com/adnan-mujagic/jan…
adnan-mujagic Oct 25, 2024
2987975
Progress parameters
adnan-mujagic Oct 25, 2024
1654c7f
Change logs
adnan-mujagic Oct 25, 2024
29ac024
Merge progress and accept into a single if statement
adnan-mujagic Oct 25, 2024
cb65d97
Fix error
adnan-mujagic Oct 25, 2024
2754814
Add if, else
adnan-mujagic Oct 25, 2024
48b9ec7
Merge pull request #1 from adnan-mujagic/fix/fix-build-issue
adnan-mujagic Oct 25, 2024
979d5fb
Fix wrong event
adnan-mujagic Oct 25, 2024
806e090
Remove empty line
adnan-mujagic Oct 25, 2024
3d2ddd8
Merge pull request #2 from adnan-mujagic/fix-wrong-event
adnan-mujagic Oct 25, 2024
46b903a
Add some TODOs to check expected behaviour
adnan-mujagic Oct 28, 2024
2f42d03
Additional checks
adnan-mujagic Oct 29, 2024
a116cee
Merge branch 'meetecho:master' into additional-checks
adnan-mujagic Oct 29, 2024
06641ec
Merge branch 'meetecho:master' into pre_accept_PoC
adnan-mujagic Oct 30, 2024
33f7fad
Remove extra lines
adnan-mujagic Oct 30, 2024
dcc2ce5
Add more check
adnan-mujagic Oct 30, 2024
36efbb9
Remove behaviour for call in progress
adnan-mujagic Oct 30, 2024
8b7e65f
Enable declining the call in progress state
adnan-mujagic Oct 30, 2024
81ded68
Remove todos
adnan-mujagic Oct 31, 2024
f52a935
Add some logs
adnan-mujagic Oct 31, 2024
1462b07
Remove extra logs
adnan-mujagic Oct 31, 2024
60f0a89
Merge pull request #3 from adnan-mujagic/additional-checks
adnan-mujagic Oct 31, 2024
e5bf910
Use ternary operator (#4)
adnan-mujagic Oct 31, 2024
e209d77
Document progress request (#5)
adnan-mujagic Nov 13, 2024
482aca2
Merge branch 'meetecho:master' into pre_accept_PoC
adnan-mujagic Nov 13, 2024
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
75 changes: 53 additions & 22 deletions src/plugins/janus_sip.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,11 @@ static struct janus_json_parameter accept_parameters[] = {
{"headers", JSON_OBJECT, 0},
{"autoaccept_reinvites", JANUS_JSON_BOOL, 0}
};
static struct janus_json_parameter progress_parameters[] = {
{"srtp", JSON_STRING, 0},
{"headers", JSON_OBJECT, 0},
{"autoaccept_reinvites", JANUS_JSON_BOOL, 0}
};
static struct janus_json_parameter decline_parameters[] = {
{"code", JANUS_JSON_INTEGER, 0},
{"headers", JSON_OBJECT, 0},
Expand Down Expand Up @@ -920,6 +925,7 @@ typedef enum {
janus_sip_call_status_idle = 0,
janus_sip_call_status_inviting,
janus_sip_call_status_invited,
janus_sip_call_status_progress,
janus_sip_call_status_incall,
janus_sip_call_status_incall_reinviting,
janus_sip_call_status_incall_reinvited,
Expand All @@ -934,6 +940,8 @@ static const char *janus_sip_call_status_string(janus_sip_call_status status) {
return "inviting";
case janus_sip_call_status_invited:
return "invited";
case janus_sip_call_status_progress:
return "progress";
case janus_sip_call_status_incall:
return "incall";
case janus_sip_call_status_incall_reinviting:
Expand Down Expand Up @@ -2551,7 +2559,7 @@ void janus_sip_incoming_rtp(janus_plugin_session *handle, janus_plugin_rtp *pack
JANUS_LOG(LOG_ERR, "No session associated with this handle...\n");
return;
}
if(!janus_sip_call_is_established(session))
if(!janus_sip_call_is_established(session) && session->status != janus_sip_call_status_progress)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just to make it a bit easier to review, in the latest changes I added janus_sip_call_status_progress as a valid status for

  • janus_sip_incoming_rtp method
  • janus_sip_incoming_rtcp method
  • decline request
  • hangup request
  • record request (because of the presumption it makes sense to start recording with early media that was already there)

return;
gboolean video = packet->video;
char *buf = packet->buffer;
Expand Down Expand Up @@ -2679,7 +2687,7 @@ void janus_sip_incoming_rtcp(janus_plugin_session *handle, janus_plugin_rtcp *pa
JANUS_LOG(LOG_ERR, "No session associated with this handle...\n");
return;
}
if(!janus_sip_call_is_established(session))
if(!janus_sip_call_is_established(session) && session->status != janus_sip_call_status_progress)
return;
gboolean video = packet->video;
char *buf = packet->buffer;
Expand Down Expand Up @@ -3937,13 +3945,20 @@ static void *janus_sip_handler(void *data) {
result = json_object();
json_object_set_new(result, "event", json_string("calling"));
json_object_set_new(result, "call_id", json_string(session->callid));
} else if(!strcasecmp(request_text, "accept")) {
if(session->status != janus_sip_call_status_invited) {
} else if(!strcasecmp(request_text, "accept") || !strcasecmp(request_text, "progress")) {
gboolean progress = !strcasecmp(request_text, "progress");
if(progress && session->status != janus_sip_call_status_invited) {
JANUS_LOG(LOG_ERR, "Wrong state (not invited? status=%s)\n", janus_sip_call_status_string(session->status));
error_code = JANUS_SIP_ERROR_WRONG_STATE;
g_snprintf(error_cause, 512, "Wrong state (not invited? status=%s)", janus_sip_call_status_string(session->status));
goto error;
}
if(!progress && session->status != janus_sip_call_status_invited && session->status != janus_sip_call_status_progress) {
JANUS_LOG(LOG_ERR, "Wrong state (not invited or progress? status=%s)\n", janus_sip_call_status_string(session->status));
error_code = JANUS_SIP_ERROR_WRONG_STATE;
g_snprintf(error_cause, 512, "Wrong state (not invited or progress? status=%s)", janus_sip_call_status_string(session->status));
goto error;
}
janus_mutex_lock(&session->mutex);
if(session->callee == NULL) {
janus_mutex_unlock(&session->mutex);
Expand All @@ -3953,9 +3968,15 @@ static void *janus_sip_handler(void *data) {
goto error;
}
janus_mutex_unlock(&session->mutex);
JANUS_VALIDATE_JSON_OBJECT(root, accept_parameters,
error_code, error_cause, TRUE,
JANUS_SIP_ERROR_MISSING_ELEMENT, JANUS_SIP_ERROR_INVALID_ELEMENT);
if(progress) {
JANUS_VALIDATE_JSON_OBJECT(root, progress_parameters,
error_code, error_cause, TRUE,
JANUS_SIP_ERROR_MISSING_ELEMENT, JANUS_SIP_ERROR_INVALID_ELEMENT);
} else {
JANUS_VALIDATE_JSON_OBJECT(root, accept_parameters,
error_code, error_cause, TRUE,
JANUS_SIP_ERROR_MISSING_ELEMENT, JANUS_SIP_ERROR_INVALID_ELEMENT);
}
Copy link
Member

Choose a reason for hiding this comment

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

This can probably be a ternary, rather than an if/else, e.g.

JANUS_VALIDATE_JSON_OBJECT(root, (progress ? progress_parameters : accept_parameters),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Heyy, just wanted to give a small update here, I initially tried to do this using a ternary but was getting errors and it took more effort to fix than to rewrite using the if/else.

Regarding the statuses, I am checking the places wherejanus_sip_call_status_incall is used and the janus_sip_call_is_established() function, will be working on this in the next few days to try and decide where we should take the new state into account as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Small update here: you were right, I should have just copy pasted your suggestion and it would have worked.

However, correct me if I am wrong, but since this JANUS_VALIDATE_JSON_OBJECT is defined as a macro, passing in an expression such as progress ? progress_parameters : accept_parameters would insert this expression wherever that parameter is used inside the macro, which is an unnecessary overhead.

The way I have changed it now is that first we compute the desired params using a ternary operator, then pass the result as a parameter to the macro.

if(error_code != 0)
goto error;
json_t *srtp = json_object_get(root, "srtp");
Expand All @@ -3982,9 +4003,9 @@ static void *janus_sip_handler(void *data) {
if(session->media.has_video)
has_srtp = (has_srtp && session->media.has_srtp_remote_video);
if(session->media.require_srtp && !has_srtp) {
JANUS_LOG(LOG_ERR, "Can't accept the call: SDES-SRTP required, but caller didn't offer it\n");
JANUS_LOG(LOG_ERR, "Can't %s the call: SDES-SRTP required, but caller didn't offer it\n", progress ? "progress" : "accept");
error_code = JANUS_SIP_ERROR_TOO_STRICT;
g_snprintf(error_cause, 512, "Can't accept the call: SDES-SRTP required, but caller didn't offer it");
g_snprintf(error_cause, 512, "Can't %s the call: SDES-SRTP required, but caller didn't offer it", progress ? "progress" : "accept");
goto error;
}
answer_srtp = answer_srtp || session->media.has_srtp_remote_audio || session->media.has_srtp_remote_video;
Expand All @@ -4006,8 +4027,8 @@ static void *janus_sip_handler(void *data) {
g_snprintf(error_cause, 512, "Media encryption unsupported by this plugin");
goto error;
}
/* Accept a call from another peer */
JANUS_LOG(LOG_VERB, "We're accepting the call from %s\n", session->callee);
/* Accept/Progress a call from another peer */
JANUS_LOG(LOG_VERB, "We're %s the call from %s\n", progress ? "progressing" : "accepting", session->callee);
gboolean answer = !strcasecmp(msg_sdp_type, "answer");
if(!answer) {
JANUS_LOG(LOG_VERB, "This is a response to an offerless INVITE\n");
Expand Down Expand Up @@ -4042,7 +4063,7 @@ static void *janus_sip_handler(void *data) {
session->media.has_video = TRUE; /* FIXME Maybe we need a better way to signal this */
}
janus_mutex_lock(&session->mutex);
if(janus_sip_allocate_local_ports(session, FALSE) < 0) {
if(janus_sip_allocate_local_ports(session, session->status == janus_sip_call_status_progress ? TRUE : FALSE) < 0) {
janus_mutex_unlock(&session->mutex);
JANUS_LOG(LOG_ERR, "Could not allocate RTP/RTCP ports\n");
janus_sdp_destroy(parsed_sdp);
Expand Down Expand Up @@ -4070,7 +4091,7 @@ static void *janus_sip_handler(void *data) {
/* Take note of the SDP (may be useful for UPDATEs or re-INVITEs) */
janus_sdp_destroy(session->sdp);
session->sdp = parsed_sdp;
JANUS_LOG(LOG_VERB, "Prepared SDP for 200 OK:\n%s", sdp);
JANUS_LOG(LOG_VERB, "Prepared SDP for %s:\n%s", progress ? "183 Session Progress" : "200 OK", sdp);
/* If the user negotiated simulcasting, just stick with the base substream */
json_t *msg_simulcast = json_object_get(msg->jsep, "simulcast");
if(msg_simulcast) {
Expand All @@ -4079,30 +4100,39 @@ static void *janus_sip_handler(void *data) {
if(s && json_array_size(s) > 0)
session->media.simulcast_ssrc = json_integer_value(json_array_get(s, 0));
}
const char *event_value;
if(progress) {
event_value = "progressed";
} else if(answer) {
event_value = "accepted";
} else {
event_value = "accepting";
}
/* Also notify event handlers */
if(notify_events && gateway->events_is_enabled()) {
json_t *info = json_object();
json_object_set_new(info, "event", json_string(answer ? "accepted" : "accepting"));
json_object_set_new(info, "event", json_string(event_value));
if(session->callid)
json_object_set_new(info, "call-id", json_string(session->callid));
gateway->notify_event(&janus_sip_plugin, session->handle, info);
}
/* Check if the OK needs to be enriched with custom headers */
char custom_headers[2048];
janus_sip_parse_custom_headers(root, (char *)&custom_headers, sizeof(custom_headers));
/* Send 200 OK */
/* Send 200 OK/183 Session progress */
if(!answer) {
if(session->transaction)
g_free(session->transaction);
session->transaction = msg->transaction ? g_strdup(msg->transaction) : NULL;
}
g_atomic_int_set(&session->hangingup, 0);
janus_sip_call_update_status(session, janus_sip_call_status_incall);
janus_sip_call_update_status(session, progress ? janus_sip_call_status_progress : janus_sip_call_status_incall);
if(session->stack->s_nh_i == NULL) {
JANUS_LOG(LOG_WARN, "NUA Handle for 200 OK still null??\n");
JANUS_LOG(LOG_WARN, "NUA Handle for %s null\n", progress ? "183 Session Progress" : "200 OK");
}
int sip_response = progress ? 183 : 200;
nua_respond(session->stack->s_nh_i,
200, sip_status_phrase(200),
sip_response, sip_status_phrase(sip_response),
SOATAG_USER_SDP_STR(sdp),
SOATAG_RTP_SELECT(SOA_RTP_SELECT_COMMON),
NUTAG_AUTOANSWER(0),
Expand All @@ -4112,7 +4142,7 @@ static void *janus_sip_handler(void *data) {
g_free(sdp);
/* Send an ack back */
result = json_object();
json_object_set_new(result, "event", json_string(answer ? "accepted" : "accepting"));
json_object_set_new(result, "event", json_string(event_value));
if(answer) {
/* Start the media */
session->media.ready = TRUE; /* FIXME Maybe we need a better way to signal this */
Expand Down Expand Up @@ -4349,7 +4379,7 @@ static void *janus_sip_handler(void *data) {
}
}
/* Reject an incoming call */
if(session->status != janus_sip_call_status_invited) {
if(session->status != janus_sip_call_status_invited && session->status != janus_sip_call_status_progress) {
JANUS_LOG(LOG_ERR, "Wrong state (not invited? status=%s)\n", janus_sip_call_status_string(session->status));
/* Ignore */
janus_sip_message_free(msg);
Expand Down Expand Up @@ -4593,8 +4623,8 @@ static void *janus_sip_handler(void *data) {
json_object_set_new(result, "event", json_string(hold ? "holding" : "resuming"));
} else if(!strcasecmp(request_text, "hangup")) {
/* Hangup an ongoing call */
if(!janus_sip_call_is_established(session) && session->status != janus_sip_call_status_inviting) {
JANUS_LOG(LOG_ERR, "Wrong state (not established/inviting? status=%s)\n",
if(!janus_sip_call_is_established(session) && session->status != janus_sip_call_status_inviting && session->status != janus_sip_call_status_progress) {
JANUS_LOG(LOG_ERR, "Wrong state (not established/inviting/progress? status=%s)\n",
janus_sip_call_status_string(session->status));
/* Ignore */
janus_sip_message_free(msg);
Expand Down Expand Up @@ -4630,6 +4660,7 @@ static void *janus_sip_handler(void *data) {
} else if(!strcasecmp(request_text, "recording")) {
/* Start or stop recording */
if(!(session->status == janus_sip_call_status_inviting || /* Presume it makes sense to start recording with early media? */
session->status == janus_sip_call_status_progress ||
janus_sip_call_is_established(session))) {
JANUS_LOG(LOG_ERR, "Wrong state (not in a call? status=%s)\n", janus_sip_call_status_string(session->status));
g_snprintf(error_cause, 512, "Wrong state (not in a call?)");
Expand Down