[pulseaudio-discuss] [PATCH v2] bluez5-device: Rewrite of thread function, reduce send buffer size for a2dp sink

[Georg Chini]

On 24.03.2018 18:13, Tanu Kaskinen wrote:
> On Mon, 2018-03-05 at 08:49 +0100, Georg Chini wrote:
>> The rewrite of the thread function does not change functionality much,
>> most of it is only cleanup, minor bug fixing  and documentation work.
>>
>> This patch also changes the send buffer size for a2dp sink to avoid lags
>> after temporary connection drops, following the proof-of-concept patch
>> posted by Dmitry Kalyanov.
>>
>> Bug-Link: https://bugs.freedesktop.org/show_bug.cgi?id=58746
>> ---
>>   src/modules/bluetooth/module-bluez5-device.c | 275 ++++++++++++++++++---------
>>   1 file changed, 182 insertions(+), 93 deletions(-)
> Thanks! Reading the new code caused less trouble than I recall the
> bluetooth thread_func() previously having caused, so the changes are
> good.
>
> There are some issues pointed out below.
>
>> diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c
>> index 7970dda7..149d1708 100644
>> --- a/src/modules/bluetooth/module-bluez5-device.c
>> +++ b/src/modules/bluetooth/module-bluez5-device.c
>> @@ -56,7 +56,6 @@ PA_MODULE_LOAD_ONCE(false);
>>   PA_MODULE_USAGE("path=<device object path>"
>>                   "autodetect_mtu=<boolean>");
>>   
>> -#define MAX_PLAYBACK_CATCH_UP_USEC (100 * PA_USEC_PER_MSEC)
>>   #define FIXED_LATENCY_PLAYBACK_A2DP (25 * PA_USEC_PER_MSEC)
>>   #define FIXED_LATENCY_PLAYBACK_SCO (125 * PA_USEC_PER_MSEC)
>>   #define FIXED_LATENCY_RECORD_A2DP   (25 * PA_USEC_PER_MSEC)
>> @@ -660,6 +659,38 @@ static int a2dp_process_push(struct userdata *u) {
>>       return ret;
>>   }
>>   
>> +static void update_buffer_size(struct userdata *u) {
>> +    int old_bufsize;
>> +    socklen_t len = sizeof(int);
>> +    int ret;
>> +
>> +    ret = getsockopt(u->stream_fd, SOL_SOCKET, SO_SNDBUF, &old_bufsize, &len);
> In case the type of old_bufsize changes (which it will never do, but
> what if it does anyway) then the calculation of len has to be
> separately updated. Therefore it would be safer to pass
> sizeof(new_bufsize) here and get rid of the len variable.
>
Unfortunately the getsockopt() call takes a pointer to the len variable
as argument, therefore I cannot pass sizeof(old_bufsize) directly. I'll
leave this as it is.