Return-Path: Date: Thu, 4 Jul 2013 11:28:29 -0300 From: Vinicius Costa Gomes To: Luiz Augusto von Dentz Cc: linux-bluetooth@vger.kernel.org Subject: Re: [RFC 01/20] audio/sink: Use service user_data for private data Message-ID: <20130704142829.GA13087@samus.indt.org> References: <1372864541-10763-1-git-send-email-luiz.dentz@gmail.com> <1372864541-10763-2-git-send-email-luiz.dentz@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <1372864541-10763-2-git-send-email-luiz.dentz@gmail.com> Sender: linux-bluetooth-owner@vger.kernel.org List-ID: Hi, On 18:15 Wed 03 Jul, Luiz Augusto von Dentz wrote: > From: Luiz Augusto von Dentz > > 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(-) > [snip] > @@ -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); I just want to bring up an issue not related to the patch in itself. Wouldn't it be better if the state changed callback were per service? I can see it being used a lot. Or was this point already discussed? > 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) > { > 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); > int sink_disconnect(struct audio_device *dev, gboolean shutdown); > -- > 1.8.1.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Cheers, -- Vinicius