[pulseaudio-discuss] [PATCH v2] bluetooth: Make use of getsockopt() to determine MTU configurable

[Tanu Kaskinen]

On Sat, 2017-02-04 at 20:32 +0100, Georg Chini wrote:
> A recent patch changed the MTU size from the default value of 48 to the value
> returned by getsockopt(). This breaks HSP for some setups. To circumvent the
> problem, this patch introduces a boolean parameter "autodetect_mtu" for
> module-bluetooth-discover, module-bluez5-discover and module-bluez5-device to
> make this use of getsockopt() configurable.
> The parameter defaults to false, so backend-native returns to the old behavior
> of using 48 as default MTU.

I think defaulting to true would be better. Defaulting to false is
never going to make all hardware work out of the box, but defaulting to
true may someday always work, since the kernel developers seem to be
willing to add hardware specific quirks.

> ---
>  src/modules/bluetooth/backend-native.c            | 25 ++++++++++++-----------
>  src/modules/bluetooth/bluez5-util.h               |  1 +
>  src/modules/bluetooth/module-bluetooth-discover.c |  1 +
>  src/modules/bluetooth/module-bluez5-device.c      | 13 +++++++++++-
>  src/modules/bluetooth/module-bluez5-discover.c    | 12 ++++++++++-
>  5 files changed, 38 insertions(+), 14 deletions(-)
> 
> diff --git a/src/modules/bluetooth/backend-native.c b/src/modules/bluetooth/backend-native.c
> index 4974c3f..2978015 100644
> --- a/src/modules/bluetooth/backend-native.c
> +++ b/src/modules/bluetooth/backend-native.c
> @@ -147,18 +147,19 @@ static int bluez5_sco_acquire_cb(pa_bluetooth_transport *t, bool optional, size_
>          goto fail_close;
>      }
>  
> -    len = sizeof(sco_opt);
> -    memset(&sco_opt, 0, len);
> -
> -    if (getsockopt(sock, SOL_SCO, SCO_OPTIONS, &sco_opt, &len) < 0) {
> -        pa_log_warn("getsockopt(SCO_OPTIONS) failed, loading defaults");
> -
> -        /* Setting defaults in case of error */
> -        if (imtu) *imtu = 48;
> -        if (omtu) *omtu = 48;
> -    } else {
> -        if (imtu) *imtu = sco_opt.mtu;
> -        if (omtu) *omtu = sco_opt.mtu;
> +    if (imtu) *imtu = 48;
> +    if (omtu) *omtu = 48;
> +
> +    if (t->device->autodetect_mtu) {
> +        len = sizeof(sco_opt);
> +        memset(&sco_opt, 0, len);

The sco_opt variable declaration could be moved inside this if block.

I'll make these small changes myself and push the patch.

-- 
Tanu

https://www.patreon.com/tanuk