[pulseaudio-discuss] [PATCH 3/4] bluetooth: Create NULL backend

Tanu Kaskinen tanu.kaskinen at linux.intel.com
Sun Aug 17 02:57:23 PDT 2014


On Wed, 2014-08-13 at 15:23 +0300, Luiz Augusto von Dentz wrote:
> From: João Paulo Rechi Vita <jprvita at openbossa.org>
> 
> ---
>  configure.ac                         | 11 +++++++++++
>  src/Makefile.am                      |  4 +++-
>  src/modules/bluetooth/backend-null.c | 37 ++++++++++++++++++++++++++++++++++++
>  src/modules/bluetooth/backend.h      | 31 ++++++++++++++++++++++++++++++
>  src/modules/bluetooth/bluez5-util.c  | 10 ++++++++--
>  5 files changed, 90 insertions(+), 3 deletions(-)
>  create mode 100644 src/modules/bluetooth/backend-null.c
>  create mode 100644 src/modules/bluetooth/backend.h
> 
> diff --git a/configure.ac b/configure.ac
> index 837e81e..2beca1f 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1028,6 +1028,16 @@ AS_IF([test "x$HAVE_BLUEZ_4" = "x1" || test "x$HAVE_BLUEZ_5" = "x1"], HAVE_BLUEZ
>  AC_SUBST(HAVE_BLUEZ)
>  AM_CONDITIONAL([HAVE_BLUEZ], [test "x$HAVE_BLUEZ" = x1])
>  
> +## Headset profiles backend ##
> +AC_ARG_WITH(bluetooth_headset_backend,
> +AS_HELP_STRING([--with-bluetooth-headset-backend=<null>],[Backend for Bluetooth headset profiles (null)]))

Please indent the AS_HELP_STRING line, since it's inside AC_ARG_WITH.

> +if test -z "$with_bluetooth_headset_backend" ; then
> +    BLUETOOTH_HEADSET_BACKEND=null
> +else
> +    BLUETOOTH_HEADSET_BACKEND=$with_bluetooth_headset_backend

It would be nice to have a check that ensures that the given backend is
one of the supported values.

> +fi
> +AC_SUBST(BLUETOOTH_HEADSET_BACKEND)
> +
>  #### UDEV support (optional) ####
>  
>  AC_ARG_ENABLE([udev],
> @@ -1489,6 +1499,7 @@ echo "
>      Enable D-Bus:                  ${ENABLE_DBUS}
>        Enable BlueZ 4:              ${ENABLE_BLUEZ_4}
>        Enable BlueZ 5:              ${ENABLE_BLUEZ_5}
> +        headset backed:            ${BLUETOOTH_HEADSET_BACKEND}
>      Enable udev:                   ${ENABLE_UDEV}
>        Enable HAL->udev compat:     ${ENABLE_HAL_COMPAT}
>      Enable systemd login:          ${ENABLE_SYSTEMD}
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 5924bd8..b20b6df 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -2085,7 +2085,9 @@ module_bluez4_device_la_CFLAGS = $(AM_CFLAGS) $(DBUS_CFLAGS) $(SBC_CFLAGS)
>  libbluez5_util_la_SOURCES = \
>  		modules/bluetooth/bluez5-util.c \
>  		modules/bluetooth/bluez5-util.h \
> -		modules/bluetooth/a2dp-codecs.h
> +		modules/bluetooth/a2dp-codecs.h \
> +		modules/bluetooth/backend.h \
> +		modules/bluetooth/backend- at BLUETOOTH_HEADSET_BACKEND@.c
>  libbluez5_util_la_LDFLAGS = -avoid-version
>  libbluez5_util_la_LIBADD = $(MODULE_LIBADD) $(DBUS_LIBS)
>  libbluez5_util_la_CFLAGS = $(AM_CFLAGS) $(DBUS_CFLAGS)
> diff --git a/src/modules/bluetooth/backend-null.c b/src/modules/bluetooth/backend-null.c
> new file mode 100644
> index 0000000..9ef1ee4
> --- /dev/null
> +++ b/src/modules/bluetooth/backend-null.c
> @@ -0,0 +1,37 @@
> +/***
> +  This file is part of PulseAudio.
> +
> +  Copyright 2013 João Paulo Rechi Vita
> +
> +  PulseAudio is free software; you can redistribute it and/or modify
> +  it under the terms of the GNU Lesser General Public License as
> +  published by the Free Software Foundation; either version 2.1 of the
> +  License, or (at your option) any later version.
> +
> +  PulseAudio is distributed in the hope that it will be useful, but
> +  WITHOUT ANY WARRANTY; without even the implied warranty of
> +  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +  General Public License for more details.
> +
> +  You should have received a copy of the GNU Lesser General Public
> +  License along with PulseAudio; if not, write to the Free Software
> +  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
> +  USA.
> +***/
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <pulsecore/log.h>
> +
> +#include "backend.h"
> +
> +hf_backend_data_t *hf_backend_new(pa_core *c) {
> +    pa_log_debug("Bluetooth Headset Backend API support disabled");
> +    return NULL;
> +}
> +
> +void hf_backend_free(hf_backend_data_t *data) {
> +    /* Nothing to do here */
> +}
> diff --git a/src/modules/bluetooth/backend.h b/src/modules/bluetooth/backend.h
> new file mode 100644
> index 0000000..78a857c
> --- /dev/null
> +++ b/src/modules/bluetooth/backend.h
> @@ -0,0 +1,31 @@
> +#ifndef foohfagenthfoo
> +#define foohfagenthfoo

These refer to the old file name still.

> +
> +/***
> +  This file is part of PulseAudio.
> +
> +  Copyright 2013 João Paulo Rechi Vita
> +
> +  PulseAudio is free software; you can redistribute it and/or modify
> +  it under the terms of the GNU Lesser General Public License as
> +  published by the Free Software Foundation; either version 2.1 of the
> +  License, or (at your option) any later version.
> +
> +  PulseAudio is distributed in the hope that it will be useful, but
> +  WITHOUT ANY WARRANTY; without even the implied warranty of
> +  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +  General Public License for more details.
> +
> +  You should have received a copy of the GNU Lesser General Public
> +  License along with PulseAudio; if not, write to the Free Software
> +  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
> +  USA.
> +***/
> +
> +#include <pulsecore/core.h>
> +
> +typedef struct hf_backend_data hf_backend_data_t;

Our convention is to not use the _t suffix with structs, and as I
already said in the previous review round, I don't like the _data suffix
either. Also, types declared in headers should have the pa_ prefix, so I
suggest that this struct is named pa_hf_backend or
pa_bluetooth_hf_backend, whichever you like more.

I'd prefer the header (and implementation) file names to follow the
struct name, i.e. call this hf-backend.h.

> +
> +hf_backend_data_t *hf_backend_new(pa_core *c);
> +void hf_backend_free(hf_backend_data_t *data);
> +#endif
> diff --git a/src/modules/bluetooth/bluez5-util.c b/src/modules/bluetooth/bluez5-util.c
> index adb8351..c6755e0 100644
> --- a/src/modules/bluetooth/bluez5-util.c
> +++ b/src/modules/bluetooth/bluez5-util.c
> @@ -34,6 +34,7 @@
>  #include <pulsecore/shared.h>
>  
>  #include "a2dp-codecs.h"
> +#include "backend.h"
>  
>  #include "bluez5-util.h"
>  
> @@ -87,6 +88,7 @@ struct pa_bluetooth_discovery {
>      pa_hashmap *devices;
>      pa_hashmap *transports;
>  
> +    hf_backend_data_t *backend;
>      PA_LLIST_HEAD(pa_dbus_pending, pending);
>  };
>  
> @@ -1110,9 +1112,9 @@ const char *pa_bluetooth_profile_to_string(pa_bluetooth_profile_t profile) {
>          case PA_BLUETOOTH_PROFILE_A2DP_SOURCE:
>              return "a2dp_source";
>          case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
> -            return "headset_head_unit";
> +            return "hsp";
>          case PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY:
> -            return "headset_audio_gateway";
> +            return "hfgw";

These changes are unrelated to the null backend. "headset_head_unit" and
"headset_audio_gateway" were introduced in patch 1/4, so that's the
right place to give names to the new profiles.

-- 
Tanu



More information about the pulseaudio-discuss mailing list