Here is the set od some startup patches for audio hal plugin plus some trivial
fixes which are not RFC actually.
As we want audio plugin to register endpoint we need some special way to
handle it having in mind that audio device is open on the system startup
and closed on system down. We need to handle bluetooth daemon start/stop
in between.
Need your comment on that. For me it does not look bad but we still have
time to consider having some fixed or configurable way (eg. some config file)
to register endpoints in bluetooth daemon instead.
Lukasz Rymanowski (6):
android/ipc: Remove not needed include
android/audio: Fix Makefile.am for libaudio
android: Minor fix to Android Bluetooth Audio protocol API doc
android: Keep stream_out example in the API doc
android/audio: Add wrapper stuct for audio structures
android/audio: Add listener thread on the Audio HAL socket
android/Makefile.am | 5 +-
android/audio-ipc-api.txt | 18 ++--
android/hal-audio.c | 242 ++++++++++++++++++++++++++++++++++++++--------
android/hal-audio.h | 18 ++++
android/ipc.c | 1 -
5 files changed, 231 insertions(+), 53 deletions(-)
create mode 100644 android/hal-audio.h
--
1.8.4
Hi Lukazs,
On Mon, Dec 30, 2013 at 10:57 PM, Lukasz Rymanowski
<[email protected]> wrote:
> Hi Luiz,
>
> On Mon, Dec 30, 2013 at 2:07 PM, Luiz Augusto von Dentz
> <[email protected]> wrote:
>> Hi Lukasz,
>>
>> On Mon, Dec 30, 2013 at 1:40 PM, Lukasz Rymanowski
>> <[email protected]> wrote:
>>> Hi Luiz,
>>>
>>> On 30 December 2013 12:31, Luiz Augusto von Dentz <[email protected]> wrote:
>>>> Hi Lukasz,
>>>>
>>>> On Mon, Dec 30, 2013 at 12:17 PM, Lukasz Rymanowski
>>>> <[email protected]> wrote:
>>>>> This patch add thread which is reponsible for listen on audio HAL
>>>>> socket, register a2dp endpoint(s) and maintain socket.
>>>>> When bluetooth daemon goes down, HAL audio plugin starts to listen on Audio HAL
>>>>> socket again.
>>>>>
>>>>> ---
>>>>> android/Makefile.am | 2 +
>>>>> android/hal-audio.c | 145 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>> android/hal-audio.h | 18 +++++++
>>>>> 3 files changed, 165 insertions(+)
>>>>> create mode 100644 android/hal-audio.h
>>>>>
>>>>> diff --git a/android/Makefile.am b/android/Makefile.am
>>>>> index eaf39bd..bd90c13 100644
>>>>> --- a/android/Makefile.am
>>>>> +++ b/android/Makefile.am
>>>>> @@ -112,6 +112,8 @@ android_libaudio_internal_la_SOURCES = android/hal-audio.c \
>>>>>
>>>>> android_libaudio_internal_la_CFLAGS = -I$(srcdir)/android
>>>>>
>>>>> +android_libaudio_internal_la_LDFLAGS = -pthread
>>>>> +
>>>>> endif
>>>>>
>>>>> EXTRA_DIST += android/Android.mk android/hal-ipc-api.txt android/README \
>>>>> diff --git a/android/hal-audio.c b/android/hal-audio.c
>>>>> index 011a699..0e3bc70 100644
>>>>> --- a/android/hal-audio.c
>>>>> +++ b/android/hal-audio.c
>>>>> @@ -16,18 +16,30 @@
>>>>> */
>>>>>
>>>>> #include <errno.h>
>>>>> +#include <pthread.h>
>>>>> +#include <poll.h>
>>>>> #include <stdio.h>
>>>>> #include <stdlib.h>
>>>>> #include <string.h>
>>>>> +#include <sys/socket.h>
>>>>> +#include <sys/un.h>
>>>>> +#include <unistd.h>
>>>>>
>>>>> #include <hardware/audio.h>
>>>>> #include <hardware/hardware.h>
>>>>>
>>>>> +#include "hal-audio.h"
>>>>> #include "hal-log.h"
>>>>>
>>>>> struct a2dp_audio_dev {
>>>>> struct audio_hw_device dev;
>>>>> struct a2dp_stream_out *stream_out;
>>>>> +
>>>>> + pthread_t bt_watcher;
>>>>> + pthread_mutex_t hal_sk_mutex;
>>>>> + pthread_cond_t bt_watcher_cond;
>>>>> +
>>>>> + int hal_sk;
>>>>> };
>>>>>
>>>>> struct a2dp_stream_out {
>>>>> @@ -384,15 +396,135 @@ static int audio_dump(const audio_hw_device_t *device, int fd)
>>>>>
>>>>> static int audio_close(hw_device_t *device)
>>>>> {
>>>>> + struct audio_hw_device *dev = (struct audio_hw_device *)device;
>>>>> + struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *)dev;
>>>>> +
>>
>> Hmm, Im afraid these are not the same pointers as you do *device =
>> &a2dp_dev->dev.common; so this will probably cause invalid accesses.
>>
> Actually this is same pointer and even I could do here direct cast
> from hw_device_t to a2dp_audio_dev with some comment why I can do it.
> Is that fine?
> Also in audio_open(), to make code less confusing, will do *device =
> (hw_device_t *)a2dp_dev
> Is that fine for you? Not sure how discussion ends or IRC about that
> as I had to go.
It seems the biggest problem with this type of usage of casts is
binary compatibility, but this is a general problem in Android we
can't do anything about and for BlueZ it shouldn't matter that much
since the code should always be available it is just a matter of
recompiling.
--
Luiz Augusto von Dentz
Hi Luiz,
On Mon, Dec 30, 2013 at 2:07 PM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Lukasz,
>
> On Mon, Dec 30, 2013 at 1:40 PM, Lukasz Rymanowski
> <[email protected]> wrote:
>> Hi Luiz,
>>
>> On 30 December 2013 12:31, Luiz Augusto von Dentz <[email protected]> wrote:
>>> Hi Lukasz,
>>>
>>> On Mon, Dec 30, 2013 at 12:17 PM, Lukasz Rymanowski
>>> <[email protected]> wrote:
>>>> This patch add thread which is reponsible for listen on audio HAL
>>>> socket, register a2dp endpoint(s) and maintain socket.
>>>> When bluetooth daemon goes down, HAL audio plugin starts to listen on Audio HAL
>>>> socket again.
>>>>
>>>> ---
>>>> android/Makefile.am | 2 +
>>>> android/hal-audio.c | 145 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>> android/hal-audio.h | 18 +++++++
>>>> 3 files changed, 165 insertions(+)
>>>> create mode 100644 android/hal-audio.h
>>>>
>>>> diff --git a/android/Makefile.am b/android/Makefile.am
>>>> index eaf39bd..bd90c13 100644
>>>> --- a/android/Makefile.am
>>>> +++ b/android/Makefile.am
>>>> @@ -112,6 +112,8 @@ android_libaudio_internal_la_SOURCES = android/hal-audio.c \
>>>>
>>>> android_libaudio_internal_la_CFLAGS = -I$(srcdir)/android
>>>>
>>>> +android_libaudio_internal_la_LDFLAGS = -pthread
>>>> +
>>>> endif
>>>>
>>>> EXTRA_DIST += android/Android.mk android/hal-ipc-api.txt android/README \
>>>> diff --git a/android/hal-audio.c b/android/hal-audio.c
>>>> index 011a699..0e3bc70 100644
>>>> --- a/android/hal-audio.c
>>>> +++ b/android/hal-audio.c
>>>> @@ -16,18 +16,30 @@
>>>> */
>>>>
>>>> #include <errno.h>
>>>> +#include <pthread.h>
>>>> +#include <poll.h>
>>>> #include <stdio.h>
>>>> #include <stdlib.h>
>>>> #include <string.h>
>>>> +#include <sys/socket.h>
>>>> +#include <sys/un.h>
>>>> +#include <unistd.h>
>>>>
>>>> #include <hardware/audio.h>
>>>> #include <hardware/hardware.h>
>>>>
>>>> +#include "hal-audio.h"
>>>> #include "hal-log.h"
>>>>
>>>> struct a2dp_audio_dev {
>>>> struct audio_hw_device dev;
>>>> struct a2dp_stream_out *stream_out;
>>>> +
>>>> + pthread_t bt_watcher;
>>>> + pthread_mutex_t hal_sk_mutex;
>>>> + pthread_cond_t bt_watcher_cond;
>>>> +
>>>> + int hal_sk;
>>>> };
>>>>
>>>> struct a2dp_stream_out {
>>>> @@ -384,15 +396,135 @@ static int audio_dump(const audio_hw_device_t *device, int fd)
>>>>
>>>> static int audio_close(hw_device_t *device)
>>>> {
>>>> + struct audio_hw_device *dev = (struct audio_hw_device *)device;
>>>> + struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *)dev;
>>>> +
>
> Hmm, Im afraid these are not the same pointers as you do *device =
> &a2dp_dev->dev.common; so this will probably cause invalid accesses.
>
Actually this is same pointer and even I could do here direct cast
from hw_device_t to a2dp_audio_dev with some comment why I can do it.
Is that fine?
Also in audio_open(), to make code less confusing, will do *device =
(hw_device_t *)a2dp_dev
Is that fine for you? Not sure how discussion ends or IRC about that
as I had to go.
>>>> DBG("");
>>>> +
>>>> + if (a2dp_dev) {
>>>> + /* TODO: We could try to unregister Endpoint here */
>>>> + shutdown(a2dp_dev->hal_sk, 2);
>>>> +
>>>> + pthread_mutex_lock(&a2dp_dev->hal_sk_mutex);
>>>> + pthread_cond_signal(&a2dp_dev->bt_watcher_cond);
>>>> + pthread_mutex_unlock(&a2dp_dev->hal_sk_mutex);
>>>> +
>>>> + (void) pthread_join(a2dp_dev->bt_watcher, NULL);
>>>> + free(a2dp_dev);
>>>> + }
>>>> +
>>>> free(device);
>>>> return 0;
>>>> }
>>>>
>>>> +static int wait_for_client(void)
>>>> +{
>>>> + struct sockaddr_un addr;
>>>> + int err;
>>>> + int sk;
>>>> + int new_sk;
>>>> +
>>>> + DBG("");
>>>> +
>>>> + sk = socket(PF_LOCAL, SOCK_SEQPACKET, 0);
>>>> + if (sk < 0) {
>>>> + err = errno;
>>>> + error("Failed to create socket: %d (%s)", err, strerror(err));
>>>> + return -1;
>>>> + }
>>>> +
>>>> + memset(&addr, 0, sizeof(addr));
>>>> + addr.sun_family = AF_UNIX;
>>>> +
>>>> + memcpy(addr.sun_path, BLUEZ_AUDIO_HAL_SK_PATH,
>>>> + sizeof(BLUEZ_AUDIO_HAL_SK_PATH));
>>>> +
>>>> + if (bind(sk, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
>>>> + err = errno;
>>>> + error("Failed to bind socket: %d (%s)", err, strerror(err));
>>>> + goto error;
>>>> + }
>>>> +
>>>> + if (listen(sk, 1) < 0) {
>>>> + err = errno;
>>>> + error("Failed to bind socket: %d (%s)", err, strerror(err));
>>>> + goto error;
>>>> + }
>>>> +
>>>> + new_sk = accept(sk, NULL, NULL);
>>>> + if (new_sk < 0) {
>>>> + err = errno;
>>>> + error("Failed to accept socket: %d (%s)", err, strerror(err));
>>>> + goto error;
>>>> + }
>>>> +
>>>> + close(sk);
>>>> + return new_sk;
>>>> +
>>>> +error:
>>>> + close(sk);
>>>> + return -1;
>>>> +}
>>>> +
>>>> +static void *bluetoothd_watcher(void *data)
>>>> +{
>>>> + int err;
>>>> + struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *)data;
>>>> + struct pollfd pfd;
>>>> + struct timeval now;
>>>> + struct timespec timeout;
>>>> +
>>>> + DBG("");
>>>> +
>>>> + while (1) {
>>>> + a2dp_dev->hal_sk = wait_for_client();
>>>> + if (a2dp_dev->hal_sk < 0) {
>>>> + error("Failed to create listening socket");
>>>> + continue;
>>>> + }
>>>> +
>>>> + DBG("Audio IPC: Connected");
>>>> +
>>>> + /* TODO: Register ENDPOINT here */
>>>> +
>>>> + memset(&pfd, 0, sizeof(pfd));
>>>> + pfd.fd = a2dp_dev->hal_sk;
>>>> + pfd.events = POLLHUP | POLLERR | POLLNVAL;
>>>> +
>>>> + pthread_mutex_lock(&a2dp_dev->hal_sk_mutex);
>>>> +
>>>> + /* Check if socket is still alive */
>>>> + err = poll(&pfd, 1, -1);
>>>> + if (pfd.revents & (POLLHUP | POLLERR | POLLNVAL)) {
>>>> + info("Audio HAL: Socket closed");
>>>> + a2dp_dev->hal_sk = -1;
>>>> + }
>>>> +
>>>> + /* Maybe audio system is closing and audio_close() has been called? */
>>>> + gettimeofday(&now, NULL);
>>>> + timeout.tv_sec = now.tv_sec;
>>>> + timeout.tv_nsec = now.tv_usec * 1000;
>>>> + timeout.tv_sec += 1;
>>>> +
>>>> + err = pthread_cond_timedwait(&a2dp_dev->bt_watcher_cond,
>>>> + &a2dp_dev->hal_sk_mutex, &timeout);
>>>> +
>>>> + pthread_mutex_unlock(&a2dp_dev->hal_sk_mutex);
>>>> + if (err != ETIMEDOUT)
>>>> + /* Seems that device has been closed */
>>>> + break;
>>>> + }
>>>> +
>>>> + info("Closing bluetooth_watcher thread");
>>>> + pthread_exit(NULL);
>>>> + return NULL;
>>>> +}
>>>> +
>>>> static int audio_open(const hw_module_t *module, const char *name,
>>>> hw_device_t **device)
>>>> {
>>>> struct a2dp_audio_dev *a2dp_dev;
>>>> + int err;
>>>>
>>>> DBG("");
>>>>
>>>> @@ -427,6 +559,19 @@ static int audio_open(const hw_module_t *module, const char *name,
>>>>
>>>> *device = &a2dp_dev->dev.common;
>>>>
>>>> + a2dp_dev->hal_sk = -1;
>>>> +
>>>> + pthread_mutex_init(&a2dp_dev->hal_sk_mutex, NULL);
>>>> + pthread_cond_init(&a2dp_dev->bt_watcher_cond, NULL);
>>>> +
>>>> + err = pthread_create(&a2dp_dev->bt_watcher, NULL, bluetoothd_watcher, a2dp_dev);
>>>> + if (err < 0) {
>>>> + a2dp_dev->bt_watcher = 0;
>>>> + error("Failed to start bluetoothd watcher thread: %d (%s)", -err,
>>>> + strerror(-err));
>>>> + return (-err);
>>>> + }
>>>> +
>>>> return 0;
>>>> }
>>>>
>>>> diff --git a/android/hal-audio.h b/android/hal-audio.h
>>>> new file mode 100644
>>>> index 0000000..93e49f6
>>>> --- /dev/null
>>>> +++ b/android/hal-audio.h
>>>> @@ -0,0 +1,18 @@
>>>> +/*
>>>> + *
>>>> + * Copyright (C) 2013 Intel Corporation
>>>> + *
>>>> + * Licensed under the Apache License, Version 2.0 (the "License");
>>>> + * you may not use this file except in compliance with the License.
>>>> + * You may obtain a copy of the License at
>>>> + *
>>>> + * http://www.apache.org/licenses/LICENSE-2.0
>>>> + *
>>>> + * Unless required by applicable law or agreed to in writing, software
>>>> + * distributed under the License is distributed on an "AS IS" BASIS,
>>>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>>>> + * See the License for the specific language governing permissions and
>>>> + * limitations under the License.
>>>> + *
>>>> + */
>>>> +static const char BLUEZ_AUDIO_HAL_SK_PATH[] = "\0bluez_audio_socket";
>>>> --
>>>> 1.8.4
>>>
>>> Ive started working on the daemon side and should be able to post some
>>> patches today, Im not really sure we are going to need a thread for
>>> command handling since all the commands will be sent by the plugin
>>> side so they can be synchronous, the only problem would be if the
>>> plugin cannot really block even for a short period.
>>>
>>
>> This Thread is only going to be used for accept, endpoint register and
>> then listening for socket close (so we can start listen for connection
>> from new bluetoothd).
>>
>> Commands and response will be handled in main thread (ex.
>> audio_open(), out_write()) and we can block there for a while - no
>> problem with that
>
> Well it seems the init/listen stage needs to be non-blocking so we
> don't interrupt the audio init procedure which may block bluetooth HAL
> to initialized, the endpoint then should be registered whenever
> bluetoothd connects.
>
>
\Łukasz
> --
> Luiz Augusto von Dentz
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Lukasz,
On Mon, Dec 30, 2013 at 1:40 PM, Lukasz Rymanowski
<[email protected]> wrote:
> Hi Luiz,
>
> On 30 December 2013 12:31, Luiz Augusto von Dentz <[email protected]> wrote:
>> Hi Lukasz,
>>
>> On Mon, Dec 30, 2013 at 12:17 PM, Lukasz Rymanowski
>> <[email protected]> wrote:
>>> This patch add thread which is reponsible for listen on audio HAL
>>> socket, register a2dp endpoint(s) and maintain socket.
>>> When bluetooth daemon goes down, HAL audio plugin starts to listen on Audio HAL
>>> socket again.
>>>
>>> ---
>>> android/Makefile.am | 2 +
>>> android/hal-audio.c | 145 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> android/hal-audio.h | 18 +++++++
>>> 3 files changed, 165 insertions(+)
>>> create mode 100644 android/hal-audio.h
>>>
>>> diff --git a/android/Makefile.am b/android/Makefile.am
>>> index eaf39bd..bd90c13 100644
>>> --- a/android/Makefile.am
>>> +++ b/android/Makefile.am
>>> @@ -112,6 +112,8 @@ android_libaudio_internal_la_SOURCES = android/hal-audio.c \
>>>
>>> android_libaudio_internal_la_CFLAGS = -I$(srcdir)/android
>>>
>>> +android_libaudio_internal_la_LDFLAGS = -pthread
>>> +
>>> endif
>>>
>>> EXTRA_DIST += android/Android.mk android/hal-ipc-api.txt android/README \
>>> diff --git a/android/hal-audio.c b/android/hal-audio.c
>>> index 011a699..0e3bc70 100644
>>> --- a/android/hal-audio.c
>>> +++ b/android/hal-audio.c
>>> @@ -16,18 +16,30 @@
>>> */
>>>
>>> #include <errno.h>
>>> +#include <pthread.h>
>>> +#include <poll.h>
>>> #include <stdio.h>
>>> #include <stdlib.h>
>>> #include <string.h>
>>> +#include <sys/socket.h>
>>> +#include <sys/un.h>
>>> +#include <unistd.h>
>>>
>>> #include <hardware/audio.h>
>>> #include <hardware/hardware.h>
>>>
>>> +#include "hal-audio.h"
>>> #include "hal-log.h"
>>>
>>> struct a2dp_audio_dev {
>>> struct audio_hw_device dev;
>>> struct a2dp_stream_out *stream_out;
>>> +
>>> + pthread_t bt_watcher;
>>> + pthread_mutex_t hal_sk_mutex;
>>> + pthread_cond_t bt_watcher_cond;
>>> +
>>> + int hal_sk;
>>> };
>>>
>>> struct a2dp_stream_out {
>>> @@ -384,15 +396,135 @@ static int audio_dump(const audio_hw_device_t *device, int fd)
>>>
>>> static int audio_close(hw_device_t *device)
>>> {
>>> + struct audio_hw_device *dev = (struct audio_hw_device *)device;
>>> + struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *)dev;
>>> +
Hmm, Im afraid these are not the same pointers as you do *device =
&a2dp_dev->dev.common; so this will probably cause invalid accesses.
>>> DBG("");
>>> +
>>> + if (a2dp_dev) {
>>> + /* TODO: We could try to unregister Endpoint here */
>>> + shutdown(a2dp_dev->hal_sk, 2);
>>> +
>>> + pthread_mutex_lock(&a2dp_dev->hal_sk_mutex);
>>> + pthread_cond_signal(&a2dp_dev->bt_watcher_cond);
>>> + pthread_mutex_unlock(&a2dp_dev->hal_sk_mutex);
>>> +
>>> + (void) pthread_join(a2dp_dev->bt_watcher, NULL);
>>> + free(a2dp_dev);
>>> + }
>>> +
>>> free(device);
>>> return 0;
>>> }
>>>
>>> +static int wait_for_client(void)
>>> +{
>>> + struct sockaddr_un addr;
>>> + int err;
>>> + int sk;
>>> + int new_sk;
>>> +
>>> + DBG("");
>>> +
>>> + sk = socket(PF_LOCAL, SOCK_SEQPACKET, 0);
>>> + if (sk < 0) {
>>> + err = errno;
>>> + error("Failed to create socket: %d (%s)", err, strerror(err));
>>> + return -1;
>>> + }
>>> +
>>> + memset(&addr, 0, sizeof(addr));
>>> + addr.sun_family = AF_UNIX;
>>> +
>>> + memcpy(addr.sun_path, BLUEZ_AUDIO_HAL_SK_PATH,
>>> + sizeof(BLUEZ_AUDIO_HAL_SK_PATH));
>>> +
>>> + if (bind(sk, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
>>> + err = errno;
>>> + error("Failed to bind socket: %d (%s)", err, strerror(err));
>>> + goto error;
>>> + }
>>> +
>>> + if (listen(sk, 1) < 0) {
>>> + err = errno;
>>> + error("Failed to bind socket: %d (%s)", err, strerror(err));
>>> + goto error;
>>> + }
>>> +
>>> + new_sk = accept(sk, NULL, NULL);
>>> + if (new_sk < 0) {
>>> + err = errno;
>>> + error("Failed to accept socket: %d (%s)", err, strerror(err));
>>> + goto error;
>>> + }
>>> +
>>> + close(sk);
>>> + return new_sk;
>>> +
>>> +error:
>>> + close(sk);
>>> + return -1;
>>> +}
>>> +
>>> +static void *bluetoothd_watcher(void *data)
>>> +{
>>> + int err;
>>> + struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *)data;
>>> + struct pollfd pfd;
>>> + struct timeval now;
>>> + struct timespec timeout;
>>> +
>>> + DBG("");
>>> +
>>> + while (1) {
>>> + a2dp_dev->hal_sk = wait_for_client();
>>> + if (a2dp_dev->hal_sk < 0) {
>>> + error("Failed to create listening socket");
>>> + continue;
>>> + }
>>> +
>>> + DBG("Audio IPC: Connected");
>>> +
>>> + /* TODO: Register ENDPOINT here */
>>> +
>>> + memset(&pfd, 0, sizeof(pfd));
>>> + pfd.fd = a2dp_dev->hal_sk;
>>> + pfd.events = POLLHUP | POLLERR | POLLNVAL;
>>> +
>>> + pthread_mutex_lock(&a2dp_dev->hal_sk_mutex);
>>> +
>>> + /* Check if socket is still alive */
>>> + err = poll(&pfd, 1, -1);
>>> + if (pfd.revents & (POLLHUP | POLLERR | POLLNVAL)) {
>>> + info("Audio HAL: Socket closed");
>>> + a2dp_dev->hal_sk = -1;
>>> + }
>>> +
>>> + /* Maybe audio system is closing and audio_close() has been called? */
>>> + gettimeofday(&now, NULL);
>>> + timeout.tv_sec = now.tv_sec;
>>> + timeout.tv_nsec = now.tv_usec * 1000;
>>> + timeout.tv_sec += 1;
>>> +
>>> + err = pthread_cond_timedwait(&a2dp_dev->bt_watcher_cond,
>>> + &a2dp_dev->hal_sk_mutex, &timeout);
>>> +
>>> + pthread_mutex_unlock(&a2dp_dev->hal_sk_mutex);
>>> + if (err != ETIMEDOUT)
>>> + /* Seems that device has been closed */
>>> + break;
>>> + }
>>> +
>>> + info("Closing bluetooth_watcher thread");
>>> + pthread_exit(NULL);
>>> + return NULL;
>>> +}
>>> +
>>> static int audio_open(const hw_module_t *module, const char *name,
>>> hw_device_t **device)
>>> {
>>> struct a2dp_audio_dev *a2dp_dev;
>>> + int err;
>>>
>>> DBG("");
>>>
>>> @@ -427,6 +559,19 @@ static int audio_open(const hw_module_t *module, const char *name,
>>>
>>> *device = &a2dp_dev->dev.common;
>>>
>>> + a2dp_dev->hal_sk = -1;
>>> +
>>> + pthread_mutex_init(&a2dp_dev->hal_sk_mutex, NULL);
>>> + pthread_cond_init(&a2dp_dev->bt_watcher_cond, NULL);
>>> +
>>> + err = pthread_create(&a2dp_dev->bt_watcher, NULL, bluetoothd_watcher, a2dp_dev);
>>> + if (err < 0) {
>>> + a2dp_dev->bt_watcher = 0;
>>> + error("Failed to start bluetoothd watcher thread: %d (%s)", -err,
>>> + strerror(-err));
>>> + return (-err);
>>> + }
>>> +
>>> return 0;
>>> }
>>>
>>> diff --git a/android/hal-audio.h b/android/hal-audio.h
>>> new file mode 100644
>>> index 0000000..93e49f6
>>> --- /dev/null
>>> +++ b/android/hal-audio.h
>>> @@ -0,0 +1,18 @@
>>> +/*
>>> + *
>>> + * Copyright (C) 2013 Intel Corporation
>>> + *
>>> + * Licensed under the Apache License, Version 2.0 (the "License");
>>> + * you may not use this file except in compliance with the License.
>>> + * You may obtain a copy of the License at
>>> + *
>>> + * http://www.apache.org/licenses/LICENSE-2.0
>>> + *
>>> + * Unless required by applicable law or agreed to in writing, software
>>> + * distributed under the License is distributed on an "AS IS" BASIS,
>>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>>> + * See the License for the specific language governing permissions and
>>> + * limitations under the License.
>>> + *
>>> + */
>>> +static const char BLUEZ_AUDIO_HAL_SK_PATH[] = "\0bluez_audio_socket";
>>> --
>>> 1.8.4
>>
>> Ive started working on the daemon side and should be able to post some
>> patches today, Im not really sure we are going to need a thread for
>> command handling since all the commands will be sent by the plugin
>> side so they can be synchronous, the only problem would be if the
>> plugin cannot really block even for a short period.
>>
>
> This Thread is only going to be used for accept, endpoint register and
> then listening for socket close (so we can start listen for connection
> from new bluetoothd).
>
> Commands and response will be handled in main thread (ex.
> audio_open(), out_write()) and we can block there for a while - no
> problem with that
Well it seems the init/listen stage needs to be non-blocking so we
don't interrupt the audio init procedure which may block bluetooth HAL
to initialized, the endpoint then should be registered whenever
bluetoothd connects.
--
Luiz Augusto von Dentz
Hi Lukasz,
On Mon, Dec 30, 2013 at 2:37 PM, Lukasz Rymanowski
<[email protected]> wrote:
> Hi Luiz,
>
> On 30 December 2013 13:04, Luiz Augusto von Dentz <[email protected]> wrote:
>> Hi Lukasz,
>>
>> On Mon, Dec 30, 2013 at 1:33 PM, Lukasz Rymanowski
>> <[email protected]> wrote:
>>> Hi Luiz,
>>>
>>> On 30 December 2013 12:27, Luiz Augusto von Dentz <[email protected]> wrote:
>>>> Hi Lukasz,
>>>>
>>>> On Mon, Dec 30, 2013 at 12:17 PM, Lukasz Rymanowski
>>>> <[email protected]> wrote:
>>>>> This patch add wrapping struct for audio_hw_dev and audio_stream_out.
>>>>> We will need this to keep some more addition info related to a2dp stream
>>>>> and also hal IPC later on
>>>>>
>>>>> ---
>>>>> android/Makefile.am | 1 +
>>>>> android/hal-audio.c | 97 +++++++++++++++++++++++++++++------------------------
>>>>> 2 files changed, 55 insertions(+), 43 deletions(-)
>>>>>
>>>>> diff --git a/android/Makefile.am b/android/Makefile.am
>>>>> index dec81ce..eaf39bd 100644
>>>>> --- a/android/Makefile.am
>>>>> +++ b/android/Makefile.am
>>>>> @@ -104,6 +104,7 @@ android_android_tester_LDFLAGS = -pthread
>>>>> noinst_LTLIBRARIES += android/libaudio-internal.la
>>>>>
>>>>> android_libaudio_internal_la_SOURCES = android/hal-audio.c \
>>>>> + android/hal-audio.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 7f4a3f2..011a699 100644
>>>>> --- a/android/hal-audio.c
>>>>> +++ b/android/hal-audio.c
>>>>> @@ -25,6 +25,15 @@
>>>>>
>>>>> #include "hal-log.h"
>>>>>
>>>>> +struct a2dp_audio_dev {
>>>>> + struct audio_hw_device dev;
>>>>> + struct a2dp_stream_out *stream_out;
>>>>> +};
>>>>> +
>>>>> +struct a2dp_stream_out {
>>>>> + struct audio_stream_out stream;
>>>>> +};
>>>>> +
>>>>> static ssize_t out_write(struct audio_stream_out *stream, const void *buffer,
>>>>> size_t bytes)
>>>>> {
>>>>> @@ -230,32 +239,34 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
>>>>> struct audio_stream_out **stream_out)
>>>>>
>>>>> {
>>>>> - struct audio_stream_out *out;
>>>>> + struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *)dev;
>>>>> + struct a2dp_stream_out *a2dp_out;
>>>>>
>>>>> - out = calloc(1, sizeof(struct audio_stream_out));
>>>>> - if (!out)
>>>>> + a2dp_out = calloc(1, sizeof(struct a2dp_stream_out));
>>>>> + if (!a2dp_out)
>>>>> return -ENOMEM;
>>>>>
>>>>> DBG("");
>>>>>
>>>>> - out->common.get_sample_rate = out_get_sample_rate;
>>>>> - out->common.set_sample_rate = out_set_sample_rate;
>>>>> - out->common.get_buffer_size = out_get_buffer_size;
>>>>> - out->common.get_channels = out_get_channels;
>>>>> - out->common.get_format = out_get_format;
>>>>> - out->common.set_format = out_set_format;
>>>>> - out->common.standby = out_standby;
>>>>> - out->common.dump = out_dump;
>>>>> - out->common.set_parameters = out_set_parameters;
>>>>> - out->common.get_parameters = out_get_parameters;
>>>>> - out->common.add_audio_effect = out_add_audio_effect;
>>>>> - out->common.remove_audio_effect = out_remove_audio_effect;
>>>>> - out->get_latency = out_get_latency;
>>>>> - out->set_volume = out_set_volume;
>>>>> - out->write = out_write;
>>>>> - out->get_render_position = out_get_render_position;
>>>>> + a2dp_out->stream.common.get_sample_rate = out_get_sample_rate;
>>>>> + a2dp_out->stream.common.set_sample_rate = out_set_sample_rate;
>>>>> + a2dp_out->stream.common.get_buffer_size = out_get_buffer_size;
>>>>> + a2dp_out->stream.common.get_channels = out_get_channels;
>>>>> + a2dp_out->stream.common.get_format = out_get_format;
>>>>> + a2dp_out->stream.common.set_format = out_set_format;
>>>>> + a2dp_out->stream.common.standby = out_standby;
>>>>> + a2dp_out->stream.common.dump = out_dump;
>>>>> + a2dp_out->stream.common.set_parameters = out_set_parameters;
>>>>> + a2dp_out->stream.common.get_parameters = out_get_parameters;
>>>>> + a2dp_out->stream.common.add_audio_effect = out_add_audio_effect;
>>>>> + a2dp_out->stream.common.remove_audio_effect = out_remove_audio_effect;
>>>>> + a2dp_out->stream.get_latency = out_get_latency;
>>>>> + a2dp_out->stream.set_volume = out_set_volume;
>>>>> + a2dp_out->stream.write = out_write;
>>>>> + a2dp_out->stream.get_render_position = out_get_render_position;
>>>>>
>>>>> - *stream_out = out;
>>>>> + *stream_out = &a2dp_out->stream;
>>>>> + a2dp_dev->stream_out = a2dp_out;
>>>>>
>>>>> return 0;
>>>>> }
>>>>> @@ -381,7 +392,7 @@ static int audio_close(hw_device_t *device)
>>>>> static int audio_open(const hw_module_t *module, const char *name,
>>>>> hw_device_t **device)
>>>>> {
>>>>> - struct audio_hw_device *audio;
>>>>> + struct a2dp_audio_dev *a2dp_dev;
>>>>>
>>>>> DBG("");
>>>>>
>>>>> @@ -391,30 +402,30 @@ static int audio_open(const hw_module_t *module, const char *name,
>>>>> return -EINVAL;
>>>>> }
>>>>>
>>>>> - audio = calloc(1, sizeof(struct audio_hw_device));
>>>>> - if (!audio)
>>>>> + a2dp_dev = calloc(1, sizeof(struct a2dp_audio_dev));
>>>>> + if (!a2dp_dev)
>>>>> return -ENOMEM;
>>>>>
>>>>> - audio->common.version = AUDIO_DEVICE_API_VERSION_CURRENT;
>>>>> - audio->common.module = (struct hw_module_t *) module;
>>>>> - audio->common.close = audio_close;
>>>>> -
>>>>> - audio->init_check = audio_init_check;
>>>>> - audio->set_voice_volume = audio_set_voice_volume;
>>>>> - audio->set_master_volume = audio_set_master_volume
>>>>> - audio->set_mode = audio_set_mode;
>>>>> - audio->set_mic_mute = audio_set_mic_mute;
>>>>> - audio->get_mic_mute = audio_get_mic_mute;
>>>>> - audio->set_parameters = audio_set_parameters;
>>>>> - audio->get_parameters = audio_get_parameters;
>>>>> - audio->get_input_buffer_size = audio_get_input_buffer_size;
>>>>> - audio->open_output_stream = audio_open_output_stream;
>>>>> - audio->close_output_stream = audio_close_output_stream;
>>>>> - audio->open_input_stream = audio_open_input_stream;
>>>>> - audio->close_input_stream = audio_close_input_stream;
>>>>> - audio->dump = audio_dump;
>>>>> -
>>>>> - *device = &audio->common;
>>>>> + a2dp_dev->dev.common.version = AUDIO_DEVICE_API_VERSION_CURRENT;
>>>>> + a2dp_dev->dev.common.module = (struct hw_module_t *) module;
>>>>> + a2dp_dev->dev.common.close = audio_close;
>>>>> +
>>>>> + a2dp_dev->dev.init_check = audio_init_check;
>>>>> + a2dp_dev->dev.set_voice_volume = audio_set_voice_volume;
>>>>> + a2dp_dev->dev.set_master_volume = audio_set_master_volume;
>>>>> + a2dp_dev->dev.set_mode = audio_set_mode;
>>>>> + a2dp_dev->dev.set_mic_mute = audio_set_mic_mute;
>>>>> + a2dp_dev->dev.get_mic_mute = audio_get_mic_mute;
>>>>> + a2dp_dev->dev.set_parameters = audio_set_parameters;
>>>>> + a2dp_dev->dev.get_parameters = audio_get_parameters;
>>>>> + a2dp_dev->dev.get_input_buffer_size = audio_get_input_buffer_size;
>>>>> + a2dp_dev->dev.open_output_stream = audio_open_output_stream;
>>>>> + a2dp_dev->dev.close_output_stream = audio_close_output_stream;
>>>>> + a2dp_dev->dev.open_input_stream = audio_open_input_stream;
>>>>> + a2dp_dev->dev.close_input_stream = audio_close_input_stream;
>>>>> + a2dp_dev->dev.dump = audio_dump;
>>>>> +
>>>>> + *device = &a2dp_dev->dev.common;
>>>>>
>>>>> return 0;
>>>>> }
>>>>> --
>>>>> 1.8.4
>>>>
>>>> How is this supposed to be accessed, are you planning on adding a
>>>> global variable?
>>>>
>>>>
>>> Have a look on audio_open_output_stream(). This is actually how they did so
>>> far and it is fine for me.
>>> I want to avoid globals here.
>>
>> I can see that you allocate a different struct for a2dp_dev and
>> a2dp_out, but then you return a member of the struct back to HAL but
>> after that you wont access these structs anymore and the HAL only uses
>> the members that you return so you probably gonna need to lookup for
>> these structs to access any auxiliary data that we might want to add,
>> also these structs never seems to be freed which I suppose can
>> generate memory leaks.
>>
>>
> You right I missed free of a2dp_out
> It will be done in audio_close_output_stream(). Access to that will
> be similar as to a2dp_dev() in audio_close()
Well it doesn't seems you are changing audio_close, in fact the only
thing that audio_close frees is hw_device_t *device not struct
a2dp_audio_dev so you still have to deal with mapping access between
those struct and it doesn't seems the HAL have any concept of
user_data to attach so this normally indicates we need to somehow
store pointer to access latter.
--
Luiz Augusto von Dentz
Hi Luiz,
On 30 December 2013 13:04, Luiz Augusto von Dentz <[email protected]> wrote:
> Hi Lukasz,
>
> On Mon, Dec 30, 2013 at 1:33 PM, Lukasz Rymanowski
> <[email protected]> wrote:
>> Hi Luiz,
>>
>> On 30 December 2013 12:27, Luiz Augusto von Dentz <[email protected]> wrote:
>>> Hi Lukasz,
>>>
>>> On Mon, Dec 30, 2013 at 12:17 PM, Lukasz Rymanowski
>>> <[email protected]> wrote:
>>>> This patch add wrapping struct for audio_hw_dev and audio_stream_out.
>>>> We will need this to keep some more addition info related to a2dp stream
>>>> and also hal IPC later on
>>>>
>>>> ---
>>>> android/Makefile.am | 1 +
>>>> android/hal-audio.c | 97 +++++++++++++++++++++++++++++------------------------
>>>> 2 files changed, 55 insertions(+), 43 deletions(-)
>>>>
>>>> diff --git a/android/Makefile.am b/android/Makefile.am
>>>> index dec81ce..eaf39bd 100644
>>>> --- a/android/Makefile.am
>>>> +++ b/android/Makefile.am
>>>> @@ -104,6 +104,7 @@ android_android_tester_LDFLAGS = -pthread
>>>> noinst_LTLIBRARIES += android/libaudio-internal.la
>>>>
>>>> android_libaudio_internal_la_SOURCES = android/hal-audio.c \
>>>> + android/hal-audio.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 7f4a3f2..011a699 100644
>>>> --- a/android/hal-audio.c
>>>> +++ b/android/hal-audio.c
>>>> @@ -25,6 +25,15 @@
>>>>
>>>> #include "hal-log.h"
>>>>
>>>> +struct a2dp_audio_dev {
>>>> + struct audio_hw_device dev;
>>>> + struct a2dp_stream_out *stream_out;
>>>> +};
>>>> +
>>>> +struct a2dp_stream_out {
>>>> + struct audio_stream_out stream;
>>>> +};
>>>> +
>>>> static ssize_t out_write(struct audio_stream_out *stream, const void *buffer,
>>>> size_t bytes)
>>>> {
>>>> @@ -230,32 +239,34 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
>>>> struct audio_stream_out **stream_out)
>>>>
>>>> {
>>>> - struct audio_stream_out *out;
>>>> + struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *)dev;
>>>> + struct a2dp_stream_out *a2dp_out;
>>>>
>>>> - out = calloc(1, sizeof(struct audio_stream_out));
>>>> - if (!out)
>>>> + a2dp_out = calloc(1, sizeof(struct a2dp_stream_out));
>>>> + if (!a2dp_out)
>>>> return -ENOMEM;
>>>>
>>>> DBG("");
>>>>
>>>> - out->common.get_sample_rate = out_get_sample_rate;
>>>> - out->common.set_sample_rate = out_set_sample_rate;
>>>> - out->common.get_buffer_size = out_get_buffer_size;
>>>> - out->common.get_channels = out_get_channels;
>>>> - out->common.get_format = out_get_format;
>>>> - out->common.set_format = out_set_format;
>>>> - out->common.standby = out_standby;
>>>> - out->common.dump = out_dump;
>>>> - out->common.set_parameters = out_set_parameters;
>>>> - out->common.get_parameters = out_get_parameters;
>>>> - out->common.add_audio_effect = out_add_audio_effect;
>>>> - out->common.remove_audio_effect = out_remove_audio_effect;
>>>> - out->get_latency = out_get_latency;
>>>> - out->set_volume = out_set_volume;
>>>> - out->write = out_write;
>>>> - out->get_render_position = out_get_render_position;
>>>> + a2dp_out->stream.common.get_sample_rate = out_get_sample_rate;
>>>> + a2dp_out->stream.common.set_sample_rate = out_set_sample_rate;
>>>> + a2dp_out->stream.common.get_buffer_size = out_get_buffer_size;
>>>> + a2dp_out->stream.common.get_channels = out_get_channels;
>>>> + a2dp_out->stream.common.get_format = out_get_format;
>>>> + a2dp_out->stream.common.set_format = out_set_format;
>>>> + a2dp_out->stream.common.standby = out_standby;
>>>> + a2dp_out->stream.common.dump = out_dump;
>>>> + a2dp_out->stream.common.set_parameters = out_set_parameters;
>>>> + a2dp_out->stream.common.get_parameters = out_get_parameters;
>>>> + a2dp_out->stream.common.add_audio_effect = out_add_audio_effect;
>>>> + a2dp_out->stream.common.remove_audio_effect = out_remove_audio_effect;
>>>> + a2dp_out->stream.get_latency = out_get_latency;
>>>> + a2dp_out->stream.set_volume = out_set_volume;
>>>> + a2dp_out->stream.write = out_write;
>>>> + a2dp_out->stream.get_render_position = out_get_render_position;
>>>>
>>>> - *stream_out = out;
>>>> + *stream_out = &a2dp_out->stream;
>>>> + a2dp_dev->stream_out = a2dp_out;
>>>>
>>>> return 0;
>>>> }
>>>> @@ -381,7 +392,7 @@ static int audio_close(hw_device_t *device)
>>>> static int audio_open(const hw_module_t *module, const char *name,
>>>> hw_device_t **device)
>>>> {
>>>> - struct audio_hw_device *audio;
>>>> + struct a2dp_audio_dev *a2dp_dev;
>>>>
>>>> DBG("");
>>>>
>>>> @@ -391,30 +402,30 @@ static int audio_open(const hw_module_t *module, const char *name,
>>>> return -EINVAL;
>>>> }
>>>>
>>>> - audio = calloc(1, sizeof(struct audio_hw_device));
>>>> - if (!audio)
>>>> + a2dp_dev = calloc(1, sizeof(struct a2dp_audio_dev));
>>>> + if (!a2dp_dev)
>>>> return -ENOMEM;
>>>>
>>>> - audio->common.version = AUDIO_DEVICE_API_VERSION_CURRENT;
>>>> - audio->common.module = (struct hw_module_t *) module;
>>>> - audio->common.close = audio_close;
>>>> -
>>>> - audio->init_check = audio_init_check;
>>>> - audio->set_voice_volume = audio_set_voice_volume;
>>>> - audio->set_master_volume = audio_set_master_volume;
>>>> - audio->set_mode = audio_set_mode;
>>>> - audio->set_mic_mute = audio_set_mic_mute;
>>>> - audio->get_mic_mute = audio_get_mic_mute;
>>>> - audio->set_parameters = audio_set_parameters;
>>>> - audio->get_parameters = audio_get_parameters;
>>>> - audio->get_input_buffer_size = audio_get_input_buffer_size;
>>>> - audio->open_output_stream = audio_open_output_stream;
>>>> - audio->close_output_stream = audio_close_output_stream;
>>>> - audio->open_input_stream = audio_open_input_stream;
>>>> - audio->close_input_stream = audio_close_input_stream;
>>>> - audio->dump = audio_dump;
>>>> -
>>>> - *device = &audio->common;
>>>> + a2dp_dev->dev.common.version = AUDIO_DEVICE_API_VERSION_CURRENT;
>>>> + a2dp_dev->dev.common.module = (struct hw_module_t *) module;
>>>> + a2dp_dev->dev.common.close = audio_close;
>>>> +
>>>> + a2dp_dev->dev.init_check = audio_init_check;
>>>> + a2dp_dev->dev.set_voice_volume = audio_set_voice_volume;
>>>> + a2dp_dev->dev.set_master_volume = audio_set_master_volume;
>>>> + a2dp_dev->dev.set_mode = audio_set_mode;
>>>> + a2dp_dev->dev.set_mic_mute = audio_set_mic_mute;
>>>> + a2dp_dev->dev.get_mic_mute = audio_get_mic_mute;
>>>> + a2dp_dev->dev.set_parameters = audio_set_parameters;
>>>> + a2dp_dev->dev.get_parameters = audio_get_parameters;
>>>> + a2dp_dev->dev.get_input_buffer_size = audio_get_input_buffer_size;
>>>> + a2dp_dev->dev.open_output_stream = audio_open_output_stream;
>>>> + a2dp_dev->dev.close_output_stream = audio_close_output_stream;
>>>> + a2dp_dev->dev.open_input_stream = audio_open_input_stream;
>>>> + a2dp_dev->dev.close_input_stream = audio_close_input_stream;
>>>> + a2dp_dev->dev.dump = audio_dump;
>>>> +
>>>> + *device = &a2dp_dev->dev.common;
>>>>
>>>> return 0;
>>>> }
>>>> --
>>>> 1.8.4
>>>
>>> How is this supposed to be accessed, are you planning on adding a
>>> global variable?
>>>
>>>
>> Have a look on audio_open_output_stream(). This is actually how they did so
>> far and it is fine for me.
>> I want to avoid globals here.
>
> I can see that you allocate a different struct for a2dp_dev and
> a2dp_out, but then you return a member of the struct back to HAL but
> after that you wont access these structs anymore and the HAL only uses
> the members that you return so you probably gonna need to lookup for
> these structs to access any auxiliary data that we might want to add,
> also these structs never seems to be freed which I suppose can
> generate memory leaks.
>
>
You right I missed free of a2dp_out
It will be done in audio_close_output_stream(). Access to that will
be similar as to a2dp_dev() in audio_close()
> --
> Luiz Augusto von Dentz
Hi Lukasz,
On Mon, Dec 30, 2013 at 12:17 PM, Lukasz Rymanowski
<[email protected]> wrote:
> Here is the set od some startup patches for audio hal plugin plus some trivial
> fixes which are not RFC actually.
>
> As we want audio plugin to register endpoint we need some special way to
> handle it having in mind that audio device is open on the system startup
> and closed on system down. We need to handle bluetooth daemon start/stop
> in between.
>
> Need your comment on that. For me it does not look bad but we still have
> time to consider having some fixed or configurable way (eg. some config file)
> to register endpoints in bluetooth daemon instead.
>
> Lukasz Rymanowski (6):
> android/ipc: Remove not needed include
> android/audio: Fix Makefile.am for libaudio
> android: Minor fix to Android Bluetooth Audio protocol API doc
> android: Keep stream_out example in the API doc
> android/audio: Add wrapper stuct for audio structures
> android/audio: Add listener thread on the Audio HAL socket
>
> android/Makefile.am | 5 +-
> android/audio-ipc-api.txt | 18 ++--
> android/hal-audio.c | 242 ++++++++++++++++++++++++++++++++++++++--------
> android/hal-audio.h | 18 ++++
> android/ipc.c | 1 -
> 5 files changed, 231 insertions(+), 53 deletions(-)
> create mode 100644 android/hal-audio.h
>
> --
> 1.8.4
Patches 1--4 are now upstream, thanks. For patches 5 and 6 lets
continue to discuss what is the best way to do the socket handling.
--
Luiz Augusto von Dentz
Hi Lukasz,
On Mon, Dec 30, 2013 at 1:33 PM, Lukasz Rymanowski
<[email protected]> wrote:
> Hi Luiz,
>
> On 30 December 2013 12:27, Luiz Augusto von Dentz <[email protected]> wrote:
>> Hi Lukasz,
>>
>> On Mon, Dec 30, 2013 at 12:17 PM, Lukasz Rymanowski
>> <[email protected]> wrote:
>>> This patch add wrapping struct for audio_hw_dev and audio_stream_out.
>>> We will need this to keep some more addition info related to a2dp stream
>>> and also hal IPC later on
>>>
>>> ---
>>> android/Makefile.am | 1 +
>>> android/hal-audio.c | 97 +++++++++++++++++++++++++++++------------------------
>>> 2 files changed, 55 insertions(+), 43 deletions(-)
>>>
>>> diff --git a/android/Makefile.am b/android/Makefile.am
>>> index dec81ce..eaf39bd 100644
>>> --- a/android/Makefile.am
>>> +++ b/android/Makefile.am
>>> @@ -104,6 +104,7 @@ android_android_tester_LDFLAGS = -pthread
>>> noinst_LTLIBRARIES += android/libaudio-internal.la
>>>
>>> android_libaudio_internal_la_SOURCES = android/hal-audio.c \
>>> + android/hal-audio.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 7f4a3f2..011a699 100644
>>> --- a/android/hal-audio.c
>>> +++ b/android/hal-audio.c
>>> @@ -25,6 +25,15 @@
>>>
>>> #include "hal-log.h"
>>>
>>> +struct a2dp_audio_dev {
>>> + struct audio_hw_device dev;
>>> + struct a2dp_stream_out *stream_out;
>>> +};
>>> +
>>> +struct a2dp_stream_out {
>>> + struct audio_stream_out stream;
>>> +};
>>> +
>>> static ssize_t out_write(struct audio_stream_out *stream, const void *buffer,
>>> size_t bytes)
>>> {
>>> @@ -230,32 +239,34 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
>>> struct audio_stream_out **stream_out)
>>>
>>> {
>>> - struct audio_stream_out *out;
>>> + struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *)dev;
>>> + struct a2dp_stream_out *a2dp_out;
>>>
>>> - out = calloc(1, sizeof(struct audio_stream_out));
>>> - if (!out)
>>> + a2dp_out = calloc(1, sizeof(struct a2dp_stream_out));
>>> + if (!a2dp_out)
>>> return -ENOMEM;
>>>
>>> DBG("");
>>>
>>> - out->common.get_sample_rate = out_get_sample_rate;
>>> - out->common.set_sample_rate = out_set_sample_rate;
>>> - out->common.get_buffer_size = out_get_buffer_size;
>>> - out->common.get_channels = out_get_channels;
>>> - out->common.get_format = out_get_format;
>>> - out->common.set_format = out_set_format;
>>> - out->common.standby = out_standby;
>>> - out->common.dump = out_dump;
>>> - out->common.set_parameters = out_set_parameters;
>>> - out->common.get_parameters = out_get_parameters;
>>> - out->common.add_audio_effect = out_add_audio_effect;
>>> - out->common.remove_audio_effect = out_remove_audio_effect;
>>> - out->get_latency = out_get_latency;
>>> - out->set_volume = out_set_volume;
>>> - out->write = out_write;
>>> - out->get_render_position = out_get_render_position;
>>> + a2dp_out->stream.common.get_sample_rate = out_get_sample_rate;
>>> + a2dp_out->stream.common.set_sample_rate = out_set_sample_rate;
>>> + a2dp_out->stream.common.get_buffer_size = out_get_buffer_size;
>>> + a2dp_out->stream.common.get_channels = out_get_channels;
>>> + a2dp_out->stream.common.get_format = out_get_format;
>>> + a2dp_out->stream.common.set_format = out_set_format;
>>> + a2dp_out->stream.common.standby = out_standby;
>>> + a2dp_out->stream.common.dump = out_dump;
>>> + a2dp_out->stream.common.set_parameters = out_set_parameters;
>>> + a2dp_out->stream.common.get_parameters = out_get_parameters;
>>> + a2dp_out->stream.common.add_audio_effect = out_add_audio_effect;
>>> + a2dp_out->stream.common.remove_audio_effect = out_remove_audio_effect;
>>> + a2dp_out->stream.get_latency = out_get_latency;
>>> + a2dp_out->stream.set_volume = out_set_volume;
>>> + a2dp_out->stream.write = out_write;
>>> + a2dp_out->stream.get_render_position = out_get_render_position;
>>>
>>> - *stream_out = out;
>>> + *stream_out = &a2dp_out->stream;
>>> + a2dp_dev->stream_out = a2dp_out;
>>>
>>> return 0;
>>> }
>>> @@ -381,7 +392,7 @@ static int audio_close(hw_device_t *device)
>>> static int audio_open(const hw_module_t *module, const char *name,
>>> hw_device_t **device)
>>> {
>>> - struct audio_hw_device *audio;
>>> + struct a2dp_audio_dev *a2dp_dev;
>>>
>>> DBG("");
>>>
>>> @@ -391,30 +402,30 @@ static int audio_open(const hw_module_t *module, const char *name,
>>> return -EINVAL;
>>> }
>>>
>>> - audio = calloc(1, sizeof(struct audio_hw_device));
>>> - if (!audio)
>>> + a2dp_dev = calloc(1, sizeof(struct a2dp_audio_dev));
>>> + if (!a2dp_dev)
>>> return -ENOMEM;
>>>
>>> - audio->common.version = AUDIO_DEVICE_API_VERSION_CURRENT;
>>> - audio->common.module = (struct hw_module_t *) module;
>>> - audio->common.close = audio_close;
>>> -
>>> - audio->init_check = audio_init_check;
>>> - audio->set_voice_volume = audio_set_voice_volume;
>>> - audio->set_master_volume = audio_set_master_volume;
>>> - audio->set_mode = audio_set_mode;
>>> - audio->set_mic_mute = audio_set_mic_mute;
>>> - audio->get_mic_mute = audio_get_mic_mute;
>>> - audio->set_parameters = audio_set_parameters;
>>> - audio->get_parameters = audio_get_parameters;
>>> - audio->get_input_buffer_size = audio_get_input_buffer_size;
>>> - audio->open_output_stream = audio_open_output_stream;
>>> - audio->close_output_stream = audio_close_output_stream;
>>> - audio->open_input_stream = audio_open_input_stream;
>>> - audio->close_input_stream = audio_close_input_stream;
>>> - audio->dump = audio_dump;
>>> -
>>> - *device = &audio->common;
>>> + a2dp_dev->dev.common.version = AUDIO_DEVICE_API_VERSION_CURRENT;
>>> + a2dp_dev->dev.common.module = (struct hw_module_t *) module;
>>> + a2dp_dev->dev.common.close = audio_close;
>>> +
>>> + a2dp_dev->dev.init_check = audio_init_check;
>>> + a2dp_dev->dev.set_voice_volume = audio_set_voice_volume;
>>> + a2dp_dev->dev.set_master_volume = audio_set_master_volume;
>>> + a2dp_dev->dev.set_mode = audio_set_mode;
>>> + a2dp_dev->dev.set_mic_mute = audio_set_mic_mute;
>>> + a2dp_dev->dev.get_mic_mute = audio_get_mic_mute;
>>> + a2dp_dev->dev.set_parameters = audio_set_parameters;
>>> + a2dp_dev->dev.get_parameters = audio_get_parameters;
>>> + a2dp_dev->dev.get_input_buffer_size = audio_get_input_buffer_size;
>>> + a2dp_dev->dev.open_output_stream = audio_open_output_stream;
>>> + a2dp_dev->dev.close_output_stream = audio_close_output_stream;
>>> + a2dp_dev->dev.open_input_stream = audio_open_input_stream;
>>> + a2dp_dev->dev.close_input_stream = audio_close_input_stream;
>>> + a2dp_dev->dev.dump = audio_dump;
>>> +
>>> + *device = &a2dp_dev->dev.common;
>>>
>>> return 0;
>>> }
>>> --
>>> 1.8.4
>>
>> How is this supposed to be accessed, are you planning on adding a
>> global variable?
>>
>>
> Have a look on audio_open_output_stream(). This is actually how they did so
> far and it is fine for me.
> I want to avoid globals here.
I can see that you allocate a different struct for a2dp_dev and
a2dp_out, but then you return a member of the struct back to HAL but
after that you wont access these structs anymore and the HAL only uses
the members that you return so you probably gonna need to lookup for
these structs to access any auxiliary data that we might want to add,
also these structs never seems to be freed which I suppose can
generate memory leaks.
--
Luiz Augusto von Dentz
Hi Luiz,
On 30 December 2013 12:31, Luiz Augusto von Dentz <[email protected]> wrote:
> Hi Lukasz,
>
> On Mon, Dec 30, 2013 at 12:17 PM, Lukasz Rymanowski
> <[email protected]> wrote:
>> This patch add thread which is reponsible for listen on audio HAL
>> socket, register a2dp endpoint(s) and maintain socket.
>> When bluetooth daemon goes down, HAL audio plugin starts to listen on Audio HAL
>> socket again.
>>
>> ---
>> android/Makefile.am | 2 +
>> android/hal-audio.c | 145 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>> android/hal-audio.h | 18 +++++++
>> 3 files changed, 165 insertions(+)
>> create mode 100644 android/hal-audio.h
>>
>> diff --git a/android/Makefile.am b/android/Makefile.am
>> index eaf39bd..bd90c13 100644
>> --- a/android/Makefile.am
>> +++ b/android/Makefile.am
>> @@ -112,6 +112,8 @@ android_libaudio_internal_la_SOURCES = android/hal-audio.c \
>>
>> android_libaudio_internal_la_CFLAGS = -I$(srcdir)/android
>>
>> +android_libaudio_internal_la_LDFLAGS = -pthread
>> +
>> endif
>>
>> EXTRA_DIST += android/Android.mk android/hal-ipc-api.txt android/README \
>> diff --git a/android/hal-audio.c b/android/hal-audio.c
>> index 011a699..0e3bc70 100644
>> --- a/android/hal-audio.c
>> +++ b/android/hal-audio.c
>> @@ -16,18 +16,30 @@
>> */
>>
>> #include <errno.h>
>> +#include <pthread.h>
>> +#include <poll.h>
>> #include <stdio.h>
>> #include <stdlib.h>
>> #include <string.h>
>> +#include <sys/socket.h>
>> +#include <sys/un.h>
>> +#include <unistd.h>
>>
>> #include <hardware/audio.h>
>> #include <hardware/hardware.h>
>>
>> +#include "hal-audio.h"
>> #include "hal-log.h"
>>
>> struct a2dp_audio_dev {
>> struct audio_hw_device dev;
>> struct a2dp_stream_out *stream_out;
>> +
>> + pthread_t bt_watcher;
>> + pthread_mutex_t hal_sk_mutex;
>> + pthread_cond_t bt_watcher_cond;
>> +
>> + int hal_sk;
>> };
>>
>> struct a2dp_stream_out {
>> @@ -384,15 +396,135 @@ static int audio_dump(const audio_hw_device_t *device, int fd)
>>
>> static int audio_close(hw_device_t *device)
>> {
>> + struct audio_hw_device *dev = (struct audio_hw_device *)device;
>> + struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *)dev;
>> +
>> DBG("");
>> +
>> + if (a2dp_dev) {
>> + /* TODO: We could try to unregister Endpoint here */
>> + shutdown(a2dp_dev->hal_sk, 2);
>> +
>> + pthread_mutex_lock(&a2dp_dev->hal_sk_mutex);
>> + pthread_cond_signal(&a2dp_dev->bt_watcher_cond);
>> + pthread_mutex_unlock(&a2dp_dev->hal_sk_mutex);
>> +
>> + (void) pthread_join(a2dp_dev->bt_watcher, NULL);
>> + free(a2dp_dev);
>> + }
>> +
>> free(device);
>> return 0;
>> }
>>
>> +static int wait_for_client(void)
>> +{
>> + struct sockaddr_un addr;
>> + int err;
>> + int sk;
>> + int new_sk;
>> +
>> + DBG("");
>> +
>> + sk = socket(PF_LOCAL, SOCK_SEQPACKET, 0);
>> + if (sk < 0) {
>> + err = errno;
>> + error("Failed to create socket: %d (%s)", err, strerror(err));
>> + return -1;
>> + }
>> +
>> + memset(&addr, 0, sizeof(addr));
>> + addr.sun_family = AF_UNIX;
>> +
>> + memcpy(addr.sun_path, BLUEZ_AUDIO_HAL_SK_PATH,
>> + sizeof(BLUEZ_AUDIO_HAL_SK_PATH));
>> +
>> + if (bind(sk, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
>> + err = errno;
>> + error("Failed to bind socket: %d (%s)", err, strerror(err));
>> + goto error;
>> + }
>> +
>> + if (listen(sk, 1) < 0) {
>> + err = errno;
>> + error("Failed to bind socket: %d (%s)", err, strerror(err));
>> + goto error;
>> + }
>> +
>> + new_sk = accept(sk, NULL, NULL);
>> + if (new_sk < 0) {
>> + err = errno;
>> + error("Failed to accept socket: %d (%s)", err, strerror(err));
>> + goto error;
>> + }
>> +
>> + close(sk);
>> + return new_sk;
>> +
>> +error:
>> + close(sk);
>> + return -1;
>> +}
>> +
>> +static void *bluetoothd_watcher(void *data)
>> +{
>> + int err;
>> + struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *)data;
>> + struct pollfd pfd;
>> + struct timeval now;
>> + struct timespec timeout;
>> +
>> + DBG("");
>> +
>> + while (1) {
>> + a2dp_dev->hal_sk = wait_for_client();
>> + if (a2dp_dev->hal_sk < 0) {
>> + error("Failed to create listening socket");
>> + continue;
>> + }
>> +
>> + DBG("Audio IPC: Connected");
>> +
>> + /* TODO: Register ENDPOINT here */
>> +
>> + memset(&pfd, 0, sizeof(pfd));
>> + pfd.fd = a2dp_dev->hal_sk;
>> + pfd.events = POLLHUP | POLLERR | POLLNVAL;
>> +
>> + pthread_mutex_lock(&a2dp_dev->hal_sk_mutex);
>> +
>> + /* Check if socket is still alive */
>> + err = poll(&pfd, 1, -1);
>> + if (pfd.revents & (POLLHUP | POLLERR | POLLNVAL)) {
>> + info("Audio HAL: Socket closed");
>> + a2dp_dev->hal_sk = -1;
>> + }
>> +
>> + /* Maybe audio system is closing and audio_close() has been called? */
>> + gettimeofday(&now, NULL);
>> + timeout.tv_sec = now.tv_sec;
>> + timeout.tv_nsec = now.tv_usec * 1000;
>> + timeout.tv_sec += 1;
>> +
>> + err = pthread_cond_timedwait(&a2dp_dev->bt_watcher_cond,
>> + &a2dp_dev->hal_sk_mutex, &timeout);
>> +
>> + pthread_mutex_unlock(&a2dp_dev->hal_sk_mutex);
>> + if (err != ETIMEDOUT)
>> + /* Seems that device has been closed */
>> + break;
>> + }
>> +
>> + info("Closing bluetooth_watcher thread");
>> + pthread_exit(NULL);
>> + return NULL;
>> +}
>> +
>> static int audio_open(const hw_module_t *module, const char *name,
>> hw_device_t **device)
>> {
>> struct a2dp_audio_dev *a2dp_dev;
>> + int err;
>>
>> DBG("");
>>
>> @@ -427,6 +559,19 @@ static int audio_open(const hw_module_t *module, const char *name,
>>
>> *device = &a2dp_dev->dev.common;
>>
>> + a2dp_dev->hal_sk = -1;
>> +
>> + pthread_mutex_init(&a2dp_dev->hal_sk_mutex, NULL);
>> + pthread_cond_init(&a2dp_dev->bt_watcher_cond, NULL);
>> +
>> + err = pthread_create(&a2dp_dev->bt_watcher, NULL, bluetoothd_watcher, a2dp_dev);
>> + if (err < 0) {
>> + a2dp_dev->bt_watcher = 0;
>> + error("Failed to start bluetoothd watcher thread: %d (%s)", -err,
>> + strerror(-err));
>> + return (-err);
>> + }
>> +
>> return 0;
>> }
>>
>> diff --git a/android/hal-audio.h b/android/hal-audio.h
>> new file mode 100644
>> index 0000000..93e49f6
>> --- /dev/null
>> +++ b/android/hal-audio.h
>> @@ -0,0 +1,18 @@
>> +/*
>> + *
>> + * Copyright (C) 2013 Intel Corporation
>> + *
>> + * Licensed under the Apache License, Version 2.0 (the "License");
>> + * you may not use this file except in compliance with the License.
>> + * You may obtain a copy of the License at
>> + *
>> + * http://www.apache.org/licenses/LICENSE-2.0
>> + *
>> + * Unless required by applicable law or agreed to in writing, software
>> + * distributed under the License is distributed on an "AS IS" BASIS,
>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>> + * See the License for the specific language governing permissions and
>> + * limitations under the License.
>> + *
>> + */
>> +static const char BLUEZ_AUDIO_HAL_SK_PATH[] = "\0bluez_audio_socket";
>> --
>> 1.8.4
>
> Ive started working on the daemon side and should be able to post some
> patches today, Im not really sure we are going to need a thread for
> command handling since all the commands will be sent by the plugin
> side so they can be synchronous, the only problem would be if the
> plugin cannot really block even for a short period.
>
This Thread is only going to be used for accept, endpoint register and
then listening for socket close (so we can start listen for connection
from new bluetoothd).
Commands and response will be handled in main thread (ex.
audio_open(), out_write()) and we can block there for a while - no
problem with that
>
> --
> Luiz Augusto von Dentz
\Łukasz
Hi Luiz,
On 30 December 2013 12:27, Luiz Augusto von Dentz <[email protected]> wrote:
> Hi Lukasz,
>
> On Mon, Dec 30, 2013 at 12:17 PM, Lukasz Rymanowski
> <[email protected]> wrote:
>> This patch add wrapping struct for audio_hw_dev and audio_stream_out.
>> We will need this to keep some more addition info related to a2dp stream
>> and also hal IPC later on
>>
>> ---
>> android/Makefile.am | 1 +
>> android/hal-audio.c | 97 +++++++++++++++++++++++++++++------------------------
>> 2 files changed, 55 insertions(+), 43 deletions(-)
>>
>> diff --git a/android/Makefile.am b/android/Makefile.am
>> index dec81ce..eaf39bd 100644
>> --- a/android/Makefile.am
>> +++ b/android/Makefile.am
>> @@ -104,6 +104,7 @@ android_android_tester_LDFLAGS = -pthread
>> noinst_LTLIBRARIES += android/libaudio-internal.la
>>
>> android_libaudio_internal_la_SOURCES = android/hal-audio.c \
>> + android/hal-audio.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 7f4a3f2..011a699 100644
>> --- a/android/hal-audio.c
>> +++ b/android/hal-audio.c
>> @@ -25,6 +25,15 @@
>>
>> #include "hal-log.h"
>>
>> +struct a2dp_audio_dev {
>> + struct audio_hw_device dev;
>> + struct a2dp_stream_out *stream_out;
>> +};
>> +
>> +struct a2dp_stream_out {
>> + struct audio_stream_out stream;
>> +};
>> +
>> static ssize_t out_write(struct audio_stream_out *stream, const void *buffer,
>> size_t bytes)
>> {
>> @@ -230,32 +239,34 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
>> struct audio_stream_out **stream_out)
>>
>> {
>> - struct audio_stream_out *out;
>> + struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *)dev;
>> + struct a2dp_stream_out *a2dp_out;
>>
>> - out = calloc(1, sizeof(struct audio_stream_out));
>> - if (!out)
>> + a2dp_out = calloc(1, sizeof(struct a2dp_stream_out));
>> + if (!a2dp_out)
>> return -ENOMEM;
>>
>> DBG("");
>>
>> - out->common.get_sample_rate = out_get_sample_rate;
>> - out->common.set_sample_rate = out_set_sample_rate;
>> - out->common.get_buffer_size = out_get_buffer_size;
>> - out->common.get_channels = out_get_channels;
>> - out->common.get_format = out_get_format;
>> - out->common.set_format = out_set_format;
>> - out->common.standby = out_standby;
>> - out->common.dump = out_dump;
>> - out->common.set_parameters = out_set_parameters;
>> - out->common.get_parameters = out_get_parameters;
>> - out->common.add_audio_effect = out_add_audio_effect;
>> - out->common.remove_audio_effect = out_remove_audio_effect;
>> - out->get_latency = out_get_latency;
>> - out->set_volume = out_set_volume;
>> - out->write = out_write;
>> - out->get_render_position = out_get_render_position;
>> + a2dp_out->stream.common.get_sample_rate = out_get_sample_rate;
>> + a2dp_out->stream.common.set_sample_rate = out_set_sample_rate;
>> + a2dp_out->stream.common.get_buffer_size = out_get_buffer_size;
>> + a2dp_out->stream.common.get_channels = out_get_channels;
>> + a2dp_out->stream.common.get_format = out_get_format;
>> + a2dp_out->stream.common.set_format = out_set_format;
>> + a2dp_out->stream.common.standby = out_standby;
>> + a2dp_out->stream.common.dump = out_dump;
>> + a2dp_out->stream.common.set_parameters = out_set_parameters;
>> + a2dp_out->stream.common.get_parameters = out_get_parameters;
>> + a2dp_out->stream.common.add_audio_effect = out_add_audio_effect;
>> + a2dp_out->stream.common.remove_audio_effect = out_remove_audio_effect;
>> + a2dp_out->stream.get_latency = out_get_latency;
>> + a2dp_out->stream.set_volume = out_set_volume;
>> + a2dp_out->stream.write = out_write;
>> + a2dp_out->stream.get_render_position = out_get_render_position;
>>
>> - *stream_out = out;
>> + *stream_out = &a2dp_out->stream;
>> + a2dp_dev->stream_out = a2dp_out;
>>
>> return 0;
>> }
>> @@ -381,7 +392,7 @@ static int audio_close(hw_device_t *device)
>> static int audio_open(const hw_module_t *module, const char *name,
>> hw_device_t **device)
>> {
>> - struct audio_hw_device *audio;
>> + struct a2dp_audio_dev *a2dp_dev;
>>
>> DBG("");
>>
>> @@ -391,30 +402,30 @@ static int audio_open(const hw_module_t *module, const char *name,
>> return -EINVAL;
>> }
>>
>> - audio = calloc(1, sizeof(struct audio_hw_device));
>> - if (!audio)
>> + a2dp_dev = calloc(1, sizeof(struct a2dp_audio_dev));
>> + if (!a2dp_dev)
>> return -ENOMEM;
>>
>> - audio->common.version = AUDIO_DEVICE_API_VERSION_CURRENT;
>> - audio->common.module = (struct hw_module_t *) module;
>> - audio->common.close = audio_close;
>> -
>> - audio->init_check = audio_init_check;
>> - audio->set_voice_volume = audio_set_voice_volume;
>> - audio->set_master_volume = audio_set_master_volume;
>> - audio->set_mode = audio_set_mode;
>> - audio->set_mic_mute = audio_set_mic_mute;
>> - audio->get_mic_mute = audio_get_mic_mute;
>> - audio->set_parameters = audio_set_parameters;
>> - audio->get_parameters = audio_get_parameters;
>> - audio->get_input_buffer_size = audio_get_input_buffer_size;
>> - audio->open_output_stream = audio_open_output_stream;
>> - audio->close_output_stream = audio_close_output_stream;
>> - audio->open_input_stream = audio_open_input_stream;
>> - audio->close_input_stream = audio_close_input_stream;
>> - audio->dump = audio_dump;
>> -
>> - *device = &audio->common;
>> + a2dp_dev->dev.common.version = AUDIO_DEVICE_API_VERSION_CURRENT;
>> + a2dp_dev->dev.common.module = (struct hw_module_t *) module;
>> + a2dp_dev->dev.common.close = audio_close;
>> +
>> + a2dp_dev->dev.init_check = audio_init_check;
>> + a2dp_dev->dev.set_voice_volume = audio_set_voice_volume;
>> + a2dp_dev->dev.set_master_volume = audio_set_master_volume;
>> + a2dp_dev->dev.set_mode = audio_set_mode;
>> + a2dp_dev->dev.set_mic_mute = audio_set_mic_mute;
>> + a2dp_dev->dev.get_mic_mute = audio_get_mic_mute;
>> + a2dp_dev->dev.set_parameters = audio_set_parameters;
>> + a2dp_dev->dev.get_parameters = audio_get_parameters;
>> + a2dp_dev->dev.get_input_buffer_size = audio_get_input_buffer_size;
>> + a2dp_dev->dev.open_output_stream = audio_open_output_stream;
>> + a2dp_dev->dev.close_output_stream = audio_close_output_stream;
>> + a2dp_dev->dev.open_input_stream = audio_open_input_stream;
>> + a2dp_dev->dev.close_input_stream = audio_close_input_stream;
>> + a2dp_dev->dev.dump = audio_dump;
>> +
>> + *device = &a2dp_dev->dev.common;
>>
>> return 0;
>> }
>> --
>> 1.8.4
>
> How is this supposed to be accessed, are you planning on adding a
> global variable?
>
>
Have a look on audio_open_output_stream(). This is actually how they did so
far and it is fine for me.
I want to avoid globals here.
> --
> Luiz Augusto von Dentz
\Lukasz
Hi Lukasz,
On Mon, Dec 30, 2013 at 12:17 PM, Lukasz Rymanowski
<[email protected]> wrote:
> This patch add thread which is reponsible for listen on audio HAL
> socket, register a2dp endpoint(s) and maintain socket.
> When bluetooth daemon goes down, HAL audio plugin starts to listen on Audio HAL
> socket again.
>
> ---
> android/Makefile.am | 2 +
> android/hal-audio.c | 145 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> android/hal-audio.h | 18 +++++++
> 3 files changed, 165 insertions(+)
> create mode 100644 android/hal-audio.h
>
> diff --git a/android/Makefile.am b/android/Makefile.am
> index eaf39bd..bd90c13 100644
> --- a/android/Makefile.am
> +++ b/android/Makefile.am
> @@ -112,6 +112,8 @@ android_libaudio_internal_la_SOURCES = android/hal-audio.c \
>
> android_libaudio_internal_la_CFLAGS = -I$(srcdir)/android
>
> +android_libaudio_internal_la_LDFLAGS = -pthread
> +
> endif
>
> EXTRA_DIST += android/Android.mk android/hal-ipc-api.txt android/README \
> diff --git a/android/hal-audio.c b/android/hal-audio.c
> index 011a699..0e3bc70 100644
> --- a/android/hal-audio.c
> +++ b/android/hal-audio.c
> @@ -16,18 +16,30 @@
> */
>
> #include <errno.h>
> +#include <pthread.h>
> +#include <poll.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> +#include <sys/socket.h>
> +#include <sys/un.h>
> +#include <unistd.h>
>
> #include <hardware/audio.h>
> #include <hardware/hardware.h>
>
> +#include "hal-audio.h"
> #include "hal-log.h"
>
> struct a2dp_audio_dev {
> struct audio_hw_device dev;
> struct a2dp_stream_out *stream_out;
> +
> + pthread_t bt_watcher;
> + pthread_mutex_t hal_sk_mutex;
> + pthread_cond_t bt_watcher_cond;
> +
> + int hal_sk;
> };
>
> struct a2dp_stream_out {
> @@ -384,15 +396,135 @@ static int audio_dump(const audio_hw_device_t *device, int fd)
>
> static int audio_close(hw_device_t *device)
> {
> + struct audio_hw_device *dev = (struct audio_hw_device *)device;
> + struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *)dev;
> +
> DBG("");
> +
> + if (a2dp_dev) {
> + /* TODO: We could try to unregister Endpoint here */
> + shutdown(a2dp_dev->hal_sk, 2);
> +
> + pthread_mutex_lock(&a2dp_dev->hal_sk_mutex);
> + pthread_cond_signal(&a2dp_dev->bt_watcher_cond);
> + pthread_mutex_unlock(&a2dp_dev->hal_sk_mutex);
> +
> + (void) pthread_join(a2dp_dev->bt_watcher, NULL);
> + free(a2dp_dev);
> + }
> +
> free(device);
> return 0;
> }
>
> +static int wait_for_client(void)
> +{
> + struct sockaddr_un addr;
> + int err;
> + int sk;
> + int new_sk;
> +
> + DBG("");
> +
> + sk = socket(PF_LOCAL, SOCK_SEQPACKET, 0);
> + if (sk < 0) {
> + err = errno;
> + error("Failed to create socket: %d (%s)", err, strerror(err));
> + return -1;
> + }
> +
> + memset(&addr, 0, sizeof(addr));
> + addr.sun_family = AF_UNIX;
> +
> + memcpy(addr.sun_path, BLUEZ_AUDIO_HAL_SK_PATH,
> + sizeof(BLUEZ_AUDIO_HAL_SK_PATH));
> +
> + if (bind(sk, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
> + err = errno;
> + error("Failed to bind socket: %d (%s)", err, strerror(err));
> + goto error;
> + }
> +
> + if (listen(sk, 1) < 0) {
> + err = errno;
> + error("Failed to bind socket: %d (%s)", err, strerror(err));
> + goto error;
> + }
> +
> + new_sk = accept(sk, NULL, NULL);
> + if (new_sk < 0) {
> + err = errno;
> + error("Failed to accept socket: %d (%s)", err, strerror(err));
> + goto error;
> + }
> +
> + close(sk);
> + return new_sk;
> +
> +error:
> + close(sk);
> + return -1;
> +}
> +
> +static void *bluetoothd_watcher(void *data)
> +{
> + int err;
> + struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *)data;
> + struct pollfd pfd;
> + struct timeval now;
> + struct timespec timeout;
> +
> + DBG("");
> +
> + while (1) {
> + a2dp_dev->hal_sk = wait_for_client();
> + if (a2dp_dev->hal_sk < 0) {
> + error("Failed to create listening socket");
> + continue;
> + }
> +
> + DBG("Audio IPC: Connected");
> +
> + /* TODO: Register ENDPOINT here */
> +
> + memset(&pfd, 0, sizeof(pfd));
> + pfd.fd = a2dp_dev->hal_sk;
> + pfd.events = POLLHUP | POLLERR | POLLNVAL;
> +
> + pthread_mutex_lock(&a2dp_dev->hal_sk_mutex);
> +
> + /* Check if socket is still alive */
> + err = poll(&pfd, 1, -1);
> + if (pfd.revents & (POLLHUP | POLLERR | POLLNVAL)) {
> + info("Audio HAL: Socket closed");
> + a2dp_dev->hal_sk = -1;
> + }
> +
> + /* Maybe audio system is closing and audio_close() has been called? */
> + gettimeofday(&now, NULL);
> + timeout.tv_sec = now.tv_sec;
> + timeout.tv_nsec = now.tv_usec * 1000;
> + timeout.tv_sec += 1;
> +
> + err = pthread_cond_timedwait(&a2dp_dev->bt_watcher_cond,
> + &a2dp_dev->hal_sk_mutex, &timeout);
> +
> + pthread_mutex_unlock(&a2dp_dev->hal_sk_mutex);
> + if (err != ETIMEDOUT)
> + /* Seems that device has been closed */
> + break;
> + }
> +
> + info("Closing bluetooth_watcher thread");
> + pthread_exit(NULL);
> + return NULL;
> +}
> +
> static int audio_open(const hw_module_t *module, const char *name,
> hw_device_t **device)
> {
> struct a2dp_audio_dev *a2dp_dev;
> + int err;
>
> DBG("");
>
> @@ -427,6 +559,19 @@ static int audio_open(const hw_module_t *module, const char *name,
>
> *device = &a2dp_dev->dev.common;
>
> + a2dp_dev->hal_sk = -1;
> +
> + pthread_mutex_init(&a2dp_dev->hal_sk_mutex, NULL);
> + pthread_cond_init(&a2dp_dev->bt_watcher_cond, NULL);
> +
> + err = pthread_create(&a2dp_dev->bt_watcher, NULL, bluetoothd_watcher, a2dp_dev);
> + if (err < 0) {
> + a2dp_dev->bt_watcher = 0;
> + error("Failed to start bluetoothd watcher thread: %d (%s)", -err,
> + strerror(-err));
> + return (-err);
> + }
> +
> return 0;
> }
>
> diff --git a/android/hal-audio.h b/android/hal-audio.h
> new file mode 100644
> index 0000000..93e49f6
> --- /dev/null
> +++ b/android/hal-audio.h
> @@ -0,0 +1,18 @@
> +/*
> + *
> + * Copyright (C) 2013 Intel Corporation
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + *
> + */
> +static const char BLUEZ_AUDIO_HAL_SK_PATH[] = "\0bluez_audio_socket";
> --
> 1.8.4
Ive started working on the daemon side and should be able to post some
patches today, Im not really sure we are going to need a thread for
command handling since all the commands will be sent by the plugin
side so they can be synchronous, the only problem would be if the
plugin cannot really block even for a short period.
--
Luiz Augusto von Dentz
Hi Lukasz,
On Mon, Dec 30, 2013 at 12:17 PM, Lukasz Rymanowski
<[email protected]> wrote:
> This patch add wrapping struct for audio_hw_dev and audio_stream_out.
> We will need this to keep some more addition info related to a2dp stream
> and also hal IPC later on
>
> ---
> android/Makefile.am | 1 +
> android/hal-audio.c | 97 +++++++++++++++++++++++++++++------------------------
> 2 files changed, 55 insertions(+), 43 deletions(-)
>
> diff --git a/android/Makefile.am b/android/Makefile.am
> index dec81ce..eaf39bd 100644
> --- a/android/Makefile.am
> +++ b/android/Makefile.am
> @@ -104,6 +104,7 @@ android_android_tester_LDFLAGS = -pthread
> noinst_LTLIBRARIES += android/libaudio-internal.la
>
> android_libaudio_internal_la_SOURCES = android/hal-audio.c \
> + android/hal-audio.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 7f4a3f2..011a699 100644
> --- a/android/hal-audio.c
> +++ b/android/hal-audio.c
> @@ -25,6 +25,15 @@
>
> #include "hal-log.h"
>
> +struct a2dp_audio_dev {
> + struct audio_hw_device dev;
> + struct a2dp_stream_out *stream_out;
> +};
> +
> +struct a2dp_stream_out {
> + struct audio_stream_out stream;
> +};
> +
> static ssize_t out_write(struct audio_stream_out *stream, const void *buffer,
> size_t bytes)
> {
> @@ -230,32 +239,34 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
> struct audio_stream_out **stream_out)
>
> {
> - struct audio_stream_out *out;
> + struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *)dev;
> + struct a2dp_stream_out *a2dp_out;
>
> - out = calloc(1, sizeof(struct audio_stream_out));
> - if (!out)
> + a2dp_out = calloc(1, sizeof(struct a2dp_stream_out));
> + if (!a2dp_out)
> return -ENOMEM;
>
> DBG("");
>
> - out->common.get_sample_rate = out_get_sample_rate;
> - out->common.set_sample_rate = out_set_sample_rate;
> - out->common.get_buffer_size = out_get_buffer_size;
> - out->common.get_channels = out_get_channels;
> - out->common.get_format = out_get_format;
> - out->common.set_format = out_set_format;
> - out->common.standby = out_standby;
> - out->common.dump = out_dump;
> - out->common.set_parameters = out_set_parameters;
> - out->common.get_parameters = out_get_parameters;
> - out->common.add_audio_effect = out_add_audio_effect;
> - out->common.remove_audio_effect = out_remove_audio_effect;
> - out->get_latency = out_get_latency;
> - out->set_volume = out_set_volume;
> - out->write = out_write;
> - out->get_render_position = out_get_render_position;
> + a2dp_out->stream.common.get_sample_rate = out_get_sample_rate;
> + a2dp_out->stream.common.set_sample_rate = out_set_sample_rate;
> + a2dp_out->stream.common.get_buffer_size = out_get_buffer_size;
> + a2dp_out->stream.common.get_channels = out_get_channels;
> + a2dp_out->stream.common.get_format = out_get_format;
> + a2dp_out->stream.common.set_format = out_set_format;
> + a2dp_out->stream.common.standby = out_standby;
> + a2dp_out->stream.common.dump = out_dump;
> + a2dp_out->stream.common.set_parameters = out_set_parameters;
> + a2dp_out->stream.common.get_parameters = out_get_parameters;
> + a2dp_out->stream.common.add_audio_effect = out_add_audio_effect;
> + a2dp_out->stream.common.remove_audio_effect = out_remove_audio_effect;
> + a2dp_out->stream.get_latency = out_get_latency;
> + a2dp_out->stream.set_volume = out_set_volume;
> + a2dp_out->stream.write = out_write;
> + a2dp_out->stream.get_render_position = out_get_render_position;
>
> - *stream_out = out;
> + *stream_out = &a2dp_out->stream;
> + a2dp_dev->stream_out = a2dp_out;
>
> return 0;
> }
> @@ -381,7 +392,7 @@ static int audio_close(hw_device_t *device)
> static int audio_open(const hw_module_t *module, const char *name,
> hw_device_t **device)
> {
> - struct audio_hw_device *audio;
> + struct a2dp_audio_dev *a2dp_dev;
>
> DBG("");
>
> @@ -391,30 +402,30 @@ static int audio_open(const hw_module_t *module, const char *name,
> return -EINVAL;
> }
>
> - audio = calloc(1, sizeof(struct audio_hw_device));
> - if (!audio)
> + a2dp_dev = calloc(1, sizeof(struct a2dp_audio_dev));
> + if (!a2dp_dev)
> return -ENOMEM;
>
> - audio->common.version = AUDIO_DEVICE_API_VERSION_CURRENT;
> - audio->common.module = (struct hw_module_t *) module;
> - audio->common.close = audio_close;
> -
> - audio->init_check = audio_init_check;
> - audio->set_voice_volume = audio_set_voice_volume;
> - audio->set_master_volume = audio_set_master_volume;
> - audio->set_mode = audio_set_mode;
> - audio->set_mic_mute = audio_set_mic_mute;
> - audio->get_mic_mute = audio_get_mic_mute;
> - audio->set_parameters = audio_set_parameters;
> - audio->get_parameters = audio_get_parameters;
> - audio->get_input_buffer_size = audio_get_input_buffer_size;
> - audio->open_output_stream = audio_open_output_stream;
> - audio->close_output_stream = audio_close_output_stream;
> - audio->open_input_stream = audio_open_input_stream;
> - audio->close_input_stream = audio_close_input_stream;
> - audio->dump = audio_dump;
> -
> - *device = &audio->common;
> + a2dp_dev->dev.common.version = AUDIO_DEVICE_API_VERSION_CURRENT;
> + a2dp_dev->dev.common.module = (struct hw_module_t *) module;
> + a2dp_dev->dev.common.close = audio_close;
> +
> + a2dp_dev->dev.init_check = audio_init_check;
> + a2dp_dev->dev.set_voice_volume = audio_set_voice_volume;
> + a2dp_dev->dev.set_master_volume = audio_set_master_volume;
> + a2dp_dev->dev.set_mode = audio_set_mode;
> + a2dp_dev->dev.set_mic_mute = audio_set_mic_mute;
> + a2dp_dev->dev.get_mic_mute = audio_get_mic_mute;
> + a2dp_dev->dev.set_parameters = audio_set_parameters;
> + a2dp_dev->dev.get_parameters = audio_get_parameters;
> + a2dp_dev->dev.get_input_buffer_size = audio_get_input_buffer_size;
> + a2dp_dev->dev.open_output_stream = audio_open_output_stream;
> + a2dp_dev->dev.close_output_stream = audio_close_output_stream;
> + a2dp_dev->dev.open_input_stream = audio_open_input_stream;
> + a2dp_dev->dev.close_input_stream = audio_close_input_stream;
> + a2dp_dev->dev.dump = audio_dump;
> +
> + *device = &a2dp_dev->dev.common;
>
> return 0;
> }
> --
> 1.8.4
How is this supposed to be accessed, are you planning on adding a
global variable?
--
Luiz Augusto von Dentz
Hi Lukasz,
On Mon, Dec 30, 2013 at 12:17 PM, Lukasz Rymanowski
<[email protected]> wrote:
> ---
> android/audio-ipc-api.txt | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/android/audio-ipc-api.txt b/android/audio-ipc-api.txt
> index 5140d77..c2d23d9 100644
> --- a/android/audio-ipc-api.txt
> +++ b/android/audio-ipc-api.txt
> @@ -27,11 +27,11 @@ HAL socket apply here as well, the abstract socket name is
> call dev->open_output_stream() --> command 0x03
> return dev->open_output_stream() <-- response 0x03
>
> - call stream_in->read() --> command 0x05
> - return stream_in->read() <-- response 0x05
> + call stream_out->write() --> command 0x05
> + return stream_out->write() <-- response 0x05
>
> - call stream_in->common.standby() --> command 0x06
> - return stream_in->common.standby() <-- response 0x06
> + call stream_out->common.standby() --> command 0x06
> + return stream_out->common.standby() <-- response 0x06
>
> call dev->close_output_stream() --> command 0x04
> return dev->close_output_stream() <-- response 0x04
> @@ -39,6 +39,8 @@ HAL socket apply here as well, the abstract socket name is
> call dev->close() --> command 0x02
> return dev->close() <-- response 0x02
>
> +Note: Similar applies to stream in scenario.
> +
> Identifier: "audio" (BT_AUDIO_ID)
>
> Opcode 0x00 - Error response
> --
> 1.8.4
I would leave it as just stream, so it should work regardless of the direction.
--
Luiz Augusto von Dentz
This patch add thread which is reponsible for listen on audio HAL
socket, register a2dp endpoint(s) and maintain socket.
When bluetooth daemon goes down, HAL audio plugin starts to listen on Audio HAL
socket again.
---
android/Makefile.am | 2 +
android/hal-audio.c | 145 ++++++++++++++++++++++++++++++++++++++++++++++++++++
android/hal-audio.h | 18 +++++++
3 files changed, 165 insertions(+)
create mode 100644 android/hal-audio.h
diff --git a/android/Makefile.am b/android/Makefile.am
index eaf39bd..bd90c13 100644
--- a/android/Makefile.am
+++ b/android/Makefile.am
@@ -112,6 +112,8 @@ android_libaudio_internal_la_SOURCES = android/hal-audio.c \
android_libaudio_internal_la_CFLAGS = -I$(srcdir)/android
+android_libaudio_internal_la_LDFLAGS = -pthread
+
endif
EXTRA_DIST += android/Android.mk android/hal-ipc-api.txt android/README \
diff --git a/android/hal-audio.c b/android/hal-audio.c
index 011a699..0e3bc70 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -16,18 +16,30 @@
*/
#include <errno.h>
+#include <pthread.h>
+#include <poll.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <unistd.h>
#include <hardware/audio.h>
#include <hardware/hardware.h>
+#include "hal-audio.h"
#include "hal-log.h"
struct a2dp_audio_dev {
struct audio_hw_device dev;
struct a2dp_stream_out *stream_out;
+
+ pthread_t bt_watcher;
+ pthread_mutex_t hal_sk_mutex;
+ pthread_cond_t bt_watcher_cond;
+
+ int hal_sk;
};
struct a2dp_stream_out {
@@ -384,15 +396,135 @@ static int audio_dump(const audio_hw_device_t *device, int fd)
static int audio_close(hw_device_t *device)
{
+ struct audio_hw_device *dev = (struct audio_hw_device *)device;
+ struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *)dev;
+
DBG("");
+
+ if (a2dp_dev) {
+ /* TODO: We could try to unregister Endpoint here */
+ shutdown(a2dp_dev->hal_sk, 2);
+
+ pthread_mutex_lock(&a2dp_dev->hal_sk_mutex);
+ pthread_cond_signal(&a2dp_dev->bt_watcher_cond);
+ pthread_mutex_unlock(&a2dp_dev->hal_sk_mutex);
+
+ (void) pthread_join(a2dp_dev->bt_watcher, NULL);
+ free(a2dp_dev);
+ }
+
free(device);
return 0;
}
+static int wait_for_client(void)
+{
+ struct sockaddr_un addr;
+ int err;
+ int sk;
+ int new_sk;
+
+ DBG("");
+
+ sk = socket(PF_LOCAL, SOCK_SEQPACKET, 0);
+ if (sk < 0) {
+ err = errno;
+ error("Failed to create socket: %d (%s)", err, strerror(err));
+ return -1;
+ }
+
+ memset(&addr, 0, sizeof(addr));
+ addr.sun_family = AF_UNIX;
+
+ memcpy(addr.sun_path, BLUEZ_AUDIO_HAL_SK_PATH,
+ sizeof(BLUEZ_AUDIO_HAL_SK_PATH));
+
+ if (bind(sk, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
+ err = errno;
+ error("Failed to bind socket: %d (%s)", err, strerror(err));
+ goto error;
+ }
+
+ if (listen(sk, 1) < 0) {
+ err = errno;
+ error("Failed to bind socket: %d (%s)", err, strerror(err));
+ goto error;
+ }
+
+ new_sk = accept(sk, NULL, NULL);
+ if (new_sk < 0) {
+ err = errno;
+ error("Failed to accept socket: %d (%s)", err, strerror(err));
+ goto error;
+ }
+
+ close(sk);
+ return new_sk;
+
+error:
+ close(sk);
+ return -1;
+}
+
+static void *bluetoothd_watcher(void *data)
+{
+ int err;
+ struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *)data;
+ struct pollfd pfd;
+ struct timeval now;
+ struct timespec timeout;
+
+ DBG("");
+
+ while (1) {
+ a2dp_dev->hal_sk = wait_for_client();
+ if (a2dp_dev->hal_sk < 0) {
+ error("Failed to create listening socket");
+ continue;
+ }
+
+ DBG("Audio IPC: Connected");
+
+ /* TODO: Register ENDPOINT here */
+
+ memset(&pfd, 0, sizeof(pfd));
+ pfd.fd = a2dp_dev->hal_sk;
+ pfd.events = POLLHUP | POLLERR | POLLNVAL;
+
+ pthread_mutex_lock(&a2dp_dev->hal_sk_mutex);
+
+ /* Check if socket is still alive */
+ err = poll(&pfd, 1, -1);
+ if (pfd.revents & (POLLHUP | POLLERR | POLLNVAL)) {
+ info("Audio HAL: Socket closed");
+ a2dp_dev->hal_sk = -1;
+ }
+
+ /* Maybe audio system is closing and audio_close() has been called? */
+ gettimeofday(&now, NULL);
+ timeout.tv_sec = now.tv_sec;
+ timeout.tv_nsec = now.tv_usec * 1000;
+ timeout.tv_sec += 1;
+
+ err = pthread_cond_timedwait(&a2dp_dev->bt_watcher_cond,
+ &a2dp_dev->hal_sk_mutex, &timeout);
+
+ pthread_mutex_unlock(&a2dp_dev->hal_sk_mutex);
+ if (err != ETIMEDOUT)
+ /* Seems that device has been closed */
+ break;
+ }
+
+ info("Closing bluetooth_watcher thread");
+ pthread_exit(NULL);
+ return NULL;
+}
+
static int audio_open(const hw_module_t *module, const char *name,
hw_device_t **device)
{
struct a2dp_audio_dev *a2dp_dev;
+ int err;
DBG("");
@@ -427,6 +559,19 @@ static int audio_open(const hw_module_t *module, const char *name,
*device = &a2dp_dev->dev.common;
+ a2dp_dev->hal_sk = -1;
+
+ pthread_mutex_init(&a2dp_dev->hal_sk_mutex, NULL);
+ pthread_cond_init(&a2dp_dev->bt_watcher_cond, NULL);
+
+ err = pthread_create(&a2dp_dev->bt_watcher, NULL, bluetoothd_watcher, a2dp_dev);
+ if (err < 0) {
+ a2dp_dev->bt_watcher = 0;
+ error("Failed to start bluetoothd watcher thread: %d (%s)", -err,
+ strerror(-err));
+ return (-err);
+ }
+
return 0;
}
diff --git a/android/hal-audio.h b/android/hal-audio.h
new file mode 100644
index 0000000..93e49f6
--- /dev/null
+++ b/android/hal-audio.h
@@ -0,0 +1,18 @@
+/*
+ *
+ * Copyright (C) 2013 Intel Corporation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+static const char BLUEZ_AUDIO_HAL_SK_PATH[] = "\0bluez_audio_socket";
--
1.8.4
This patch add wrapping struct for audio_hw_dev and audio_stream_out.
We will need this to keep some more addition info related to a2dp stream
and also hal IPC later on
---
android/Makefile.am | 1 +
android/hal-audio.c | 97 +++++++++++++++++++++++++++++------------------------
2 files changed, 55 insertions(+), 43 deletions(-)
diff --git a/android/Makefile.am b/android/Makefile.am
index dec81ce..eaf39bd 100644
--- a/android/Makefile.am
+++ b/android/Makefile.am
@@ -104,6 +104,7 @@ android_android_tester_LDFLAGS = -pthread
noinst_LTLIBRARIES += android/libaudio-internal.la
android_libaudio_internal_la_SOURCES = android/hal-audio.c \
+ android/hal-audio.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 7f4a3f2..011a699 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -25,6 +25,15 @@
#include "hal-log.h"
+struct a2dp_audio_dev {
+ struct audio_hw_device dev;
+ struct a2dp_stream_out *stream_out;
+};
+
+struct a2dp_stream_out {
+ struct audio_stream_out stream;
+};
+
static ssize_t out_write(struct audio_stream_out *stream, const void *buffer,
size_t bytes)
{
@@ -230,32 +239,34 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
struct audio_stream_out **stream_out)
{
- struct audio_stream_out *out;
+ struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *)dev;
+ struct a2dp_stream_out *a2dp_out;
- out = calloc(1, sizeof(struct audio_stream_out));
- if (!out)
+ a2dp_out = calloc(1, sizeof(struct a2dp_stream_out));
+ if (!a2dp_out)
return -ENOMEM;
DBG("");
- out->common.get_sample_rate = out_get_sample_rate;
- out->common.set_sample_rate = out_set_sample_rate;
- out->common.get_buffer_size = out_get_buffer_size;
- out->common.get_channels = out_get_channels;
- out->common.get_format = out_get_format;
- out->common.set_format = out_set_format;
- out->common.standby = out_standby;
- out->common.dump = out_dump;
- out->common.set_parameters = out_set_parameters;
- out->common.get_parameters = out_get_parameters;
- out->common.add_audio_effect = out_add_audio_effect;
- out->common.remove_audio_effect = out_remove_audio_effect;
- out->get_latency = out_get_latency;
- out->set_volume = out_set_volume;
- out->write = out_write;
- out->get_render_position = out_get_render_position;
+ a2dp_out->stream.common.get_sample_rate = out_get_sample_rate;
+ a2dp_out->stream.common.set_sample_rate = out_set_sample_rate;
+ a2dp_out->stream.common.get_buffer_size = out_get_buffer_size;
+ a2dp_out->stream.common.get_channels = out_get_channels;
+ a2dp_out->stream.common.get_format = out_get_format;
+ a2dp_out->stream.common.set_format = out_set_format;
+ a2dp_out->stream.common.standby = out_standby;
+ a2dp_out->stream.common.dump = out_dump;
+ a2dp_out->stream.common.set_parameters = out_set_parameters;
+ a2dp_out->stream.common.get_parameters = out_get_parameters;
+ a2dp_out->stream.common.add_audio_effect = out_add_audio_effect;
+ a2dp_out->stream.common.remove_audio_effect = out_remove_audio_effect;
+ a2dp_out->stream.get_latency = out_get_latency;
+ a2dp_out->stream.set_volume = out_set_volume;
+ a2dp_out->stream.write = out_write;
+ a2dp_out->stream.get_render_position = out_get_render_position;
- *stream_out = out;
+ *stream_out = &a2dp_out->stream;
+ a2dp_dev->stream_out = a2dp_out;
return 0;
}
@@ -381,7 +392,7 @@ static int audio_close(hw_device_t *device)
static int audio_open(const hw_module_t *module, const char *name,
hw_device_t **device)
{
- struct audio_hw_device *audio;
+ struct a2dp_audio_dev *a2dp_dev;
DBG("");
@@ -391,30 +402,30 @@ static int audio_open(const hw_module_t *module, const char *name,
return -EINVAL;
}
- audio = calloc(1, sizeof(struct audio_hw_device));
- if (!audio)
+ a2dp_dev = calloc(1, sizeof(struct a2dp_audio_dev));
+ if (!a2dp_dev)
return -ENOMEM;
- audio->common.version = AUDIO_DEVICE_API_VERSION_CURRENT;
- audio->common.module = (struct hw_module_t *) module;
- audio->common.close = audio_close;
-
- audio->init_check = audio_init_check;
- audio->set_voice_volume = audio_set_voice_volume;
- audio->set_master_volume = audio_set_master_volume;
- audio->set_mode = audio_set_mode;
- audio->set_mic_mute = audio_set_mic_mute;
- audio->get_mic_mute = audio_get_mic_mute;
- audio->set_parameters = audio_set_parameters;
- audio->get_parameters = audio_get_parameters;
- audio->get_input_buffer_size = audio_get_input_buffer_size;
- audio->open_output_stream = audio_open_output_stream;
- audio->close_output_stream = audio_close_output_stream;
- audio->open_input_stream = audio_open_input_stream;
- audio->close_input_stream = audio_close_input_stream;
- audio->dump = audio_dump;
-
- *device = &audio->common;
+ a2dp_dev->dev.common.version = AUDIO_DEVICE_API_VERSION_CURRENT;
+ a2dp_dev->dev.common.module = (struct hw_module_t *) module;
+ a2dp_dev->dev.common.close = audio_close;
+
+ a2dp_dev->dev.init_check = audio_init_check;
+ a2dp_dev->dev.set_voice_volume = audio_set_voice_volume;
+ a2dp_dev->dev.set_master_volume = audio_set_master_volume;
+ a2dp_dev->dev.set_mode = audio_set_mode;
+ a2dp_dev->dev.set_mic_mute = audio_set_mic_mute;
+ a2dp_dev->dev.get_mic_mute = audio_get_mic_mute;
+ a2dp_dev->dev.set_parameters = audio_set_parameters;
+ a2dp_dev->dev.get_parameters = audio_get_parameters;
+ a2dp_dev->dev.get_input_buffer_size = audio_get_input_buffer_size;
+ a2dp_dev->dev.open_output_stream = audio_open_output_stream;
+ a2dp_dev->dev.close_output_stream = audio_close_output_stream;
+ a2dp_dev->dev.open_input_stream = audio_open_input_stream;
+ a2dp_dev->dev.close_input_stream = audio_close_input_stream;
+ a2dp_dev->dev.dump = audio_dump;
+
+ *device = &a2dp_dev->dev.common;
return 0;
}
--
1.8.4
Use CFLAGS instread of CPPFLAGS
---
android/Makefile.am | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/android/Makefile.am b/android/Makefile.am
index a5533de..dec81ce 100644
--- a/android/Makefile.am
+++ b/android/Makefile.am
@@ -109,7 +109,7 @@ android_libaudio_internal_la_SOURCES = android/hal-audio.c \
android/hardware/hardware.h \
android/system/audio.h
-android_libaudio_internal_la_CPPFLAGS = -I$(srcdir)/android
+android_libaudio_internal_la_CFLAGS = -I$(srcdir)/android
endif
--
1.8.4
---
android/audio-ipc-api.txt | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/android/audio-ipc-api.txt b/android/audio-ipc-api.txt
index 5140d77..c2d23d9 100644
--- a/android/audio-ipc-api.txt
+++ b/android/audio-ipc-api.txt
@@ -27,11 +27,11 @@ HAL socket apply here as well, the abstract socket name is
call dev->open_output_stream() --> command 0x03
return dev->open_output_stream() <-- response 0x03
- call stream_in->read() --> command 0x05
- return stream_in->read() <-- response 0x05
+ call stream_out->write() --> command 0x05
+ return stream_out->write() <-- response 0x05
- call stream_in->common.standby() --> command 0x06
- return stream_in->common.standby() <-- response 0x06
+ call stream_out->common.standby() --> command 0x06
+ return stream_out->common.standby() <-- response 0x06
call dev->close_output_stream() --> command 0x04
return dev->close_output_stream() <-- response 0x04
@@ -39,6 +39,8 @@ HAL socket apply here as well, the abstract socket name is
call dev->close() --> command 0x02
return dev->close() <-- response 0x02
+Note: Similar applies to stream in scenario.
+
Identifier: "audio" (BT_AUDIO_ID)
Opcode 0x00 - Error response
--
1.8.4
---
android/ipc.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/android/ipc.c b/android/ipc.c
index 6f940cd..9e8ccc3 100644
--- a/android/ipc.c
+++ b/android/ipc.c
@@ -32,7 +32,6 @@
#include <signal.h>
#include <stdbool.h>
#include <sys/socket.h>
-#include <sys/socket.h>
#include <sys/un.h>
#include <unistd.h>
#include <glib.h>
--
1.8.4
Make picture consistent with example below it.
---
android/audio-ipc-api.txt | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/android/audio-ipc-api.txt b/android/audio-ipc-api.txt
index 4983ad7..5140d77 100644
--- a/android/audio-ipc-api.txt
+++ b/android/audio-ipc-api.txt
@@ -5,12 +5,12 @@ The audio plugin happen to be in a different socket but all the rules for
HAL socket apply here as well, the abstract socket name is
"\0bluez_audio_socket" (tentative):
- .--Android--. .---Audio---.
- | daemon | | Plugin |
+ .---Audio---. .--Android--.
+ | Plugin | | Daemon |
| | Command | |
- | | <-------------------------- | |
- | | | |
| | --------------------------> | |
+ | | | |
+ | | <-------------------------- | |
| | Response | |
| | | |
| | | |
--
1.8.4