Hi Mikel,
On Thu, Jul 4, 2013 at 9:36 AM, Mikel Astiz <mikel.astiz.oss@xxxxxxxxx> wrote:
> Hi Luiz,
>
> On Wed, Jul 3, 2013 at 5:15 PM, Luiz Augusto von Dentz
> <luiz.dentz@xxxxxxxxx> wrote:
>> From: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
>>
>> This remove the need of forward declaration of struct sink and prepare
>> for a complete removal of struct audio_device.
>> ---
>> profiles/audio/avdtp.c | 2 +-
>> profiles/audio/device.c | 54 ++++++++++++++++++++++++++++++-------------------
>> profiles/audio/device.h | 3 +--
>> profiles/audio/sink.c | 29 +++++++++++++++-----------
>> profiles/audio/sink.h | 5 +++--
>> 5 files changed, 55 insertions(+), 38 deletions(-)
>>
>> diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
>> index bd73572..0d67e67 100644
>> --- a/profiles/audio/avdtp.c
>> +++ b/profiles/audio/avdtp.c
>> @@ -1165,7 +1165,7 @@ static gboolean disconnect_timeout(gpointer user_data)
>> dev = manager_get_audio_device(session->device, FALSE);
>>
>> if (dev && dev->sink && stream_setup)
>> - sink_setup_stream(dev->sink, session);
>> + sink_setup_stream(dev, session);
>> else if (dev && dev->source && stream_setup)
>> source_setup_stream(dev->source, session);
>> else
>> diff --git a/profiles/audio/device.c b/profiles/audio/device.c
>> index 3fa49ad..f11c728 100644
>> --- a/profiles/audio/device.c
>> +++ b/profiles/audio/device.c
>> @@ -42,10 +42,12 @@
>> #include <dbus/dbus.h>
>> #include <gdbus/gdbus.h>
>>
>> -#include "log.h"
>> -#include "../src/adapter.h"
>> -#include "../src/device.h"
>> +#include "lib/uuid.h"
>> +#include "src/adapter.h"
>> +#include "src/device.h"
>> +#include "src/service.h"
>>
>> +#include "log.h"
>> #include "error.h"
>> #include "dbus-common.h"
>> #include "device.h"
>> @@ -61,7 +63,7 @@
>> #define AVDTP_CONNECT_TIMEOUT_BOOST 1
>>
>> struct dev_priv {
>> - sink_state_t sink_state;
>> + btd_service_state_t sink_state;
>> avctp_state_t avctp_state;
>>
>> guint control_timer;
>> @@ -69,9 +71,9 @@ struct dev_priv {
>>
>> gboolean disconnecting;
>>
>> + unsigned int service_cb_id;
>> unsigned int avdtp_callback_id;
>> unsigned int avctp_callback_id;
>> - unsigned int sink_callback_id;
>> };
>>
>> static void device_free(struct audio_device *dev)
>> @@ -87,7 +89,7 @@ static void device_free(struct audio_device *dev)
>>
>> avdtp_remove_state_cb(priv->avdtp_callback_id);
>> avctp_remove_state_cb(priv->avctp_callback_id);
>> - sink_remove_state_cb(priv->sink_callback_id);
>> + btd_service_remove_state_cb(priv->service_cb_id);
>>
>> g_free(priv);
>> }
>> @@ -138,6 +140,7 @@ static void disconnect_cb(struct btd_device *btd_dev, gboolean removal,
>> {
>> struct audio_device *dev = user_data;
>> struct dev_priv *priv = dev->priv;
>> + struct btd_service *sink;
>>
>> if (priv->disconnecting)
>> return;
>> @@ -149,7 +152,8 @@ static void disconnect_cb(struct btd_device *btd_dev, gboolean removal,
>> if (dev->control && priv->avctp_state != AVCTP_STATE_DISCONNECTED)
>> avrcp_disconnect(dev);
>>
>> - if (dev->sink && priv->sink_state != SINK_STATE_DISCONNECTED)
>> + sink = btd_device_get_service(btd_dev, A2DP_SINK_UUID);
>> + if (sink)
>> sink_disconnect(dev, TRUE);
>> else
>> priv->disconnecting = FALSE;
>> @@ -171,30 +175,25 @@ static void device_avdtp_cb(struct audio_device *dev, struct avdtp *session,
>> }
>>
>> static void device_sink_cb(struct audio_device *dev,
>> - sink_state_t old_state,
>> - sink_state_t new_state,
>> - void *user_data)
>> + btd_service_state_t old_state,
>> + btd_service_state_t new_state)
>> {
>> struct dev_priv *priv = dev->priv;
>>
>> - if (!dev->sink)
>> - return;
>> -
>> priv->sink_state = new_state;
>>
>> switch (new_state) {
>> - case SINK_STATE_DISCONNECTED:
>> + case BTD_SERVICE_STATE_UNAVAILABLE:
>> + case BTD_SERVICE_STATE_DISCONNECTED:
>> if (dev->control) {
>> device_remove_control_timer(dev);
>> if (priv->avctp_state != AVCTP_STATE_DISCONNECTED)
>> avrcp_disconnect(dev);
>> }
>> break;
>> - case SINK_STATE_CONNECTING:
>> - break;
>> - case SINK_STATE_CONNECTED:
>> - break;
>> - case SINK_STATE_PLAYING:
>> + case BTD_SERVICE_STATE_CONNECTING:
>> + case BTD_SERVICE_STATE_CONNECTED:
>> + case BTD_SERVICE_STATE_DISCONNECTING:
>> break;
>> }
>> }
>> @@ -222,6 +221,20 @@ static void device_avctp_cb(struct audio_device *dev, avctp_state_t old_state,
>> }
>> }
>>
>> +static void service_cb(struct btd_service *service,
>> + btd_service_state_t old_state,
>> + btd_service_state_t new_state,
>> + void *user_data)
>> +{
>> + struct audio_device *dev = user_data;
>> +
>> + if (dev->btd_dev != btd_service_get_device(service))
>> + return;
>> +
>> + if (service == dev->sink)
>> + device_sink_cb(dev, old_state, new_state);
>> +}
>> +
>> struct audio_device *audio_device_register(struct btd_device *device)
>> {
>> struct audio_device *dev;
>> @@ -236,8 +249,7 @@ struct audio_device *audio_device_register(struct btd_device *device)
>> dev->priv->dc_id = device_add_disconnect_watch(dev->btd_dev,
>> disconnect_cb, dev,
>> NULL);
>> - dev->priv->sink_callback_id = sink_add_state_cb(dev, device_sink_cb,
>> - NULL);
>> + dev->priv->service_cb_id = btd_service_add_state_cb(service_cb, dev);
>> dev->priv->avdtp_callback_id = avdtp_add_state_cb(dev, device_avdtp_cb);
>> dev->priv->avctp_callback_id = avctp_add_state_cb(dev, device_avctp_cb);
>>
>> diff --git a/profiles/audio/device.h b/profiles/audio/device.h
>> index 286bcdd..e24bdf9 100644
>> --- a/profiles/audio/device.h
>> +++ b/profiles/audio/device.h
>> @@ -25,13 +25,12 @@
>> struct audio_device;
>> struct source;
>> struct control;
>> -struct sink;
>> struct dev_priv;
>>
>> struct audio_device {
>> struct btd_device *btd_dev;
>>
>> - struct sink *sink;
>> + struct btd_service *sink;
>> struct source *source;
>> struct control *control;
>>
>> diff --git a/profiles/audio/sink.c b/profiles/audio/sink.c
>> index 3969417..f5b4e67 100644
>> --- a/profiles/audio/sink.c
>> +++ b/profiles/audio/sink.c
>> @@ -87,7 +87,7 @@ static char *str_state[] = {
>>
>> static void sink_set_state(struct audio_device *dev, sink_state_t new_state)
>> {
>> - struct sink *sink = dev->sink;
>> + struct sink *sink = btd_service_get_user_data(dev->sink);
>> sink_state_t old_state = sink->state;
>> GSList *l;
>>
>> @@ -119,7 +119,7 @@ static void avdtp_state_callback(struct audio_device *dev,
>> avdtp_session_state_t old_state,
>> avdtp_session_state_t new_state)
>> {
>> - struct sink *sink = dev->sink;
>> + struct sink *sink = btd_service_get_user_data(dev->sink);
>>
>> switch (new_state) {
>> case AVDTP_SESSION_STATE_DISCONNECTED:
>> @@ -142,7 +142,7 @@ static void stream_state_changed(struct avdtp_stream *stream,
>> void *user_data)
>> {
>> struct audio_device *dev = user_data;
>> - struct sink *sink = dev->sink;
>> + struct sink *sink = btd_service_get_user_data(dev->sink);
>>
>> if (err)
>> return;
>> @@ -291,8 +291,10 @@ failed:
>> sink->session = NULL;
>> }
>>
>> -gboolean sink_setup_stream(struct sink *sink, struct avdtp *session)
>> +gboolean sink_setup_stream(struct audio_device *dev, struct avdtp *session)
>> {
>> + struct sink *sink = btd_service_get_user_data(dev->sink);
>> +
>> if (sink->connect_id > 0 || sink->disconnect_id > 0)
>> return FALSE;
>>
>> @@ -310,7 +312,7 @@ gboolean sink_setup_stream(struct sink *sink, struct avdtp *session)
>>
>> int sink_connect(struct audio_device *dev)
>> {
>> - struct sink *sink = dev->sink;
>> + struct sink *sink = btd_service_get_user_data(dev->sink);
>>
>> if (!sink->session)
>> sink->session = avdtp_get(dev);
>> @@ -326,7 +328,7 @@ int sink_connect(struct audio_device *dev)
>> if (sink->stream_state >= AVDTP_STATE_OPEN)
>> return -EALREADY;
>>
>> - if (!sink_setup_stream(sink, NULL)) {
>> + if (!sink_setup_stream(dev, NULL)) {
>> DBG("Failed to create a stream");
>> return -EIO;
>> }
>> @@ -338,7 +340,7 @@ int sink_connect(struct audio_device *dev)
>>
>> static void sink_free(struct audio_device *dev)
>> {
>> - struct sink *sink = dev->sink;
>> + struct sink *sink = btd_service_get_user_data(dev->sink);
>>
>> if (sink->cb_id)
>> avdtp_stream_remove_cb(sink->session, sink->stream,
>> @@ -375,7 +377,8 @@ void sink_unregister(struct audio_device *dev)
>> sink_free(dev);
>> }
>>
>> -struct sink *sink_init(struct audio_device *dev, struct btd_service *service)
>> +struct btd_service *sink_init(struct audio_device *dev,
>> + struct btd_service *service)
>
> Why not make the return type bool or even void?
>
> Another interesting alternative would be to return 'int', such that
> the function eventually matches the btd_profile probe callback, and
> therefore the manager can be bypassed.
They idea here is that it would work as a ref so the return is stored
in audio_device but you are right we can probably return int to match
probe, gonna change this.
>
>> {
>> struct sink *sink;
>>
>> @@ -388,12 +391,14 @@ struct sink *sink_init(struct audio_device *dev, struct btd_service *service)
>>
>> sink->avdtp_callback_id = avdtp_add_state_cb(dev, avdtp_state_callback);
>>
>> - return sink;
>> + btd_service_set_user_data(service, sink);
>> +
>> + return service;
>> }
>>
>> gboolean sink_is_active(struct audio_device *dev)
>> {
>> - struct sink *sink = dev->sink;
>> + struct sink *sink = btd_service_get_user_data(dev->sink);
>>
>> if (sink->session)
>> return TRUE;
>> @@ -404,7 +409,7 @@ gboolean sink_is_active(struct audio_device *dev)
>> gboolean sink_new_stream(struct audio_device *dev, struct avdtp *session,
>> struct avdtp_stream *stream)
>> {
>> - struct sink *sink = dev->sink;
>> + struct sink *sink = btd_service_get_user_data(dev->sink);
>>
>> if (sink->stream)
>> return FALSE;
>> @@ -422,7 +427,7 @@ gboolean sink_new_stream(struct audio_device *dev, struct avdtp *session,
>>
>> int sink_disconnect(struct audio_device *dev, gboolean shutdown)
>> {
>> - struct sink *sink = dev->sink;
>> + struct sink *sink = btd_service_get_user_data(dev->sink);
>>
>> if (!sink->session)
>> return -ENOTCONN;
>> diff --git a/profiles/audio/sink.h b/profiles/audio/sink.h
>> index 1a80756..e3f974b 100644
>> --- a/profiles/audio/sink.h
>> +++ b/profiles/audio/sink.h
>> @@ -40,11 +40,12 @@ unsigned int sink_add_state_cb(struct audio_device *dev, sink_state_cb cb,
>> void *user_data);
>> gboolean sink_remove_state_cb(unsigned int id);
>>
>> -struct sink *sink_init(struct audio_device *dev, struct btd_service *service);
>> +struct btd_service *sink_init(struct audio_device *dev,
>> + struct btd_service *service);
>> void sink_unregister(struct audio_device *dev);
>> gboolean sink_is_active(struct audio_device *dev);
>> int sink_connect(struct audio_device *dev);
>> gboolean sink_new_stream(struct audio_device *dev, struct avdtp *session,
>> struct avdtp_stream *stream);
>> -gboolean sink_setup_stream(struct sink *sink, struct avdtp *session);
>> +gboolean sink_setup_stream(struct audio_device *dev, struct avdtp *session);
>
> Is there any reason not to use btd_service here directly, as later
> changed in patch 4?
The idea is to pack as little changes as possible, but I will take a
look perhaps such a change can be done sooner rather than latter.
--
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
