2011-03-15 14:27:40

by Arun Raghavan

[permalink] [raw]
Subject: Switching between SBC and MPEG audio on headsets

Hello,
I've been trying to set up PulseAudio to be able to switch the A2DP sink
dynamically between SBC and MPEG modes. I've got this working now [1], but I
did face one problem on the bluez side. When sending a reconfigure request, the
request always goes to the SEID that was used previously (=> always to the SBC
SEID). Trying to reconfigure for MPEG therefore results the headset returning
an error. I'm not very familiar with how this is supposed to work, but the
patch following this mail seems to work (I can now switch back and forth
between SBC and MPEG modes). It basically forces figuring out what remote SEP
to talk to while reconfiguring.

Is this the right approach?

Thanks,
Arun

[1] http://git.collabora.co.uk/?p=user/arun/pulseaudio.git;a=shortlog;h=refs/heads/passthrough-bt



2011-03-18 09:43:03

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH] Recalculate remote SEP if the codec type changes

Hi Arun,

On Thu, Mar 17, 2011, Arun Raghavan wrote:
> This forces recalculating the remote SEP if the local SEP's codec type
> is no longer the same as the remote SEP's codec type. This can happen
> after we issue a BT_STOP_STREAM+BT_CLOSE followed by a
> BT_SET_CONFIGURATION with a new SEID.
> ---
> audio/a2dp.c | 9 ++++++++-
> 1 files changed, 8 insertions(+), 1 deletions(-)

Pushed upstream. Thanks.

Johan

2011-03-17 21:33:49

by Peter Dons Tychsen

[permalink] [raw]
Subject: Re: Switching between SBC and MPEG audio on headsets

Hi Brian,

On Thu, 2011-03-17 at 09:19 -0700, Brian Gix wrote:
> AVDTP only allows a single signaling channel to exist between any two
> peers. You are allowed to have separate signaling channels to
> separate
> remote devices, but even if you are supporting multiple media streams
> (video/audio, or audio/audio) to the same device, all AVDTP signaling
> must be done on a single channel.

Yes, exactly. But i am not sure everyone caught that part back when A2DP
was introduced. :-)

> Actually, I thought I was agreeing with you about *not* having two
> media
> channels open simultaneously.

Yes, you were. I just wanted to add an extra warning if someone still
wanted to go that way :-). I agree that compatibility is very important.

Thanks,

/pedro







2011-03-17 16:19:37

by Brian Gix

[permalink] [raw]
Subject: Re: Switching between SBC and MPEG audio on headsets

Hi Pedro,

On 3/17/2011 2:35 AM, Peter Dons Tychsen wrote:
> Hi Brian,
>
> On Wed, 2011-03-16 at 16:09 -0700, Brian Gix wrote:
>> This is not actually true. You can have multiple media channels open
>> simultaneously (for instance one for Video and one for Audio) which
>> are
>> opened with careful handshaking with the AVDTP_OPEN signaling
>> command.
>> In theory, this could be two audio channels as well. If someone wants
>> a
>> Journaling channel, it would be opened without the AVDTP_OPEN
>> signaling.
>
> Yes, except protocol-wise the new channel on PSM=0x19 could also be the
> start of a new A2DP signaling connection (however very unlikely). As you
> mentioned, the problem with the Journaling channel can be filtered out
> with correct signaling.

AVDTP only allows a single signaling channel to exist between any two
peers. You are allowed to have separate signaling channels to separate
remote devices, but even if you are supporting multiple media streams
(video/audio, or audio/audio) to the same device, all AVDTP signaling
must be done on a single channel.

>
> I totally agree with you that your solution is the most elegant. However
> back when A2DP was introduced it did not seem clear to everyone how
> multiple streams would work. Some thought it would work with multiple
> media channels (like you suggest), and some thought it would work with
> multiple signaling channels (with max 1 media channel).
>
> Most of the implementations i have seen (for headsets), unfortunately
> ended up with the last solution. This means that they will not accept a
> secondary media channel. This is unfortunately basically true for all of
> the most popular chipsets and SDKs.

Actually, I thought I was agreeing with you about *not* having two media
channels open simultaneously. There are too many legacy devices in the
market that do not support a second media channel for audio. I was just
pointing out that the limitations are in the implementations, not the
specification. And I am a big believer in maintaining compatibility with
legacy devices. It is possible to have two spec compliant devices that
do not interoperate, which of course is why we have UPFs.

> Your suggestion is clever and follows the standard nicely (and would get
> my vote for a white-paper), but i doubt it will work with many headsets
> out there.
>
> Even worse, many headsets are hardcoded (not nice) to believe that the
> 3rd channel is always the Journaling channel. That is of course, just
> plain wrong due to the reasons you have stated.

I haven't actually even encountered any headsets in the wild that
support Journaling.

>
> Thanks,
>
> /pedro
>


--
Brian Gix
[email protected]
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

2011-03-17 09:35:42

by Peter Dons Tychsen

[permalink] [raw]
Subject: Re: Switching between SBC and MPEG audio on headsets

Hi Brian,

On Wed, 2011-03-16 at 16:09 -0700, Brian Gix wrote:
> This is not actually true. You can have multiple media channels open
> simultaneously (for instance one for Video and one for Audio) which
> are
> opened with careful handshaking with the AVDTP_OPEN signaling
> command.
> In theory, this could be two audio channels as well. If someone wants
> a
> Journaling channel, it would be opened without the AVDTP_OPEN
> signaling.

Yes, except protocol-wise the new channel on PSM=0x19 could also be the
start of a new A2DP signaling connection (however very unlikely). As you
mentioned, the problem with the Journaling channel can be filtered out
with correct signaling.

I totally agree with you that your solution is the most elegant. However
back when A2DP was introduced it did not seem clear to everyone how
multiple streams would work. Some thought it would work with multiple
media channels (like you suggest), and some thought it would work with
multiple signaling channels (with max 1 media channel).

Most of the implementations i have seen (for headsets), unfortunately
ended up with the last solution. This means that they will not accept a
secondary media channel. This is unfortunately basically true for all of
the most popular chipsets and SDKs.

Your suggestion is clever and follows the standard nicely (and would get
my vote for a white-paper), but i doubt it will work with many headsets
out there.

Even worse, many headsets are hardcoded (not nice) to believe that the
3rd channel is always the Journaling channel. That is of course, just
plain wrong due to the reasons you have stated.

Thanks,

/pedro




2011-03-16 23:09:40

by Brian Gix

[permalink] [raw]
Subject: Re: Switching between SBC and MPEG audio on headsets

On 3/16/2011 3:38 PM, Peter Dons Tychsen wrote:
> Hello,
>
> On Tue, 2011-03-15 at 16:29 -0300, Luiz Augusto von Dentz wrote:
>> Actually
>> I would suggest configuring both endpoint since the beginning so that
>> we only need to suspend/resume to switch between them, but I don't
>> think many headsets would be able to handle this situation.
>
> Unfortunately, because of a rather silly flaw in the A2DP specifications
> you cannot do this. The reason is simple. You would need a separate
> L2CAP channel for each data link. A2DP uses up to three L2CAP channels
> all on L2CAP PSM=0x19:
>
> 1) First is always signalling channel.
> 2) Second is always data channel.
> 3) Third is always Journaling channel (never really used).
>
> So if you already had the first two, and you opened yet another on
> PSM=0x19 from the same device, then it could be one of the following:
>
> 1) A new signalling channel (multi-profile).
> 2) A new data channel for already opened profile.
> 3) A new Journalling channel for already opened profile.
>

This is not actually true. You can have multiple media channels open
simultaneously (for instance one for Video and one for Audio) which are
opened with careful handshaking with the AVDTP_OPEN signaling command.
In theory, this could be two audio channels as well. If someone wants a
Journaling channel, it would be opened without the AVDTP_OPEN signaling.

The support for multiple streaming channels is explicitly allowed for in
the specification (AVDTP v1.2, section 6.10 - Stream Establishment).


[...]

--
Brian Gix
[email protected]
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

2011-03-16 22:38:31

by Peter Dons Tychsen

[permalink] [raw]
Subject: Re: Switching between SBC and MPEG audio on headsets

Hello,

On Tue, 2011-03-15 at 16:29 -0300, Luiz Augusto von Dentz wrote:
> Actually
> I would suggest configuring both endpoint since the beginning so that
> we only need to suspend/resume to switch between them, but I don't
> think many headsets would be able to handle this situation.

Unfortunately, because of a rather silly flaw in the A2DP specifications
you cannot do this. The reason is simple. You would need a separate
L2CAP channel for each data link. A2DP uses up to three L2CAP channels
all on L2CAP PSM=0x19:

1) First is always signalling channel.
2) Second is always data channel.
3) Third is always Journaling channel (never really used).

So if you already had the first two, and you opened yet another on
PSM=0x19 from the same device, then it could be one of the following:

1) A new signalling channel (multi-profile).
2) A new data channel for already opened profile.
3) A new Journalling channel for already opened profile.

Because of the specification problem mentioned above, the problem is not
really solvable, and the implementation is forced to assume that the new
L2CAP channel is the 3rd channel (Journalling). The correct solution
would of course have been a dynamic PSM for each service, and for each
profile.

So current the only portable way of supporting multiple codecs is:

1) Close existing data channel.
2) Setup config.
3) Re-open data channel.
4) Send "start".

Anything else will most likely not work with the headsets in the wild.

And also a note: Do not *ever* cache the data from the discovery phase.
That would only make the same mistake many have done with SDP queries.
If you are using a multi-link headset, the other connected device might
have changed which codecs are available (or left), and in some cases it
could also change which options and settings are supported.

Thanks,

/pedro

2011-03-16 20:05:57

by Arun Raghavan

[permalink] [raw]
Subject: [PATCH] Recalculate remote SEP if the codec type changes

This forces recalculating the remote SEP if the local SEP's codec type
is no longer the same as the remote SEP's codec type. This can happen
after we issue a BT_STOP_STREAM+BT_CLOSE followed by a
BT_SET_CONFIGURATION with a new SEID.
---
audio/a2dp.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/audio/a2dp.c b/audio/a2dp.c
index 3407d6f..7827cda 100644
--- a/audio/a2dp.c
+++ b/audio/a2dp.c
@@ -1058,8 +1058,15 @@ static gboolean a2dp_reconfigure(gpointer data)
struct a2dp_setup *setup = data;
struct a2dp_sep *sep = setup->sep;
int posix_err;
+ struct avdtp_media_codec_capability *rsep_codec;
+ struct avdtp_service_capability *cap;

- if (!setup->rsep)
+ if (setup->rsep) {
+ cap = avdtp_get_codec(setup->rsep);
+ rsep_codec = (struct avdtp_media_codec_capability *) cap->data;
+ }
+
+ if (!setup->rsep || sep->codec != rsep_codec->media_codec_type)
setup->rsep = avdtp_find_remote_sep(setup->session, sep->lsep);

posix_err = avdtp_set_configuration(setup->session, setup->rsep,
--
1.7.4.1


2011-03-16 18:19:55

by Arun Raghavan

[permalink] [raw]
Subject: Re: Switching between SBC and MPEG audio on headsets

On Tue, 2011-03-15 at 13:43 -0700, Brian Gix wrote:
> Hi Arun,
>
> On 3/15/2011 12:51 PM, Arun Raghavan wrote:
> > Hey Brian,
> >
>
> [...]
>
> >
> > I'm quite unfamiliar with how this works, so please excuse any wild
> > inaccuracies in terminology. I believe that I am doing it this way
> > already (mostly discovered by trial and error). When I want to
> > reconfigure, I do the following:
> >
> > - Send a BT_STOP_STREAM request, get the expected response
> > - Send a BT_CLOSE request, get the expected response
> > - Send a BT_OPEN request with the new seid, get the expected response
> > - Send a BT_SET_CONFIGURATION request with the new caps, get the
> > expected response
> > - Send a BT_START_STREAM, wait for the response
> > - Start streaming
>
> Well, with full knowledge that I *haven't* examined this part of BlueZ,
> I can say that from a protocol perspective, the AVDTP_SET_CONFIG must go
> over the air/be acknowledged before the AVDTP_OPEN can be
> sent/acknowledged. I haven't checked to see if those step are
> obfuscated at all by the BT_OPEN/BT_SET_CONFIGURATION naming convention,
> or if it is a simple mapping.
>
> On a compliant device, an Open directly following a Close should return
> a NOT_CONFIGURED error. But it may be that a less strict SNK device
> might assume the last configuration and overlook the protocol error.

You're right - the BT_OPEN doesn't result in an AVDTP_OPEN, this happens
after the SET_CONFIGURATION is done. I'll try to hack up a patch to look
up a new rsep if the media type of the lsep and currently selected rsep
do not match.

Cheers,
Arun

p.s.: FWIW, the debug log looks like this:

bluetoothd[17790]: audio/unix.c:client_cb() Audio API: BT_REQUEST <- BT_STOP_STREAM
bluetoothd[17790]: audio/avdtp.c:avdtp_ref() 0x7fc7f57aa260: ref=4
bluetoothd[17790]: audio/a2dp.c:setup_ref() 0x7fc7f57a86d0: ref=1
bluetoothd[17790]: audio/avdtp.c:session_cb()
bluetoothd[17790]: audio/avdtp.c:avdtp_parse_resp() SUSPEND request succeeded
bluetoothd[17790]: audio/avdtp.c:avdtp_sep_set_state() stream state changed: STREAMING -> OPEN
bluetoothd[17790]: audio/sink.c:sink_set_state() State changed /org/bluez/17790/hci0/dev_00_0B_E4_91_11_A1: SINK_STATE_PLAYING -> SINK_STATE_CONNECTED
bluetoothd[17790]: audio/a2dp.c:suspend_cfm() Source 0x7fc7f57869d0: Suspend_Cfm
bluetoothd[17790]: audio/unix.c:unix_ipc_sendmsg() Audio API: BT_RESPONSE -> BT_STOP_STREAM
bluetoothd[17790]: audio/a2dp.c:setup_unref() 0x7fc7f57a86d0: ref=0
bluetoothd[17790]: audio/a2dp.c:setup_free() 0x7fc7f57a86d0
bluetoothd[17790]: audio/avdtp.c:avdtp_unref() 0x7fc7f57aa260: ref=3
bluetoothd[17790]: audio/unix.c:client_cb() Audio API: BT_REQUEST <- BT_SUSPEND_STREAM
bluetoothd[17790]: audio/a2dp.c:a2dp_sep_unlock() SEP 0x7fc7f57869d0 unlocked
bluetoothd[17790]: audio/avdtp.c:avdtp_unref() 0x7fc7f57aa260: ref=2
bluetoothd[17790]: audio/unix.c:unix_ipc_sendmsg() Audio API: BT_RESPONSE -> BT_SUSPEND_STREAM
bluetoothd[17790]: audio/unix.c:client_cb() Audio API: BT_REQUEST <- BT_OPEN
bluetoothd[17790]: audio/unix.c:handle_a2dp_open() open a2dp - object=/org/bluez/17790/hci0/dev_00_0B_E4_91_11_A1 source=ANY destination=ANY lock=write
bluetoothd[17790]: audio/avdtp.c:avdtp_ref() 0x7fc7f57aa260: ref=3
bluetoothd[17790]: audio/a2dp.c:a2dp_sep_lock() SEP 0x7fc7f5787250 locked
bluetoothd[17790]: audio/unix.c:unix_ipc_sendmsg() Audio API: BT_RESPONSE -> BT_OPEN
bluetoothd[17790]: audio/unix.c:client_cb() Audio API: BT_REQUEST <- BT_SET_CONFIGURATION
bluetoothd[17790]: audio/unix.c:print_mpeg12() Media Codec: MPEG12 Channel Modes: Stereo Frequencies: 44.1Khz Layers: 3 CRC: No
bluetoothd[17790]: audio/a2dp.c:a2dp_config() a2dp_config: selected SEP 0x7fc7f5787250
bluetoothd[17790]: audio/avdtp.c:avdtp_ref() 0x7fc7f57aa260: ref=4
bluetoothd[17790]: audio/a2dp.c:setup_ref() 0x7fc7f57a86d0: ref=1
bluetoothd[17790]: audio/avdtp.c:session_cb()
bluetoothd[17790]: audio/avdtp.c:avdtp_parse_resp() CLOSE request succeeded
bluetoothd[17790]: audio/avdtp.c:avdtp_sep_set_state() stream state changed: OPEN -> CLOSING
bluetoothd[17790]: audio/a2dp.c:close_cfm() Source 0x7fc7f57869d0: Close_Cfm
bluetoothd[17790]: audio/avdtp.c:avdtp_sep_set_state() stream state changed: CLOSING -> IDLE
bluetoothd[17790]: audio/avdtp.c:avdtp_unref() 0x7fc7f57aa260: ref=3
bluetoothd[17790]: audio/avdtp.c:avdtp_set_configuration() 0x7fc7f57aa260: int_seid=2, acp_seid=2
bluetoothd[17790]: audio/avdtp.c:session_cb()
bluetoothd[17790]: audio/avdtp.c:avdtp_parse_resp() SET_CONFIGURATION request succeeded
bluetoothd[17790]: audio/a2dp.c:setconf_cfm() Source 0x7fc7f5787250: Set_Configuration_Cfm
bluetoothd[17790]: audio/avdtp.c:avdtp_ref() 0x7fc7f57aa260: ref=4
bluetoothd[17790]: audio/avdtp.c:avdtp_sep_set_state() stream state changed: IDLE -> CONFIGURED
bluetoothd[17790]: audio/avdtp.c:session_cb()
bluetoothd[17790]: audio/avdtp.c:avdtp_parse_resp() OPEN request succeeded
bluetoothd[17790]: audio/avdtp.c:avdtp_connect_cb() AVDTP: connected transport channel to 00:0B:E4:91:11:A1
bluetoothd[17790]: audio/avdtp.c:handle_transport_connect() sk 26, omtu 895, send buffer size 61440
bluetoothd[17790]: audio/a2dp.c:open_cfm() Source 0x7fc7f5787250: Open_Cfm
bluetoothd[17790]: audio/unix.c:unix_ipc_sendmsg() Audio API: BT_RESPONSE -> BT_SET_CONFIGURATION
bluetoothd[17790]: audio/a2dp.c:setup_unref() 0x7fc7f57a86d0: ref=0
bluetoothd[17790]: audio/a2dp.c:setup_free() 0x7fc7f57a86d0
bluetoothd[17790]: audio/avdtp.c:avdtp_unref() 0x7fc7f57aa260: ref=3
bluetoothd[17790]: audio/avdtp.c:avdtp_sep_set_state() stream state changed: CONFIGURED -> OPEN
bluetoothd[17790]: audio/sink.c:sink_set_state() State changed /org/bluez/17790/hci0/dev_00_0B_E4_91_11_A1: SINK_STATE_CONNECTED -> SINK_STATE_CONNECTED
bluetoothd[17790]: audio/device.c:device_set_state() state change attempted from connected to connected
bluetoothd[17790]: audio/unix.c:client_cb() Audio API: BT_REQUEST <- BT_START_STREAM
bluetoothd[17790]: audio/avdtp.c:avdtp_ref() 0x7fc7f57aa260: ref=4
bluetoothd[17790]: audio/a2dp.c:setup_ref() 0x7fc7f57a86d0: ref=1
bluetoothd[17790]: audio/avdtp.c:session_cb()
bluetoothd[17790]: audio/avdtp.c:avdtp_parse_resp() START request succeeded
bluetoothd[17790]: audio/a2dp.c:start_cfm() Source 0x7fc7f5787250: Start_Cfm
bluetoothd[17790]: audio/unix.c:unix_ipc_sendmsg() Audio API: BT_RESPONSE -> BT_START_STREAM
bluetoothd[17790]: audio/unix.c:unix_ipc_sendmsg() Audio API: BT_RESPONSE -> BT_NEW_STREAM
bluetoothd[17790]: audio/a2dp.c:setup_unref() 0x7fc7f57a86d0: ref=0
bluetoothd[17790]: audio/a2dp.c:setup_free() 0x7fc7f57a86d0
bluetoothd[17790]: audio/avdtp.c:avdtp_unref() 0x7fc7f57aa260: ref=3
bluetoothd[17790]: audio/avdtp.c:avdtp_sep_set_state() stream state changed: OPEN -> STREAMING
bluetoothd[17790]: audio/sink.c:sink_set_state() State changed /org/bluez/17790/hci0/dev_00_0B_E4_91_11_A1: SINK_STATE_CONNECTED -> SINK_STATE_PLAYING


2011-03-15 20:50:59

by Brian Gix

[permalink] [raw]
Subject: Re: Switching between SBC and MPEG audio on headsets

Hi Luiz,

On 3/15/2011 1:21 PM, Luiz Augusto von Dentz wrote:
> Hi Brian,
>
> On Tue, Mar 15, 2011 at 4:41 PM, Brian Gix<[email protected]> wrote:
>> On 3/15/2011 12:29 PM, Luiz Augusto von Dentz wrote:
>>>
>>> Hi,
>>>
>>> On Tue, Mar 15, 2011 at 2:01 PM, Johan Hedberg<[email protected]>
>>> wrote:
>>>>
>>>> Hi Arun,
>>>>
>>>> On Tue, Mar 15, 2011, Arun Raghavan wrote:
>>>>>
>>>>> I've been trying to set up PulseAudio to be able to switch the A2DP sink
>>>>> dynamically between SBC and MPEG modes. I've got this working now [1],
>>>>> but I
>>>>> did face one problem on the bluez side. When sending a reconfigure
>>>>> request, the
>>>>> request always goes to the SEID that was used previously (=> always to
>>>>> the SBC
>>>>> SEID). Trying to reconfigure for MPEG therefore results the headset
>>>>> returning
>>>>> an error. I'm not very familiar with how this is supposed to work, but
>>>>> the
>>>>> patch following this mail seems to work (I can now switch back and forth
>>>>> between SBC and MPEG modes). It basically forces figuring out what
>>>>> remote SEP
>>>>> to talk to while reconfiguring.
>>>>>
>>>>> Is this the right approach?
>>>>
>>>> I'm not sure about the places where you set the value to NULL, but the
>>>> place in close_cfm which you remove isn't acceptable as such. It was
>>>> originally created to fix the issue reported in this thread:
>>>> http://marc.info/?l=linux-bluetooth&m=129190286303247&w=2
>>>>
>>>> Only a fix which doesn't break the use-case reported there can be
>>>> accepted upstream. FWIW, the commit that introduced the fix is de96fcd8.
>>>
>>> Also the solution should consider a transition and not dropping the
>>> stream completely before configuring the other, otherwise errors may
>>> cause a complete disconnect just to switch between endpoints. Actually
>>> I would suggest configuring both endpoint since the beginning so that
>>> we only need to suspend/resume to switch between them, but I don't
>>> think many headsets would be able to handle this situation.
>>>
>>
>> I think most headsets will not support this because it would require an
>> extra L2CAP connection to be maintained.
>>
>> I don't think there is much of a big deal with closing the first endpoint
>> relationship prior to configuring the next, becuase the AVDTP signaling
>> channel will keep the underlying ACL connection open. Headsets I have worked
>> on in the past would reject a SET_CONFIG if an existing local/remote SEID
>> relationship already existed, not only for the extra resources required to
>> maintain the extra L2CAP channel, but also because the SET_CONFIG is when
>> the hw resources are reserved and configured for the actual PCM data stream.
>>
>> And assuming that the ACL doesn't get dropped, the critical re-setup time is
>> probably most bounded by the underlying hw reconfiguration, and not so much
>> by the few extra small ACL pkts that need to be exchanged.
>
> The problem is that the audio routing is broken if the new stream
> configuration fails, and even if sbc stream is configured latter it
> may not be recovered and some audio maybe lost of rerouted to the
> speakers which I believe is not the best experience for users.
>
> Also we only need another connection in case of AVDTP_Open is sent,
> this sound more a limitation of some implementation than a avdtp/a2dp
> problem, the problem with disconnect is that if the stream setup fails
> the session reference is dropped and the session is released (avdtp
> signalling) so right now we have no means to hold the ACL link if
> nothing else is connected e.g. hfp, we could in theory fix this by
> having a setup which revert to the last configuration if the new one
> fails but I fear this would required a special API to applications be
> able to tells us when this is necessary.
>

I agree that it would be a limitation of the implementation and not the
specification to not allow multiple endpoints to be configured
simultaneously. However many of these headsets are designed on resource
constrained platforms, and it is perfectly allowable for them within the
specification to reject a second endpoint configuration due to
insufficient resources.

And although you could configure two endpoints simultaneously without
opening up both of their streaming L2CAP channels, as soon as you made
the first switch, the second streaming channel would be open. A
suspended streaming channel is still open and consuming resources. The
only way to recover those resources is to issue CLOSE, which then
requires another SET_CONFIG before it could be opened again.

I suppose you could close the channel, and then set up the configuration
again while streaming to the 2nd, so that it is ready to be opened for
the switch-back.


--
Brian Gix
[email protected]
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

2011-03-15 20:43:29

by Brian Gix

[permalink] [raw]
Subject: Re: Switching between SBC and MPEG audio on headsets

Hi Arun,

On 3/15/2011 12:51 PM, Arun Raghavan wrote:
> Hey Brian,
>

[...]

>
> I'm quite unfamiliar with how this works, so please excuse any wild
> inaccuracies in terminology. I believe that I am doing it this way
> already (mostly discovered by trial and error). When I want to
> reconfigure, I do the following:
>
> - Send a BT_STOP_STREAM request, get the expected response
> - Send a BT_CLOSE request, get the expected response
> - Send a BT_OPEN request with the new seid, get the expected response
> - Send a BT_SET_CONFIGURATION request with the new caps, get the
> expected response
> - Send a BT_START_STREAM, wait for the response
> - Start streaming

Well, with full knowledge that I *haven't* examined this part of BlueZ,
I can say that from a protocol perspective, the AVDTP_SET_CONFIG must go
over the air/be acknowledged before the AVDTP_OPEN can be
sent/acknowledged. I haven't checked to see if those step are
obfuscated at all by the BT_OPEN/BT_SET_CONFIGURATION naming convention,
or if it is a simple mapping.

On a compliant device, an Open directly following a Close should return
a NOT_CONFIGURED error. But it may be that a less strict SNK device
might assume the last configuration and overlook the protocol error.

>
> This actually does work here after I apply my patch, although switching
> sample rates doesn't seem to actually happen (probably just something
> broken in my code).
>
> So if I understand this correctly, it should be sufficient to modify my
> patch so we only search for a new remote endpoint if the local endpoint
> changed (or a more specific change limited to if the codec type
> changed)?
>
> Cheers,
> Arun
>
> --
> 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


--
Brian Gix
[email protected]
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

2011-03-15 20:21:38

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: Switching between SBC and MPEG audio on headsets

Hi Brian,

On Tue, Mar 15, 2011 at 4:41 PM, Brian Gix <[email protected]> wrote:
> On 3/15/2011 12:29 PM, Luiz Augusto von Dentz wrote:
>>
>> Hi,
>>
>> On Tue, Mar 15, 2011 at 2:01 PM, Johan Hedberg<[email protected]>
>> ?wrote:
>>>
>>> Hi Arun,
>>>
>>> On Tue, Mar 15, 2011, Arun Raghavan wrote:
>>>>
>>>> I've been trying to set up PulseAudio to be able to switch the A2DP sink
>>>> dynamically between SBC and MPEG modes. I've got this working now [1],
>>>> but I
>>>> did face one problem on the bluez side. When sending a reconfigure
>>>> request, the
>>>> request always goes to the SEID that was used previously (=> ?always to
>>>> the SBC
>>>> SEID). Trying to reconfigure for MPEG therefore results the headset
>>>> returning
>>>> an error. I'm not very familiar with how this is supposed to work, but
>>>> the
>>>> patch following this mail seems to work (I can now switch back and forth
>>>> between SBC and MPEG modes). It basically forces figuring out what
>>>> remote SEP
>>>> to talk to while reconfiguring.
>>>>
>>>> Is this the right approach?
>>>
>>> I'm not sure about the places where you set the value to NULL, but the
>>> place in close_cfm which you remove isn't acceptable as such. It was
>>> originally created to fix the issue reported in this thread:
>>> http://marc.info/?l=linux-bluetooth&m=129190286303247&w=2
>>>
>>> Only a fix which doesn't break the use-case reported there can be
>>> accepted upstream. FWIW, the commit that introduced the fix is de96fcd8.
>>
>> Also the solution should consider a transition and not dropping the
>> stream completely before configuring the other, otherwise errors may
>> cause a complete disconnect just to switch between endpoints. Actually
>> I would suggest configuring both endpoint since the beginning so that
>> we only need to suspend/resume to switch between them, but I don't
>> think many headsets would be able to handle this situation.
>>
>
> I think most headsets will not support this because it would require an
> extra L2CAP connection to be maintained.
>
> I don't think there is much of a big deal with closing the first endpoint
> relationship prior to configuring the next, becuase the AVDTP signaling
> channel will keep the underlying ACL connection open. Headsets I have worked
> on in the past would reject a SET_CONFIG if an existing local/remote SEID
> relationship already existed, not only for the extra resources required to
> maintain the extra L2CAP channel, but also because the SET_CONFIG is when
> the hw resources are reserved and configured for the actual PCM data stream.
>
> And assuming that the ACL doesn't get dropped, the critical re-setup time is
> probably most bounded by the underlying hw reconfiguration, and not so much
> by the few extra small ACL pkts that need to be exchanged.

The problem is that the audio routing is broken if the new stream
configuration fails, and even if sbc stream is configured latter it
may not be recovered and some audio maybe lost of rerouted to the
speakers which I believe is not the best experience for users.

Also we only need another connection in case of AVDTP_Open is sent,
this sound more a limitation of some implementation than a avdtp/a2dp
problem, the problem with disconnect is that if the stream setup fails
the session reference is dropped and the session is released (avdtp
signalling) so right now we have no means to hold the ACL link if
nothing else is connected e.g. hfp, we could in theory fix this by
having a setup which revert to the last configuration if the new one
fails but I fear this would required a special API to applications be
able to tells us when this is necessary.

--
Luiz Augusto von Dentz
Computer Engineer

2011-03-15 19:51:10

by Arun Raghavan

[permalink] [raw]
Subject: Re: Switching between SBC and MPEG audio on headsets

Hey Brian,

On Tue, 2011-03-15 at 09:22 -0700, Brian Gix wrote:
[...]
> No this is the incorrect approach.

Thanks (to you and Johan) for the detailed explanation! :)

> Both SRC and SNK devices have a variety of endpoints that have
> capabilities describing what that endpoint is capable of, the most
> important of which is the Codec Capability. Each endpoint is allowed a
> single capability of a particular type, which means that an endpoint
> that supports SBC should not attempt to use a different codec (for MPEG
> in this case). Any device that attempts to wedge both codecs into a
> single endpoint would most definitely be non-compliant.
>
> The SUSPEND/RECONFIGURE/START procedure will not work to switch between
> SBC and MPEG for this reason. The purpose for that procedure is to
> allow quick changes within the bounds of the current (remote and local)
> endpoint capabilities. To quickly switch between a 44.1KHz and 48KHz
> sampling for instance.
>
> The correct procedure to switch between SBC and MPEG would be to close
> the existing media channel (but NOT the AVDTP signaling channel) and
> reset up a new endpoint. If you know you will be doing this, you can
> save time by caching all of the remote devices endpoints the first time
> you do an AVDTP_DISCOVER, so that you have all the remote endpoint info
> you need to make the switch (and even know if the switch will work). You
> also should have all supported local endpoints separated into their own
> endpoints of course.
>
> The to switch, you will need to:
>
> <halt streaming>
>
> AVDTP_CLOSE (on AVDTP signaling channel)
>
> L2CAP_CLOSE (close the media L2CAP channel for SBC)
>
> <<<< You should Rx AVDTP_CLOSE_CFM
>
> AVDTP_SET_CONFIG (on AVDTP signaling channel,
> specify a local and remote endpoint that are compatible)
>
> <<<<< You should Rx SET_CONFIG_CFM
>
> AVDTP_OPEN (on AVDTP signaling channel)
>
> L2CAP_OPEN (open new media L2CAP channel for MPEG)
>
> <<<<< You should Rx AVDTP_OPEN_CFM
>
> AVDTP_START (on AVDTP signaling channel)
>
> <<<<< You should Rx AVDTP_START_CFM
>
> <start streaming>

I'm quite unfamiliar with how this works, so please excuse any wild
inaccuracies in terminology. I believe that I am doing it this way
already (mostly discovered by trial and error). When I want to
reconfigure, I do the following:

- Send a BT_STOP_STREAM request, get the expected response
- Send a BT_CLOSE request, get the expected response
- Send a BT_OPEN request with the new seid, get the expected response
- Send a BT_SET_CONFIGURATION request with the new caps, get the
expected response
- Send a BT_START_STREAM, wait for the response
- Start streaming

This actually does work here after I apply my patch, although switching
sample rates doesn't seem to actually happen (probably just something
broken in my code).

So if I understand this correctly, it should be sufficient to modify my
patch so we only search for a new remote endpoint if the local endpoint
changed (or a more specific change limited to if the codec type
changed)?

Cheers,
Arun


2011-03-15 19:41:06

by Brian Gix

[permalink] [raw]
Subject: Re: Switching between SBC and MPEG audio on headsets

On 3/15/2011 12:29 PM, Luiz Augusto von Dentz wrote:
> Hi,
>
> On Tue, Mar 15, 2011 at 2:01 PM, Johan Hedberg<[email protected]> wrote:
>> Hi Arun,
>>
>> On Tue, Mar 15, 2011, Arun Raghavan wrote:
>>> I've been trying to set up PulseAudio to be able to switch the A2DP sink
>>> dynamically between SBC and MPEG modes. I've got this working now [1], but I
>>> did face one problem on the bluez side. When sending a reconfigure request, the
>>> request always goes to the SEID that was used previously (=> always to the SBC
>>> SEID). Trying to reconfigure for MPEG therefore results the headset returning
>>> an error. I'm not very familiar with how this is supposed to work, but the
>>> patch following this mail seems to work (I can now switch back and forth
>>> between SBC and MPEG modes). It basically forces figuring out what remote SEP
>>> to talk to while reconfiguring.
>>>
>>> Is this the right approach?
>>
>> I'm not sure about the places where you set the value to NULL, but the
>> place in close_cfm which you remove isn't acceptable as such. It was
>> originally created to fix the issue reported in this thread:
>> http://marc.info/?l=linux-bluetooth&m=129190286303247&w=2
>>
>> Only a fix which doesn't break the use-case reported there can be
>> accepted upstream. FWIW, the commit that introduced the fix is de96fcd8.
>
> Also the solution should consider a transition and not dropping the
> stream completely before configuring the other, otherwise errors may
> cause a complete disconnect just to switch between endpoints. Actually
> I would suggest configuring both endpoint since the beginning so that
> we only need to suspend/resume to switch between them, but I don't
> think many headsets would be able to handle this situation.
>

I think most headsets will not support this because it would require an
extra L2CAP connection to be maintained.

I don't think there is much of a big deal with closing the first
endpoint relationship prior to configuring the next, becuase the AVDTP
signaling channel will keep the underlying ACL connection open. Headsets
I have worked on in the past would reject a SET_CONFIG if an existing
local/remote SEID relationship already existed, not only for the extra
resources required to maintain the extra L2CAP channel, but also because
the SET_CONFIG is when the hw resources are reserved and configured for
the actual PCM data stream.

And assuming that the ACL doesn't get dropped, the critical re-setup
time is probably most bounded by the underlying hw reconfiguration, and
not so much by the few extra small ACL pkts that need to be exchanged.

--
Brian Gix
[email protected]
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

2011-03-15 19:29:34

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: Switching between SBC and MPEG audio on headsets

Hi,

On Tue, Mar 15, 2011 at 2:01 PM, Johan Hedberg <[email protected]> wrote:
> Hi Arun,
>
> On Tue, Mar 15, 2011, Arun Raghavan wrote:
>> I've been trying to set up PulseAudio to be able to switch the A2DP sink
>> dynamically between SBC and MPEG modes. I've got this working now [1], but I
>> did face one problem on the bluez side. When sending a reconfigure request, the
>> request always goes to the SEID that was used previously (=> always to the SBC
>> SEID). Trying to reconfigure for MPEG therefore results the headset returning
>> an error. I'm not very familiar with how this is supposed to work, but the
>> patch following this mail seems to work (I can now switch back and forth
>> between SBC and MPEG modes). It basically forces figuring out what remote SEP
>> to talk to while reconfiguring.
>>
>> Is this the right approach?
>
> I'm not sure about the places where you set the value to NULL, but the
> place in close_cfm which you remove isn't acceptable as such. It was
> originally created to fix the issue reported in this thread:
> http://marc.info/?l=linux-bluetooth&m=129190286303247&w=2
>
> Only a fix which doesn't break the use-case reported there can be
> accepted upstream. FWIW, the commit that introduced the fix is de96fcd8.

Also the solution should consider a transition and not dropping the
stream completely before configuring the other, otherwise errors may
cause a complete disconnect just to switch between endpoints. Actually
I would suggest configuring both endpoint since the beginning so that
we only need to suspend/resume to switch between them, but I don't
think many headsets would be able to handle this situation.

--
Luiz Augusto von Dentz
Computer Engineer

2011-03-15 17:11:51

by Brian Gix

[permalink] [raw]
Subject: Re: [PATCH] Always reset the remote SEP when reconfiguring A2DP

Hi Johan, Arun,

On 3/15/2011 10:08 AM, Johan Hedberg wrote:
> Hi Brian,
>
> On Tue, Mar 15, 2011, Brian Gix wrote:
>> This patch violates the AVDTP specification, as indicated in other
>> email. SEID may not be changed by RECONFIGURE.
>
> Actually this patch doesn't seem to be about AVDTP_Reconfigure but about
> AVDTP_Close + AVDTP_SetConfiguration + AVDTP_Open. So from that
> perspective there's no violation of the spec as far as I can see.
> However, as I mentioned in my other email this will break interop with
> some car kits which expect the new stream to be configured to the same
> SBC SEP in the case that the new codec is SBC. So some checks for
> matching codec needs to be done and only if the new codec is different
> than the old one can we clear setup->rsep.
>
> Johan


Yes, Sorry I didn't look closer at the patch. I saw "Reconfigure" and
my internal alarm bells were going off.

My apologies.

--
Brian Gix
[email protected]
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

2011-03-15 17:08:18

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH] Always reset the remote SEP when reconfiguring A2DP

Hi Brian,

On Tue, Mar 15, 2011, Brian Gix wrote:
> This patch violates the AVDTP specification, as indicated in other
> email. SEID may not be changed by RECONFIGURE.

Actually this patch doesn't seem to be about AVDTP_Reconfigure but about
AVDTP_Close + AVDTP_SetConfiguration + AVDTP_Open. So from that
perspective there's no violation of the spec as far as I can see.
However, as I mentioned in my other email this will break interop with
some car kits which expect the new stream to be configured to the same
SBC SEP in the case that the new codec is SBC. So some checks for
matching codec needs to be done and only if the new codec is different
than the old one can we clear setup->rsep.

Johan

2011-03-15 17:01:24

by Johan Hedberg

[permalink] [raw]
Subject: Re: Switching between SBC and MPEG audio on headsets

Hi Arun,

On Tue, Mar 15, 2011, Arun Raghavan wrote:
> I've been trying to set up PulseAudio to be able to switch the A2DP sink
> dynamically between SBC and MPEG modes. I've got this working now [1], but I
> did face one problem on the bluez side. When sending a reconfigure request, the
> request always goes to the SEID that was used previously (=> always to the SBC
> SEID). Trying to reconfigure for MPEG therefore results the headset returning
> an error. I'm not very familiar with how this is supposed to work, but the
> patch following this mail seems to work (I can now switch back and forth
> between SBC and MPEG modes). It basically forces figuring out what remote SEP
> to talk to while reconfiguring.
>
> Is this the right approach?

I'm not sure about the places where you set the value to NULL, but the
place in close_cfm which you remove isn't acceptable as such. It was
originally created to fix the issue reported in this thread:
http://marc.info/?l=linux-bluetooth&m=129190286303247&w=2

Only a fix which doesn't break the use-case reported there can be
accepted upstream. FWIW, the commit that introduced the fix is de96fcd8.

Johan

2011-03-15 16:30:08

by Brian Gix

[permalink] [raw]
Subject: Re: [PATCH] Always reset the remote SEP when reconfiguring A2DP

This patch violates the AVDTP specification, as indicated in other
email. SEID may not be changed by RECONFIGURE.

On 3/15/2011 7:27 AM, Arun Raghavan wrote:
> This forces the remote SEP to be recalculated when reconfiguring an A2DP
> stream. This is required, for example, when reconfiguring the sink to
> accept MPEG audio instead of SBC (and thus the remote SEID changes).
> ---
> audio/a2dp.c | 5 ++---
> 1 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/audio/a2dp.c b/audio/a2dp.c
> index 3407d6f..27759b8 100644
> --- a/audio/a2dp.c
> +++ b/audio/a2dp.c
> @@ -1101,9 +1101,6 @@ static void close_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
> return;
> }
>
> - if (!setup->rsep)
> - setup->rsep = avdtp_stream_get_remote_sep(stream);
> -
> if (setup->reconfigure)
> g_timeout_add(RECONFIGURE_TIMEOUT, a2dp_reconfigure, setup);
> }
> @@ -2048,6 +2045,7 @@ unsigned int a2dp_config(struct avdtp *session, struct a2dp_sep *sep,
> if (a2dp_sep_get_lock(tmp))
> goto failed;
> setup->reconfigure = TRUE;
> + setup->rsep = NULL;
> if (avdtp_close(session, tmp->stream, FALSE)< 0) {
> error("avdtp_close failed");
> goto failed;
> @@ -2077,6 +2075,7 @@ unsigned int a2dp_config(struct avdtp *session, struct a2dp_sep *sep,
> g_idle_add((GSourceFunc) finalize_config, setup);
> } else if (!setup->reconfigure) {
> setup->reconfigure = TRUE;
> + setup->rsep = NULL;
> if (avdtp_close(session, sep->stream, FALSE)< 0) {
> error("avdtp_close failed");
> goto failed;


--
Brian Gix
[email protected]
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

2011-03-15 16:22:15

by Brian Gix

[permalink] [raw]
Subject: Re: Switching between SBC and MPEG audio on headsets

Hi Arun,

On 3/15/2011 7:27 AM, Arun Raghavan wrote:
> Hello,
> I've been trying to set up PulseAudio to be able to switch the A2DP sink
> dynamically between SBC and MPEG modes. I've got this working now [1], but I
> did face one problem on the bluez side. When sending a reconfigure request, the
> request always goes to the SEID that was used previously (=> always to the SBC
> SEID). Trying to reconfigure for MPEG therefore results the headset returning
> an error. I'm not very familiar with how this is supposed to work, but the
> patch following this mail seems to work (I can now switch back and forth
> between SBC and MPEG modes). It basically forces figuring out what remote SEP
> to talk to while reconfiguring.
>
> Is this the right approach?

No this is the incorrect approach.

Both SRC and SNK devices have a variety of endpoints that have
capabilities describing what that endpoint is capable of, the most
important of which is the Codec Capability. Each endpoint is allowed a
single capability of a particular type, which means that an endpoint
that supports SBC should not attempt to use a different codec (for MPEG
in this case). Any device that attempts to wedge both codecs into a
single endpoint would most definitely be non-compliant.

The SUSPEND/RECONFIGURE/START procedure will not work to switch between
SBC and MPEG for this reason. The purpose for that procedure is to
allow quick changes within the bounds of the current (remote and local)
endpoint capabilities. To quickly switch between a 44.1KHz and 48KHz
sampling for instance.

The correct procedure to switch between SBC and MPEG would be to close
the existing media channel (but NOT the AVDTP signaling channel) and
reset up a new endpoint. If you know you will be doing this, you can
save time by caching all of the remote devices endpoints the first time
you do an AVDTP_DISCOVER, so that you have all the remote endpoint info
you need to make the switch (and even know if the switch will work). You
also should have all supported local endpoints separated into their own
endpoints of course.

The to switch, you will need to:

<halt streaming>

AVDTP_CLOSE (on AVDTP signaling channel)

L2CAP_CLOSE (close the media L2CAP channel for SBC)

<<<< You should Rx AVDTP_CLOSE_CFM

AVDTP_SET_CONFIG (on AVDTP signaling channel,
specify a local and remote endpoint that are compatible)

<<<<< You should Rx SET_CONFIG_CFM

AVDTP_OPEN (on AVDTP signaling channel)

L2CAP_OPEN (open new media L2CAP channel for MPEG)

<<<<< You should Rx AVDTP_OPEN_CFM

AVDTP_START (on AVDTP signaling channel)

<<<<< You should Rx AVDTP_START_CFM

<start streaming>



Regards,


--
Brian Gix
[email protected]
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

2011-03-15 14:27:41

by Arun Raghavan

[permalink] [raw]
Subject: [PATCH] Always reset the remote SEP when reconfiguring A2DP

This forces the remote SEP to be recalculated when reconfiguring an A2DP
stream. This is required, for example, when reconfiguring the sink to
accept MPEG audio instead of SBC (and thus the remote SEID changes).
---
audio/a2dp.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/audio/a2dp.c b/audio/a2dp.c
index 3407d6f..27759b8 100644
--- a/audio/a2dp.c
+++ b/audio/a2dp.c
@@ -1101,9 +1101,6 @@ static void close_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
return;
}

- if (!setup->rsep)
- setup->rsep = avdtp_stream_get_remote_sep(stream);
-
if (setup->reconfigure)
g_timeout_add(RECONFIGURE_TIMEOUT, a2dp_reconfigure, setup);
}
@@ -2048,6 +2045,7 @@ unsigned int a2dp_config(struct avdtp *session, struct a2dp_sep *sep,
if (a2dp_sep_get_lock(tmp))
goto failed;
setup->reconfigure = TRUE;
+ setup->rsep = NULL;
if (avdtp_close(session, tmp->stream, FALSE) < 0) {
error("avdtp_close failed");
goto failed;
@@ -2077,6 +2075,7 @@ unsigned int a2dp_config(struct avdtp *session, struct a2dp_sep *sep,
g_idle_add((GSourceFunc) finalize_config, setup);
} else if (!setup->reconfigure) {
setup->reconfigure = TRUE;
+ setup->rsep = NULL;
if (avdtp_close(session, sep->stream, FALSE) < 0) {
error("avdtp_close failed");
goto failed;
--
1.7.4.1