Bug in sbc decoder gstreamer plugin

Bug in sbc decoder gstreamer plugin

[Bas Vermeulen]

[-- Attachment #1: Type: text/plain, Size: 973 bytes --]

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

[-- Attachment #2: bluez-gstsbcdec.patch --]
[-- Type: text/plain, Size: 439 bytes --]

--- /usr/src/nokia/mms_p2430-mute/mms_p2430/src/plugins/audio/decoder/gstrtpsbc/src/gstsbcdec.c	2009-12-01 11:32:45.000000000 +0100
+++ gstsbcdec.c	2009-12-04 10:06:34.000000000 +0100
@@ -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) {

[PATCH] Re: Bug in sbc decoder gstreamer plugin

[Bas Vermeulen]

[-- Attachment #1: Type: text/plain, Size: 1466 bytes --]

Bas Vermeulen wrote:
> 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


[-- Attachment #2: bluez-gstsbcdec-try2.patch --]
[-- Type: text/plain, Size: 810 bytes --]

--- /usr/src/nokia/mms_p2430-mute/mms_p2430/src/plugins/audio/decoder/gstrtpsbc/src/gstsbcdec.c	2009-12-01 11:32:45.000000000 +0100
+++ gstsbcdec.c	2009-12-04 12:23:04.000000000 +0100
@@ -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) {