Fix overwriting of 7-bit PictureID when doing VP8 simulcast (#3121)

Author: lminiero

fuzzers/rtp_fuzzer.c

@@ -92,12 +92,13 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
 	janus_h264_is_keyframe(payload, plen);
 
 	/* VP8 targets */
+	gboolean m = FALSE;
 	uint16_t picid = 0;
 	uint8_t tlzi = 0, tid = 0, ybit = 0, keyidx = 0;
 	janus_vp8_simulcast_context vp8_context;
 	memset(&vp8_context, 0, sizeof(janus_vp8_simulcast_context));
 	janus_vp8_is_keyframe(payload, plen);
-	janus_vp8_parse_descriptor(payload, plen, &picid, &tlzi, &tid, &ybit, &keyidx);
+	janus_vp8_parse_descriptor(payload, plen, &m, &picid, &tlzi, &tid, &ybit, &keyidx);
 	janus_vp8_simulcast_descriptor_update(copy_payload, plen, &vp8_context, TRUE);
 
 	/* VP9 targets */

src/rtp.c

@@ -1210,12 +1210,13 @@ gboolean janus_rtp_simulcasting_context_process_rtp(janus_rtp_simulcasting_conte
 	/* Temporal layers are only available for VP8, so don't do anything else for other codecs */
 	if(vcodec == JANUS_VIDEOCODEC_VP8) {
 		/* Check if there's any temporal scalability to take into account */
+		gboolean m = FALSE;
 		uint16_t picid = 0;
 		uint8_t tlzi = 0;
 		uint8_t tid = 0;
 		uint8_t ybit = 0;
 		uint8_t keyidx = 0;
-		if(janus_vp8_parse_descriptor(payload, plen, &picid, &tlzi, &tid, &ybit, &keyidx) == 0) {
+		if(janus_vp8_parse_descriptor(payload, plen, &m, &picid, &tlzi, &tid, &ybit, &keyidx) == 0) {
 			//~ JANUS_LOG(LOG_WARN, "%"SCNu16", %u, %u, %u, %u\n", picid, tlzi, tid, ybit, keyidx);
 			if(context->templayer != context->templayer_target && tid == context->templayer_target) {
 				/* FIXME We should be smarter in deciding when to switch */

src/utils.c

@@ -916,7 +916,7 @@ gboolean janus_h265_is_keyframe(const char *buffer, int len) {
 }
 
 int janus_vp8_parse_descriptor(char *buffer, int len,
-		uint16_t *picid, uint8_t *tl0picidx, uint8_t *tid, uint8_t *y, uint8_t *keyidx) {
+		gboolean *m, uint16_t *picid, uint8_t *tl0picidx, uint8_t *tid, uint8_t *y, uint8_t *keyidx) {
 	if(!buffer || len < 6)
 		return -1;
 	if(picid)
@@ -951,6 +951,8 @@ int janus_vp8_parse_descriptor(char *buffer, int len,
 				partpicid = (wholepicid & 0x7FFF);
 				buffer++;
 			}
+			if(m)
+				*m = (mbit ? TRUE : FALSE);
 			if(picid)
 				*picid = partpicid;
 		}
@@ -976,7 +978,7 @@ int janus_vp8_parse_descriptor(char *buffer, int len,
 	return 0;
 }
 
-static int janus_vp8_replace_descriptor(char *buffer, int len, uint16_t picid, uint8_t tl0picidx) {
+static int janus_vp8_replace_descriptor(char *buffer, int len, gboolean m, uint16_t picid, uint8_t tl0picidx) {
 	if(!buffer || len < 6)
 		return -1;
 	uint8_t vp8pd = *buffer;
@@ -994,7 +996,7 @@ static int janus_vp8_replace_descriptor(char *buffer, int len, uint16_t picid, u
 			buffer++;
 			vp8pd = *buffer;
 			uint8_t mbit = (vp8pd & 0x80);
-			if(!mbit) {
+			if(!mbit || !m) {
 				*buffer = picid;
 			} else {
 				uint16_t wholepicid = htons(picid);
@@ -1031,13 +1033,14 @@ void janus_vp8_simulcast_context_reset(janus_vp8_simulcast_context *context) {
 void janus_vp8_simulcast_descriptor_update(char *buffer, int len, janus_vp8_simulcast_context *context, gboolean switched) {
 	if(!buffer || len < 0)
 		return;
+	gboolean m = FALSE;
 	uint16_t picid = 0;
 	uint8_t tlzi = 0;
 	uint8_t tid = 0;
 	uint8_t ybit = 0;
 	uint8_t keyidx = 0;
 	/* Parse the identifiers in the VP8 payload descriptor */
-	if(janus_vp8_parse_descriptor(buffer, len, &picid, &tlzi, &tid, &ybit, &keyidx) < 0)
+	if(janus_vp8_parse_descriptor(buffer, len, &m, &picid, &tlzi, &tid, &ybit, &keyidx) < 0)
 		return;
 	if(switched) {
 		context->base_picid_prev = context->last_picid;
@@ -1046,9 +1049,16 @@ void janus_vp8_simulcast_descriptor_update(char *buffer, int len, janus_vp8_simu
 		context->base_tlzi = tlzi;
 	}
 	context->last_picid = (picid-context->base_picid)+context->base_picid_prev+1;
+	if(!m && context->last_picid > 127) {
+		context->last_picid -= 128;
+		if(context->last_picid > 127)
+			context->last_picid = 0;
+	} else if(m && context->last_picid > 32767) {
+		context->last_picid -= 32768;
+	}
 	context->last_tlzi = (tlzi-context->base_tlzi)+context->base_tlzi_prev+1;
 	/* Overwrite the values in the VP8 payload descriptors with the ones we have */
-	janus_vp8_replace_descriptor(buffer, len, context->last_picid, context->last_tlzi);
+	janus_vp8_replace_descriptor(buffer, len, m, context->last_picid, context->last_tlzi);
 }
 
 /* Helper method to parse a VP9 RTP video frame and get some SVC-related info:

src/utils.h

@@ -356,14 +356,15 @@ void janus_vp8_simulcast_context_reset(janus_vp8_simulcast_context *context);
 /*! \brief Helper method to parse a VP8 payload descriptor for useful info (e.g., when simulcasting)
  * @param[in] buffer The RTP payload to process
  * @param[in] len The length of the RTP payload
+ * @param[out] m Whether the Picture ID is 15 bit or 7 bit
  * @param[out] picid The Picture ID
  * @param[out] tl0picidx Temporal level zero index
  * @param[out] tid Temporal-layer index
  * @param[out] y Layer sync bit
  * @param[out] keyidx Temporal key frame index
  * @returns 0 in case of success, a negative integer otherwise */
 int janus_vp8_parse_descriptor(char *buffer, int len,
-		uint16_t *picid, uint8_t *tl0picidx, uint8_t *tid, uint8_t *y, uint8_t *keyidx);
+		gboolean *m, uint16_t *picid, uint8_t *tl0picidx, uint8_t *tid, uint8_t *y, uint8_t *keyidx);
 
 /*! \brief Use the context info to update the RTP header of a packet, if needed
  * @param[in] buffer The RTP payload to process