Skip to contentSkip to conversation

Bug in sbc decoder gstreamer plugin

PATCH Bas Vermeulen 1 participant 2 messages Dec 4, 2009

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

Hello *,

We've run into a problem with the gstreamer sbcdec plugin, where a pipeline would accumulate memory at the rate of data sent into it. This is caused by invalid data in a packet (incomplete or wrong data in the stream), causing sbc_decode to return -2 (sbc_unpack_frame can't find the SBC_SYNCWORD in data[0]). The sbcdec plugin handles this by putting away the buffer, and trying again when a new buffer comes in. Since the buffer isn't changed (only new data tacked on), sbc_decode will continue to send the -2 response from sbc_unpack_frame function, and the sbcdec plugin will continue to accumulate data into the buffer it is working on.

The fix is to simply drop a buffer when sbc_decode returns -2.

Regards,

Bas Vermeulen
--
novero gmbh | parsevalstr. 7 a | 40468 düsseldorf | germany | amtsgericht düsseldorf | hrb 58283 | umsatzsteueridentifikationsnummer: de 814973142 | geschäftsführender gesellschafter: razvan olosu

gstsbcdec.c renamed from usr/src/nokia/mms_p2430-mute/mms_p2430/src/plugins/audio/decoder/gstrtpsbc/src/gstsbcdec.c +6 -1
@@ -95,7 +95,12 @@
 					GST_BUFFER_DATA(output), codesize,
 					NULL);
 		if (consumed <= 0)
-			break;
+		{
+			if (-2 == consumed)
+				goto done;
+			else 
+				break;
+		}
 
 		/* we will reuse the same caps object */
 		if (dec->outcaps == NULL) {
#1 lore raw
Quoted 5 lines · Bas Vermeulen

We've run into a problem with the gstreamer sbcdec plugin, where a pipeline would accumulate memory at the rate of data sent into it. This is caused by invalid data in a packet (incomplete or wrong data in the stream), causing sbc_decode to return -2 (sbc_unpack_frame can't find the SBC_SYNCWORD in data[0]). The sbcdec plugin handles this by putting away the buffer, and trying again when a new buffer comes in. Since the buffer isn't changed (only new data tacked on), sbc_decode will continue to send the -2 response from sbc_unpack_frame function, and the sbcdec plugin will continue to accumulate data into the buffer it is working on.

The fix is to simply drop a buffer when sbc_decode returns -2.

I just realised this doesn't do enough, as sbc_decode can return negative values in the range of -1 to -4 (-1 being need more data, the rest being can't handle this frame for various reasons).

I've included a new version of the patch, which handles a -1 return value by keeping the buffer for the next decode, and the other values by dropping the buffer. This should also fix similar errors in case we get a bad CRC or a faulty packet.

Regards,

Bas Vermeulen

--
novero gmbh | parsevalstr. 7 a | 40468 düsseldorf | germany | amtsgericht düsseldorf | hrb 58283 | umsatzsteueridentifikationsnummer: de 814973142 | geschäftsführender gesellschafter: razvan olosu

gstsbcdec.c renamed from usr/src/nokia/mms_p2430-mute/mms_p2430/src/plugins/audio/decoder/gstrtpsbc/src/gstsbcdec.c +12 -1
@@ -95,7 +95,18 @@
 					GST_BUFFER_DATA(output), codesize,
 					NULL);
 		if (consumed <= 0)
-			break;
+		{
+			/*
+			 * sbc_decode returns -1 when not enough data is available to process.
+			 * If that is the case, we need to tack on another buffer.
+			 * In case of other errors (crc failure, SBC_SYNCWORD not found, etc),
+			 * keeping the offending data does no good, as we will keep getting the
+			 * same error over and over. So just free the buffer by jumping to done. 
+			 */
+			if (-1 == consumed)
+				break;
+			goto done;
+		}
 
 		/* we will reuse the same caps object */
 		if (dec->outcaps == NULL) {