Re: [PATCH 09/26] android/hal-audio: Make codecs configurable

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Andrzej,

On Mon, May 26, 2014 at 4:16 PM, Andrzej Kaczmarek
<andrzej.kaczmarek@xxxxxxxxx> wrote:
> When supporting more codecs we need way to easily configure which ones
> should be registered as endpoints. This patch adds support to specify
> list of codecs to be loaded in "persist.sys.bluetooth.codecs" property
> which is comma-separated case-insensitive list.
>
> Codecs shall be listed in order of preference, i.e. 1st one is most
> preferred. SBC codec is always loaded and there's no need to specify
> it.
> ---
>  android/Makefile.am |  1 +
>  android/hal-audio.c | 85 ++++++++++++++++++++++++++++++++++++++++++++---------
>  2 files changed, 72 insertions(+), 14 deletions(-)
>
> diff --git a/android/Makefile.am b/android/Makefile.am
> index 2d74505..6891cbc 100644
> --- a/android/Makefile.am
> +++ b/android/Makefile.am
> @@ -169,6 +169,7 @@ android_audio_a2dp_default_la_SOURCES = android/audio-msg.h \
>                                         android/hal-audio.h \
>                                         android/hal-audio.c \
>                                         android/hal-audio-sbc.c \
> +                                       android/cutils/properties.h \
>                                         android/hardware/audio.h \
>                                         android/hardware/audio_effect.h \
>                                         android/hardware/hardware.h \
> diff --git a/android/hal-audio.c b/android/hal-audio.c
> index 8b82498..e6015f2 100644
> --- a/android/hal-audio.c
> +++ b/android/hal-audio.c
> @@ -27,6 +27,7 @@
>  #include <arpa/inet.h>
>  #include <fcntl.h>
>
> +#include <cutils/properties.h>
>  #include <hardware/audio.h>
>  #include <hardware/hardware.h>
>
> @@ -96,8 +97,11 @@ extern int clock_nanosleep(clockid_t clock_id, int flags,
>                                         struct timespec *remain);
>  #endif
>
> -static const audio_codec_get_t audio_codecs[] = {
> -               codec_sbc,
> +static const struct audio_codec_info {
> +       const char *name;
> +       const struct audio_codec * (*get) (void);
> +} audio_codecs[] = {
> +               { "sbc",        codec_sbc },
>  };
>
>  #define NUM_CODECS (sizeof(audio_codecs) / sizeof(audio_codecs[0]))
> @@ -415,27 +419,80 @@ static int ipc_suspend_stream_cmd(uint8_t endpoint_id)
>         return result;
>  }
>
> -static int register_endpoints(void)
> +static void register_endpoint(const char *codec_name)
>  {
> -       struct audio_endpoint *ep = &audio_endpoints[0];
> +       const struct audio_codec *codec = NULL;
> +       struct audio_endpoint *ep;
>         size_t i;
>
> -       for (i = 0; i < NUM_CODECS; i++, ep++) {
> -               const struct audio_codec *codec = audio_codecs[i]();
> +       for (i = 0; i < NUM_CODECS; i++)
> +               if (!strcasecmp(audio_codecs[i].name, codec_name)) {
> +                       codec = audio_codecs[i].get();
> +                       break;
> +               }
>
> -               if (!codec)
> -                       return AUDIO_STATUS_FAILED;
> +       if (!codec) {
> +               error("Failed to register endpoint for %s", codec_name);
> +               return;
> +       }
>
> -               ep->id = ipc_open_cmd(codec);
> +       for (i = 0; i < MAX_AUDIO_ENDPOINTS; i++) {
> +               /* Do not register the same codec twice */
> +               if (audio_endpoints[i].codec == codec)
> +                       return;
>
> -               if (!ep->id)
> -                       return AUDIO_STATUS_FAILED;
> +               if (!audio_endpoints[i].id)
> +                       break;
> +       }
>
> -               ep->codec = codec;
> -               ep->codec_data = NULL;
> -               ep->fd = -1;
> +       /* We have the same number of endpoints and codecs so we'll always find
> +        * free endpoint - no need to check this here.
> +        */
> +
> +       ep = &audio_endpoints[i];
> +
> +       ep->id = ipc_open_cmd(codec);
> +
> +       if (!ep->id) {
> +               error("Failed to open endpoint for %s", codec_name);
> +               return;
>         }
>
> +       ep->codec = codec;
> +       ep->codec_data = NULL;
> +       ep->fd = -1;
> +
> +       info("Opened endpoint for %s", codec_name);
> +}
> +
> +static int register_endpoints(void)
> +{
> +       char value[PROPERTY_VALUE_MAX];
> +       char *name, *p;
> +
> +       if (property_get("persist.sys.bluetooth.codecs", value, "") < 0)
> +               return AUDIO_STATUS_FAILED;
> +
> +       p = value;
> +
> +       do {
> +               name = p;
> +               p = strchr(name, ',');
> +               if (p) {
> +                       *p = '\0';
> +                       p++;
> +               }
> +               if (!strlen(name))
> +                       continue;
> +
> +               register_endpoint(name);
> +       } while (p);
> +
> +       /* SBC is mandatory and should always be registered, even if it's not
> +        * specified in prop
> +        */
> +       register_endpoint("sbc");
> +
>         return AUDIO_STATUS_SUCCESS;
>  }
>
> --
> 1.9.3

Ive stopped pushing here, I believe this makes it a little too
complicated and for now I would prefer we just hardcode the codecs we
support in the order we prefer, this is actually not that simple if we
were to mix with pass-through codecs. Furthermore I believe it is
better to load from a common location so we can just try to load
directly on codec_aptx and if that fails we proceed to the next.


-- 
Luiz Augusto von Dentz
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux