Re: SBC encoder/decoder API & errors handling

[Luiz Augusto von Dentz <luiz.dentz@gmail.com>]

Hi,

On Wed, Jun 30, 2010 at 10:53 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Siarhei,
>
> On Tue, Jun 29, 2010, Siarhei Siamashka wrote:
>> When I tried to run some SBC encoder tests a few days ago, I noticed that
>> there is a regression introduced by commit:
>> http://git.kernel.org/?p=bluetooth/bluez.git;a=commit;h=c43f8bdcc1d527e2d77481a66217771038be3acd
>>
>> It is caused by the change from 'int' to 'size_t' for the type of variable
>> 'encoded' in 'sbcenc.c'. After this modification, the check for 'encoded <= 0'
>> does not catch the case when 'sbc_encode' tries to return a negative number in
>> 'encoded' variable. Later we end up calling 'write' function with a negative
>> size for the data block.
>>
>> Right now, in the case of error 'sbc_encode' function may either return a
>> negative number as a return value, or return a negative value in 'encoded'
>> variable. But this second type of error is apparently not handled by anything
>> other than 'sbcenc' tool at the moment.
>>
>> Any opinions about how to fix it in the best way? Because it is a flaw in the
>> library API, comments from the interested parties are welcome.
>
> In general the API feels a bit weird in that it can return errors in two
> different ways. A less obtrusive fix would be to make the variable type
> ssize_t instead of size_t. Regarding breaking the SBC library API, I
> don't see any big issue with that since it's internal to bluetoothd and
> we can easily update the pulseaudio copy accordingly. Would the attached
> patch make sense?

I guess it make sense, but we should also fix alsa and gstreamer code
to use variables of type ssize_t instead of int:

audio/pcm_bluetooth.c:1060:		encoded = sbc_encode(&a2dp->sbc,
data->buffer, a2dp->codesize,
audio/pcm_bluetooth.c:1096:		encoded = sbc_encode(&a2dp->sbc, buff,
a2dp->codesize,
audio/gstsbcenc.c:391:		consumed = sbc_encode(&enc->sbc, (gpointer) data,

-- 
Luiz Augusto von Dentz
Computer Engineer