pulseaudio aborts with assertion decoded == a2dp->frame_length in function a2dp_process_push()

The assertion is incorrectly testing for '==' when logically it should test for '<='. The attached patch gets us past the assert. The next part of the puzzle is the loopback of the a2dp audio to the computer speakers, and then to make it play smoothly.

Hi and thanks for the patch! Sorry for the late reply. I'm sending the patch upstream for review; they are more qualified than I when it comes to the bluetooth protocol.

According to the patch author, Karl Hegbloom:
"The assertion is incorrectly testing for '==' when logically it
should test for '<='."

BugLink: https://bugs.launchpad.net/bugs/689915
Signed-off-by: David Henningsson <email address hidden>
---

This is a patch I found attached to a bug in Ubuntu. I don't know
much about bluetooth, and the error was reported against something
around 0.9.22. But I'm sending it here, hoping that one of our
bluetooth people can review and say whether this is correct or not.

 src/modules/bluetooth/module-bluetooth-device.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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;
--
1.7.9.5

On Wed, 2012-10-10 at 09:17 +0200, David Henningsson wrote:
> According to the patch author, Karl Hegbloom:
> "The assertion is incorrectly testing for '==' when logically it
> should test for '<='."
>
> BugLink: https://bugs.launchpad.net/bugs/689915
> Signed-off-by: David Henningsson <email address hidden>
> ---
>
> This is a patch I found attached to a bug in Ubuntu. I don't know
> much about bluetooth, and the error was reported against something
> around 0.9.22. But I'm sending it here, hoping that one of our
> bluetooth people can review and say whether this is correct or not.
>
> src/modules/bluetooth/module-bluetooth-device.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> 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.

--
Tanu

Hi,

On Fri, Oct 19, 2012 at 7:12 PM, Arun Raghavan
<email address hidden> 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 <email address hidden>
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

Thank you for reporting this bug to Ubuntu.
Ubuntu 10.04 (lucid) reached end-of-life on May 9, 2013.

See this document for currently supported Ubuntu releases:
https://wiki.ubuntu.com/Releases

Please upgrade to the latest version and re-test.

[Expired for pulseaudio (Ubuntu) because there has been no activity for 60 days.]