Sdp-utils: janus_sdp_get_video_profile() function > I think exception handling is needed

General
1

(My english is not that good.)
I am deeply grateful for Janus.

Exception handling seems necessary when extracting the profile from the following fmtp string if there is a space after the semicolon (;).

vfmtp ex)
blank_x = "packetization-mode=1;profile-level-id=4D401F;sprop-parameter-sets=SPS,PPS";
blank_o = "packetization-mode=1; profile-level-id=4D401F; sprop-parameter-sets=SPS,PPS"

test 1) janus_sdp_get_video_profile(JANUS_VIDEOCODEC_H264, blank_x)
--> result is "4D401F"

test 2) janus_sdp_get_video_profile(JANUS_VIDEOCODEC_H264, blank_o);
--> result is "=4D401F"

src/sdp-utils.c

char *janus_sdp_get_video_profile(janus_videocodec codec, const char *fmtp) {
	...
	while(index != NULL) {
		if(strstr(index, needle) != NULL) {
			profile = index + strlen(needle); <- *ADD WHITESPACE LENGTH*
			if(strlen(profile) > 0)
				profile = g_strdup(profile);
			else
				profile = NULL;
			break;
		}
		...
	}
	...
}
2

You’re right that the parsing could be improved, but I don’t think there’s an easy way to figure out that whitespace length.

1 Like
3

lorenzo, Thanks for replying.
Happy New Year!

(I believe you’ve already looked at the janus_sdp_get_video_profile() function.)
Our thoughts may differ, but for now, since we need an appropriate profile-level-id value, we’re using it with the following modification.

I regularly check for code updates and will see what you think about them.

char *janus_sdp_get_video_profile(janus_videocodec codec, const char *fmtp) {
	...
	const char *needle = NULL;
	if(codec == JANUS_VIDEOCODEC_H264) {
		needle = "profile-level-id=";
	...
	while(index != NULL) {
		#if defined(JANUS_SDP_UTILS_FORGETECH)
		char *pos = NULL;
		if((pos = strstr(index, needle)) != NULL) {
			profile = index + (pos - index) + strlen(needle);
			if(strlen(profile) > 0)
				profile = g_strdup(profile);
			else
				profile = NULL;
			break;
		}
		#else
		if(strstr(index, needle) != NULL) {
			profile = index + strlen(needle);
			if(strlen(profile) > 0)
				profile = g_strdup(profile);
			else
				profile = NULL;
			break;
		}
		#endif
		i++;
		index = list[i];
	}
	g_clear_pointer(&list, g_strfreev);
	return profile;
}
4

While investigating a fix, I stumbled on pretty much the same exact one you suggested! I just pushed a commit here, that does seem indeed to fix it for me: Fixed inconsistencies when parsing SDP video profiles · meetecho/janus-gateway@47178fc · GitHub

Not sure if we have other places where we use strstr the same way ignoring the actual result, in case we’ll need to fix that too. Thanks for the help!

1 Like
5

Thank you for checking.
I will update the Janus code later.