Skip to content

Commit

Permalink
Add JSEP flag to invert processing order of rid in SDP (#2385)
Browse files Browse the repository at this point in the history
  • Loading branch information
@lminiero
lminiero authored Oct 8, 2020
1 parent 3cfdf6f commit c07a727
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 13 deletions.
2 changes: 2 additions & 0 deletions ice.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,8 @@ struct janus_ice_stream {
guint32 video_ssrc_peer_rtx[3], video_ssrc_peer_rtx_new[3], video_ssrc_peer_rtx_orig[3];
/*! \brief Array of RTP Stream IDs (for Firefox simulcasting, if enabled) */
char *rid[3];
/*! \brief Whether the order of the rids in the SDP will be h-m-l (TRUE) or l-m-h (FALSE) */
gboolean rids_hml;
/*! \brief Whether we should use the legacy simulcast syntax (a=simulcast:recv rid=..) or the proper one (a=simulcast:recv ..) */
gboolean legacy_rid;
/*! \brief RTP switching context(s) in case of renegotiations (audio+video and/or simulcast) */
Expand Down
21 changes: 19 additions & 2 deletions janus.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ static struct janus_json_parameter jsep_parameters[] = {
{"type", JSON_STRING, JANUS_JSON_PARAM_REQUIRED},
{"sdp", JSON_STRING, JANUS_JSON_PARAM_REQUIRED},
{"trickle", JANUS_JSON_BOOL, 0},
{"rid_order", JSON_STRING, 0},
{"e2ee", JANUS_JSON_BOOL, 0}
};
static struct janus_json_parameter add_token_parameters[] = {
Expand Down Expand Up @@ -1314,6 +1315,21 @@ int janus_process_incoming_request(janus_request *request) {
type = NULL;
json_t *jsep_trickle = json_object_get(jsep, "trickle");
gboolean do_trickle = jsep_trickle ? json_is_true(jsep_trickle) : TRUE;
json_t *jsep_rids = json_object_get(jsep, "rid_order");
gboolean rids_hml = TRUE;
if(jsep_rids != NULL) {
const char *jsep_rids_value = json_string_value(jsep_rids);
if(jsep_rids_value != NULL) {
if(!strcasecmp(jsep_rids_value, "hml")) {
rids_hml = TRUE;
} else if(!strcasecmp(jsep_rids_value, "lmh")) {
rids_hml = FALSE;
} else {
JANUS_LOG(LOG_WARN, "[%"SCNu64"] Invalid 'rid_order' value, falling back to 'hml'\n", handle->handle_id);
}
}
json_object_del(jsep, "rid_order");
}
json_t *jsep_e2ee = json_object_get(jsep, "e2ee");
gboolean e2ee = jsep_e2ee ? json_is_true(jsep_e2ee) : FALSE;
/* Are we still cleaning up from a previous media session? */
Expand Down Expand Up @@ -1420,7 +1436,7 @@ int janus_process_incoming_request(janus_request *request) {
goto jsondone;
}
}
if(janus_sdp_process(handle, parsed_sdp, FALSE) < 0) {
if(janus_sdp_process(handle, parsed_sdp, rids_hml, FALSE) < 0) {
JANUS_LOG(LOG_ERR, "Error processing SDP\n");
janus_sdp_destroy(parsed_sdp);
g_free(jsep_type);
Expand Down Expand Up @@ -1458,7 +1474,7 @@ int janus_process_incoming_request(janus_request *request) {
/* FIXME This is a renegotiation: we can currently only handle simple changes in media
* direction and ICE restarts: anything more complex than that will result in an error */
JANUS_LOG(LOG_INFO, "[%"SCNu64"] Negotiation update, checking what changed...\n", handle->handle_id);
if(janus_sdp_process(handle, parsed_sdp, TRUE) < 0) {
if(janus_sdp_process(handle, parsed_sdp, rids_hml, TRUE) < 0) {
JANUS_LOG(LOG_ERR, "Error processing SDP\n");
janus_sdp_destroy(parsed_sdp);
g_free(jsep_type);
Expand Down Expand Up @@ -2915,6 +2931,7 @@ json_t *janus_admin_stream_summary(janus_ice_stream *stream) {
json_object_set_new(sr, "rid-ext-id", json_integer(stream->rid_ext_id));
if(stream->ridrtx_ext_id > 0)
json_object_set_new(sr, "ridrtx-ext-id", json_integer(stream->ridrtx_ext_id));
json_object_set_new(sr, "rid-order", json_string(stream->rids_hml ? "hml" : "lmh"));
if(stream->legacy_rid)
json_object_set_new(sr, "rid-syntax", json_string("legacy"));
json_object_set_new(s, "rid-simulcast", sr);
Expand Down
22 changes: 12 additions & 10 deletions sdp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ janus_sdp *janus_sdp_preparse(void *ice_handle, const char *jsep_sdp, char *erro
}

/* Parse SDP */
int janus_sdp_process(void *ice_handle, janus_sdp *remote_sdp, gboolean update) {
int janus_sdp_process(void *ice_handle, janus_sdp *remote_sdp, gboolean rids_hml, gboolean update) {
if(!ice_handle || !remote_sdp)
return -1;
janus_ice_handle *handle = (janus_ice_handle *)ice_handle;
Expand Down Expand Up @@ -418,6 +418,7 @@ int janus_sdp_process(void *ice_handle, janus_sdp *remote_sdp, gboolean update)
}
/* Is simulcasting enabled, using rid? (we need to check this before parsing SSRCs) */
tempA = m->attributes;
stream->rids_hml = rids_hml;
while(tempA) {
janus_sdp_attribute *a = (janus_sdp_attribute *)tempA->data;
if(a->name && !strcasecmp(a->name, "rid") && a->value) {
Expand All @@ -427,12 +428,12 @@ int janus_sdp_process(void *ice_handle, janus_sdp *remote_sdp, gboolean update)
JANUS_LOG(LOG_ERR, "[%"SCNu64"] Failed to parse rid attribute...\n", handle->handle_id);
} else {
JANUS_LOG(LOG_VERB, "[%"SCNu64"] Parsed rid: %s\n", handle->handle_id, rid);
if(stream->rid[0] == NULL) {
stream->rid[0] = g_strdup(rid);
if(stream->rid[rids_hml ? 0 : 2] == NULL) {
stream->rid[rids_hml ? 0 : 2] = g_strdup(rid);
} else if(stream->rid[1] == NULL) {
stream->rid[1] = g_strdup(rid);
} else if(stream->rid[2] == NULL) {
stream->rid[2] = g_strdup(rid);
} else if(stream->rid[rids_hml ? 2 : 0] == NULL) {
stream->rid[rids_hml ? 2 : 0] = g_strdup(rid);
} else {
JANUS_LOG(LOG_WARN, "[%"SCNu64"] Too many RTP Stream IDs, ignoring '%s'...\n", handle->handle_id, rid);
}
Expand Down Expand Up @@ -1494,17 +1495,18 @@ char *janus_sdp_merge(void *ice_handle, janus_sdp *anon, gboolean offer) {
if(m->type == JANUS_SDP_VIDEO && stream->rid[0] != NULL) {
char rids[50];
rids[0] = '\0';
int i=0;
int i=0, index=0;
for(i=0; i<3; i++) {
if(stream->rid[i] == NULL)
index = (stream->rids_hml ? i : (2-i));
if(stream->rid[index] == NULL)
continue;
a = janus_sdp_attribute_create("rid", "%s recv", stream->rid[i]);
a = janus_sdp_attribute_create("rid", "%s recv", stream->rid[index]);
m->attributes = g_list_append(m->attributes, a);
if(strlen(rids) == 0) {
g_strlcat(rids, stream->rid[i], sizeof(rids));
g_strlcat(rids, stream->rid[index], sizeof(rids));
} else {
g_strlcat(rids, ";", sizeof(rids));
g_strlcat(rids, stream->rid[i], sizeof(rids));
g_strlcat(rids, stream->rid[index], sizeof(rids));
}
}
if(stream->legacy_rid) {
Expand Down
3 changes: 2 additions & 1 deletion sdp.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,10 @@ janus_sdp *janus_sdp_preparse(void *handle, const char *jsep_sdp, char *error_st
* and supporting multiple streams in the same PeerConnection are still WIP.
* @param[in] handle Opaque pointer to the ICE handle this session description will modify
* @param[in] sdp The Janus SDP object to process
* @param[in] rids_hml Whether the order of rids in the SDP, if present, will be h-m-l (TRUE) or l-m-h (FALSE)
* @param[in] update Whether this SDP is an update to an existing session or not
* @returns 0 in case of success, -1 in case of an error */
int janus_sdp_process(void *handle, janus_sdp *sdp, gboolean update);
int janus_sdp_process(void *handle, janus_sdp *sdp, gboolean rids_hml, gboolean update);

/*! \brief Method to parse a single candidate
* \details This method will parse a single remote candidate provided by a peer, whether it is trickling or not
Expand Down

0 comments on commit c07a727

Please sign in to comment.