Hi Luiz,
On Tue, Feb 19, 2013 at 3:33 PM, Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx> wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
>
> If SEP is in configured state that means OPEN is about to happen so just
> mark start flag and send START once OPEN completes.
> ---
> profiles/audio/a2dp.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 54 insertions(+)
>
> diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
> index f1646ee..d9680a7 100644
> --- a/profiles/audio/a2dp.c
> +++ b/profiles/audio/a2dp.c
> @@ -336,6 +336,20 @@ static struct a2dp_setup *find_setup_by_dev(struct audio_device *dev)
> return NULL;
> }
>
> +static struct a2dp_setup *find_setup_by_stream(struct avdtp_stream *stream)
> +{
> + GSList *l;
> +
> + for (l = setups; l != NULL; l = l->next) {
> + struct a2dp_setup *setup = l->data;
> +
> + if (setup->stream == stream)
> + return setup;
> + }
> +
> + return NULL;
> +}
> +
> static void stream_state_changed(struct avdtp_stream *stream,
> avdtp_state_t old_state,
> avdtp_state_t new_state,
> @@ -344,6 +358,29 @@ static void stream_state_changed(struct avdtp_stream *stream,
> {
> struct a2dp_sep *sep = user_data;
>
> + if (new_state == AVDTP_STATE_OPEN) {
> + struct a2dp_setup *setup;
> + int err;
> +
> + setup = find_setup_by_stream(stream);
> + if (!setup || !setup->start)
> + return;
> +
> + setup->start = FALSE;
> +
> + err = avdtp_start(setup->session, stream);
> + if (err < 0 && err != -EINPROGRESS) {
> + error("avdtp_start: %s (%d)", strerror(-err), -err);
> + finalize_setup_errno(setup, err, finalize_resume,
> + NULL);
> + return;
> + }
> +
> + sep->starting = TRUE;
> +
> + return;
> + }
> +
Shouldn't you handle the error case here? Any other transition from
AVDTP_STATE_CONFIGURED should probably be considered a failure.
> if (new_state != AVDTP_STATE_IDLE)
> return;
>
> @@ -661,6 +698,12 @@ static gboolean open_ind(struct avdtp *session, struct avdtp_local_sep *sep,
>
> finalize_config(setup);
>
> + if (!setup->start || !err)
> + return TRUE;
> +
> + setup->start = FALSE;
> + finalize_resume(setup);
> +
> return TRUE;
> }
>
> @@ -689,6 +732,14 @@ static void open_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
> }
>
> finalize_config(setup);
> +
> + if (!setup->start || !err)
> + return;
> +
> + setup->start = FALSE;
> + finalize_resume(setup);
> +
> + return;
> }
>
> static gboolean suspend_timeout(struct a2dp_sep *sep)
> @@ -1718,6 +1769,9 @@ unsigned int a2dp_resume(struct avdtp *session, struct a2dp_sep *sep,
> case AVDTP_STATE_IDLE:
> goto failed;
> break;
> + case AVDTP_STATE_CONFIGURED:
> + setup->start = TRUE;
> + break;
> case AVDTP_STATE_OPEN:
> if (avdtp_start(session, sep->stream) < 0) {
> error("avdtp_start failed");
> --
This alternative has the obvious advantage of not introducing an
additional state to the transport and simpler APIs are generally good.
However, I see little benefit in the client's side and the
implementation gets a bit tricky in the server's side (BlueZ and
oFono).
I'd personally prefer to go simple and make this state explicit,
instead of trying to internally defer the operation.
Cheers,
Mikel
--
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
