[pulseaudio-discuss] [RFC PATCH] bluetooth: Fix assertion failure (decoded == a2dp->frame_length)

[Luiz Augusto von Dentz]

Hi,

On Fri, Oct 19, 2012 at 7:12 PM, Arun Raghavan
<arun.raghavan at collabora.co.uk> wrote:
> On Sun, 2012-10-14 at 21:03 +0300, Tanu Kaskinen wrote:
> [...]
>> > diff --git a/src/modules/bluetooth/module-bluetooth-device.c b/src/modules/bluetooth/module-bluetooth-device.c
>> > index e3ec6ae..06c783d 100644
>> > --- a/src/modules/bluetooth/module-bluetooth-device.c
>> > +++ b/src/modules/bluetooth/module-bluetooth-device.c
>> > @@ -999,9 +999,9 @@ static int a2dp_process_push(struct userdata *u) {
>> >              a2dp->frame_length = sbc_get_frame_length(&a2dp->sbc);
>> >
>> >              pa_assert_fp((size_t) decoded <= to_decode);
>> > -            pa_assert_fp((size_t) decoded == a2dp->frame_length);
>> > +            pa_assert_fp((size_t) decoded <= a2dp->frame_length);
>> >
>> > -            pa_assert_fp((size_t) written == a2dp->codesize);
>> > +            pa_assert_fp((size_t) written <= a2dp->codesize);
>> >
>> >              p = (const uint8_t*) p + decoded;
>> >              to_decode -= decoded;
>>
>> I'm no SBC expert either, but I had a look at sbc_decode()
>> implementation, and to me it looks like at least "decoded ==
>> a2dp->frame_length" should hold. Extrapolating from that, "written ==
>> a2dp->codesize" probably should hold too. And it makes sense:
>> sbc_decode() is supposed to decode one frame, and I think one frame will
>> always have encoded size of a2dp->frame_length and decoded size of
>> a2dp->codesize.
>
> Same caveat about not being an expert, but my understanding of the SBC
> codec is that the original assertion is correct. If this problem is
> consistently reproducible, I think it's worth taking up with the BlueZ
> folks.
>
> -- Arun

Sorry for late response, sbc only support decoding one frame at time
so we should hold to that if nothing has changed in this respect.
Actually by looking at the bug it seems to be using 0.9.21, right? I
think this it is a different problem were the source changes the
bitpool during stream (SBC support VBR) thus the frame size changes
causing it to assert, but this has been fixed upstream already:

commit 97f7c5759e65a700a934790ee0d846a33c4a7f66
Author: Luiz Augusto von Dentz <luiz.dentz-von at nokia.com>
Date:   Thu Dec 23 15:24:39 2010 +0200

    bluetooth: fix a2dp_process_push

    Use minimum bitpool configured to get the maximum block_size possible,
    also remove checks for how much has been written when decoding sbc frames
    since the block size may change due to bitpool changes.


-- 
Luiz Augusto von Dentz