The choice of the option name is unfortunate. Imagine if another
situation appears where we would either use or not use getsockopt() to
get some other parameter. How about renaming it to something like
autodetect_mtu?
2017-02-04 23:13 GMT+05:00 Georg Chini <georg at chini.tk>:
> 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 "use_getsockopt" for
> module-bluetooth-discover, module-bluez5-discover and module-bluez5-device to
> make the use of getsockopt() configurable.
> The parameter defaults to false, so backend-native returns to the old behavior
> of using 48 as default MTU.
>
> ---
> 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..68d771e 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->use_getsockopt) {
> + 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");
> + else {
> + if (imtu) *imtu = sco_opt.mtu;
> + if (omtu) *omtu = sco_opt.mtu;
> + }
> }
>
> return sock;
> diff --git a/src/modules/bluetooth/bluez5-util.h b/src/modules/bluetooth/bluez5-util.h
> index 7f124e2..bfcf547 100644
> --- a/src/modules/bluetooth/bluez5-util.h
> +++ b/src/modules/bluetooth/bluez5-util.h
> @@ -95,6 +95,7 @@ struct pa_bluetooth_device {
> bool properties_received;
> bool tried_to_link_with_adapter;
> bool valid;
> + bool use_getsockopt;
>
> /* Device information */
> char *path;
> diff --git a/src/modules/bluetooth/module-bluetooth-discover.c b/src/modules/bluetooth/module-bluetooth-discover.c
> index fe8aec9..a716972 100644
> --- a/src/modules/bluetooth/module-bluetooth-discover.c
> +++ b/src/modules/bluetooth/module-bluetooth-discover.c
> @@ -33,6 +33,7 @@ PA_MODULE_VERSION(PACKAGE_VERSION);
> PA_MODULE_LOAD_ONCE(true);
> PA_MODULE_USAGE(
> "headset=ofono|native|auto (bluez 5 only)"
> + "use_getsockopt=<boolean> (bluez 5 only)"
> );
>
> struct userdata {
> diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c
> index 065fcaa..a9a8bf6 100644
> --- a/src/modules/bluetooth/module-bluez5-device.c
> +++ b/src/modules/bluetooth/module-bluez5-device.c
> @@ -54,7 +54,8 @@ PA_MODULE_AUTHOR("João Paulo Rechi Vita");
> PA_MODULE_DESCRIPTION("BlueZ 5 Bluetooth audio sink and source");
> PA_MODULE_VERSION(PACKAGE_VERSION);
> PA_MODULE_LOAD_ONCE(false);
> -PA_MODULE_USAGE("path=<device object path>");
> +PA_MODULE_USAGE("path=<device object path>"
> + "use_getsockopt=<boolean>");
>
> #define MAX_PLAYBACK_CATCH_UP_USEC (100 * PA_USEC_PER_MSEC)
> #define FIXED_LATENCY_PLAYBACK_A2DP (25 * PA_USEC_PER_MSEC)
> @@ -68,6 +69,7 @@ PA_MODULE_USAGE("path=<device object path>");
>
> static const char* const valid_modargs[] = {
> "path",
> + "use_getsockopt",
> NULL
> };
>
> @@ -2158,6 +2160,7 @@ int pa__init(pa_module* m) {
> struct userdata *u;
> const char *path;
> pa_modargs *ma;
> + bool use_getsockopt;
>
> pa_assert(m);
>
> @@ -2187,6 +2190,14 @@ int pa__init(pa_module* m) {
> goto fail_free_modargs;
> }
>
> + use_getsockopt = false;
> + if (pa_modargs_get_value_boolean(ma, "use_getsockopt", &use_getsockopt) < 0) {
> + pa_log("Invalid boolean value for use_getsockopt parameter");
> + goto fail_free_modargs;
> + }
> +
> + u->device->use_getsockopt = use_getsockopt;
> +
> pa_modargs_free(ma);
>
> u->device_connection_changed_slot =
> diff --git a/src/modules/bluetooth/module-bluez5-discover.c b/src/modules/bluetooth/module-bluez5-discover.c
> index 080e5d0..d49e960 100644
> --- a/src/modules/bluetooth/module-bluez5-discover.c
> +++ b/src/modules/bluetooth/module-bluez5-discover.c
> @@ -42,6 +42,7 @@ PA_MODULE_USAGE(
>
> static const char* const valid_modargs[] = {
> "headset",
> + "use_getsockopt",
> NULL
> };
>
> @@ -51,6 +52,7 @@ struct userdata {
> pa_hashmap *loaded_device_paths;
> pa_hook_slot *device_connection_changed_slot;
> pa_bluetooth_discovery *discovery;
> + bool use_getsockopt;
> };
>
> static pa_hook_result_t device_connection_changed_cb(pa_bluetooth_discovery *y, const pa_bluetooth_device *d, struct userdata *u) {
> @@ -71,7 +73,7 @@ static pa_hook_result_t device_connection_changed_cb(pa_bluetooth_discovery *y,
> if (!module_loaded && pa_bluetooth_device_any_transport_connected(d)) {
> /* a new device has been connected */
> pa_module *m;
> - char *args = pa_sprintf_malloc("path=%s", d->path);
> + char *args = pa_sprintf_malloc("path=%s use_getsockopt=%i", d->path, (int)u->use_getsockopt);
>
> pa_log_debug("Loading module-bluez5-device %s", args);
> m = pa_module_load(u->module->core, "module-bluez5-device", args);
> @@ -101,6 +103,7 @@ int pa__init(pa_module *m) {
> pa_modargs *ma;
> const char *headset_str;
> int headset_backend;
> + bool use_getsockopt;
>
> pa_assert(m);
>
> @@ -121,9 +124,16 @@ int pa__init(pa_module *m) {
> goto fail;
> }
>
> + use_getsockopt = false;
> + if (pa_modargs_get_value_boolean(ma, "use_getsockopt", &use_getsockopt) < 0) {
> + pa_log("Invalid boolean value for use_getsockopt parameter");
> + goto fail;
> + }
> +
> m->userdata = u = pa_xnew0(struct userdata, 1);
> u->module = m;
> u->core = m->core;
> + u->use_getsockopt = use_getsockopt;
> u->loaded_device_paths = pa_hashmap_new(pa_idxset_string_hash_func, pa_idxset_string_compare_func);
>
> if (!(u->discovery = pa_bluetooth_discovery_get(u->core, headset_backend)))
> --
> 2.10.1
>
> _______________________________________________
> pulseaudio-discuss mailing list
> pulseaudio-discuss at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
--
Alexander E. Patrakov
