[pulseaudio-discuss] [PATCH v0 5/5] bluetooth: Consider different input and output MTU

[Mikel Astiz]

Hi Tanu,

On Thu, Jul 26, 2012 at 6:36 AM, Tanu Kaskinen <tanuk at iki.fi> wrote:
> On Fri, 2012-07-06 at 11:19 +0200, Mikel Astiz wrote:
>> @@ -226,13 +229,19 @@ static void a2dp_set_bitpool(struct userdata *u, uint8_t bitpool)
>>
>>      pa_log_debug("Bitpool has changed to %u", a2dp->sbc.bitpool);
>>
>> -    u->block_size =
>> -            (u->link_mtu - sizeof(struct rtp_header) - sizeof(struct rtp_payload))
>> -            / a2dp->frame_length * a2dp->codesize;
>> +    u->read_block_size =
>> +        ((u->read_link_mtu - sizeof(struct rtp_header) - sizeof(struct rtp_payload))
>> +        / a2dp->frame_length
>> +        * a2dp->codesize);
>
> What's the point of the new parentheses?
>
> Ok, now I noticed that later in the code there are the same
> calculations, and there the old code contains the extra parentheses, so
> maybe this is just copy-pasted code. I'd prefer you to remove the
> parentheses from the old code that has them rather than to add them
> here.

I will send a separate patch to make this consistent first.

>> @@ -328,7 +337,7 @@ static int bt_transport_acquire(struct userdata *u, pa_bool_t start) {
>>      }
>>
>>      /* FIXME: Handle in/out MTU properly when unix socket is not longer supported */
>> -    u->stream_fd = pa_bluetooth_transport_acquire(t, accesstype, NULL, &u->link_mtu);
>> +    u->stream_fd = pa_bluetooth_transport_acquire(t, accesstype, &u->read_link_mtu, &u->write_link_mtu);
>
> Shouldn't the FIXME note be now removed?

Yes, I would also guess this FIXME refers to the issue addressed in
this patch so I will remove it. We should ask Luiz for confirmation
though.

>> @@ -792,7 +803,7 @@ static int a2dp_process_render(struct userdata *u) {
>>          pa_memblock_unref(u->write_memchunk.memblock);
>>          pa_memchunk_reset(&u->write_memchunk);
>>
>> -        ret = 1;
>> +        ret = l;
>
> The code that calls a2dp_process_render saves the return value in
> variable n_written, and then substracts n_written from do_write. I
> believe do_write doesn't use bytes as units, so maybe this is why
> Frédéric had trouble?

Yes, you are actually right. This change is not needed at all.

> Also, it looks like you forgot to change hsp_process_render() to return
> bytes too.

No, as you said do_write doesn't use bytes as units so
hsp_process_render() should return the number of blocks sent.

Patchset v1 to be submitted soon.

Cheers,
Mikel