Hi Luiz,
Op 13-12-10 09:55, Luiz Augusto von Dentz schreef:
> 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.
Well it seems the block_size is not fixed on my phone, sometimes it is 1
sample more, sometimes 2, the net effect is 1 or 2 frames stay undecoded
leading to massive underruns.
I don't think u->block_size has to be fixed per se, a2dp->code_size
seems to be though.
>> 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.
Well, a2dp->frame_length was calculated based on the maximum bitpool
size. The frames decoded have a smaller bitpool size. a2dp->codesize is
still the same though, so I don't think this change matters much.
>> 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.
I don't think the bitpool changed, the code just assumes the maximum
bitpool size, while my phone sends data at a lower one.
Of course I still don't know how to handle packet loss correctly, which
is easy to create by moving phone out of the range of my bluetooth device.
Cheers,
Maarten
