[pulseaudio-discuss] [PATCH] bluetooth: Fix rendering a2dp data

[Luiz Augusto von Dentz]

Hi,

On Sat, Dec 11, 2010 at 1:05 AM, Maarten Lankhorst
<m.b.lankhorst at gmail.com> wrote:
> makes my android phone slightly happier
> ---
>  src/modules/bluetooth/module-bluetooth-device.c |   11 ++++++++---
>  1 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/src/modules/bluetooth/module-bluetooth-device.c b/src/modules/bluetooth/module-bluetooth-device.c
> index 6d31c1e..8664001 100644
> --- a/src/modules/bluetooth/module-bluetooth-device.c
> +++ b/src/modules/bluetooth/module-bluetooth-device.c
> @@ -1387,7 +1387,7 @@ static int a2dp_process_push(struct userdata *u) {
>     pa_assert(u->source);
>     pa_assert(u->read_smoother);
>
> -    memchunk.memblock = pa_memblock_new(u->core->mempool, u->block_size);
> +    memchunk.memblock = pa_memblock_new(u->core->mempool, u->block_size * 2);
>     memchunk.index = memchunk.length = 0;

Im not sure how this would help, we decode sbc frame by frame so
having twice as much memory doesn't really make any sense, there could
be that we should decode the entire buffer read, but that would take
much more memory. What could be the real problem is that the source is
changing the bitpool but that shouldn't be a problem since the
block_size is supposed to be calculated from maximum bitpool range so
the buffer will always be big enough.

>     for (;;) {
> @@ -1442,7 +1442,8 @@ static int a2dp_process_push(struct userdata *u) {
>         to_decode = l - sizeof(*header) - sizeof(*payload);
>
>         d = pa_memblock_acquire(memchunk.memblock);
> -        to_write = memchunk.length = pa_memblock_get_length(memchunk.memblock);
> +        to_write = pa_memblock_get_length(memchunk.memblock);
> +        memchunk.length = 0;
>
>         while (PA_LIKELY(to_decode > 0 && to_write > 0)) {
>             size_t written;
> @@ -1464,7 +1465,7 @@ static int a2dp_process_push(struct userdata *u) {
>  /*             pa_log_debug("SBC: frame_length: %lu; codesize: %lu", (unsigned long) a2dp->frame_length, (unsigned long) a2dp->codesize); */
>
>             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);

This is the real problem, either we do this if the bitpool is changing
or we have to call sbc_reinit in each frame.

>             pa_assert_fp((size_t) written <= to_write);
>             pa_assert_fp((size_t) written == a2dp->codesize);
> @@ -1474,10 +1475,14 @@ static int a2dp_process_push(struct userdata *u) {
>
>             d = (uint8_t*) d + written;
>             to_write -= written;
> +            memchunk.length += written;
>
>             frame_count++;
>         }
>
> +        if (to_decode)
> +            pa_log_error("SBC: %lu bytes not decoded\n", to_decode);
> +
>         pa_memblock_release(memchunk.memblock);

Actually I think we should be solving this in libsbc, both sbc_encode
and sbc_decode should be checking if the bitpool has changed and
reinit automatically, otherwise we gonna get performance penalty doing
sbc_reinit for every frame.


Regards,

-- 
Luiz Augusto von Dentz
Computer Engineer