2008-12-15 23:14:43

by event

[permalink] [raw]
Subject: [PATCH] Gateway profile

Hi all,

I've implemented gateway profile. I've tested basic things, like
place/cancel/answer call. Others are in development. Some could not be
tested as my carrier doesn't provide corresponding services (like
3-way call, etc.) so any help welcome.

Vale,
Leonid Movshovich


Attachments:
(No filename) (275.00 B)
gateway.patch (47.44 kB)
Download all attachments

2008-12-19 07:36:33

by event

[permalink] [raw]
Subject: Re: [PATCH] Gateway profile

Hello Marcel,

Thank you for your comments. Here are reworked fixed patches attached.
I've missed when you changed repo :).

I've noticed IPC infrastructure and going to make use of it in future version.

I fully agree for code style. Sorry for that. Was too stupid to setup
my editor correctly :).

Vale,
event



On Wed, Dec 17, 2008 at 04:48, Marcel Holtmann <[email protected]> wrote:
> Hi Leonid,
>
>> I've implemented gateway profile. I've tested basic things, like
>> place/cancel/answer call. Others are in development. Some could not be
>> tested as my carrier doesn't provide corresponding services (like
>> 3-way call, etc.) so any help welcome.
>
> thanks for the works, but can you please base the patch against the
> latest GIT tree. It is kinda hard to review things that might already
> have been implemented like sco_listen.
>
> audio/audio-api.txt | 94 +++++
> audio/device.h | 7
> audio/gateway.c | 938 +++++++++++++++++++++++++++++++++++++++++++++++++++
> audio/gateway.h | 11
> audio/manager.c | 124 ++++--
> common/glib-helper.c | 85 +++-
> common/glib-helper.h | 1
> 7 files changed, 1205 insertions(+), 55 deletions(-)
>
> So any changes to glib-helper.[ch] have to be in a separate patch and
> need to be discussed independent from the gateway implementation.
>
> Any audio-api.txt stuff should also go separately since that has to be
> discussed. Also we can't send PCM data over D-Bus. It just doesn't work
> like that. We do have the internal IPC for that and plugins for ALSA,
> GStreamer and PulseAudio that should be used.
>
> However the most important part is that you follow the coding style and
> that is the kernel coding style. You make it really hard for us to
> review the code like this and it can't be applied. I really want you to
> add support for the gateway role to BlueZ, but the overall code in the
> project needs to follow the same rules.
>
> So please fix these issues first and then we do a deep review of it.
>
> Regards
>
> Marcel
>
>
>


Attachments:
(No filename) (1.99 kB)
gateway.patch (38.02 kB)
audio-api.patch (3.63 kB)
Download all attachments

2008-12-17 02:48:53

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] Gateway profile

Hi Leonid,

> I've implemented gateway profile. I've tested basic things, like
> place/cancel/answer call. Others are in development. Some could not be
> tested as my carrier doesn't provide corresponding services (like
> 3-way call, etc.) so any help welcome.

thanks for the works, but can you please base the patch against the
latest GIT tree. It is kinda hard to review things that might already
have been implemented like sco_listen.

audio/audio-api.txt | 94 +++++
audio/device.h | 7
audio/gateway.c | 938 +++++++++++++++++++++++++++++++++++++++++++++++++++
audio/gateway.h | 11
audio/manager.c | 124 ++++--
common/glib-helper.c | 85 +++-
common/glib-helper.h | 1
7 files changed, 1205 insertions(+), 55 deletions(-)

So any changes to glib-helper.[ch] have to be in a separate patch and
need to be discussed independent from the gateway implementation.

Any audio-api.txt stuff should also go separately since that has to be
discussed. Also we can't send PCM data over D-Bus. It just doesn't work
like that. We do have the internal IPC for that and plugins for ALSA,
GStreamer and PulseAudio that should be used.

However the most important part is that you follow the coding style and
that is the kernel coding style. You make it really hard for us to
review the code like this and it can't be applied. I really want you to
add support for the gateway role to BlueZ, but the overall code in the
project needs to follow the same rules.

So please fix these issues first and then we do a deep review of it.

Regards

Marcel



2009-01-19 10:11:22

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] Gateway profile

Hi,

> >> >> >> I've implemented gateway profile. I've tested basic things, like
> >> >> >> place/cancel/answer call. Others are in development. Some could not be
> >> >> >> tested as my carrier doesn't provide corresponding services (like
> >> >> >> 3-way call, etc.) so any help welcome.
> >> >> >
> >> >> > thanks for the works, but can you please base the patch against the
> >> >> > latest GIT tree. It is kinda hard to review things that might already
> >> >> > have been implemented like sco_listen.
> >> >> >
> >> >> > audio/audio-api.txt | 94 +++++
> >> >> > audio/device.h | 7
> >> >> > audio/gateway.c | 938 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >> >> > audio/gateway.h | 11
> >> >> > audio/manager.c | 124 ++++--
> >> >> > common/glib-helper.c | 85 +++-
> >> >> > common/glib-helper.h | 1
> >> >> > 7 files changed, 1205 insertions(+), 55 deletions(-)
> >> >> >
> >> >> > So any changes to glib-helper.[ch] have to be in a separate patch and
> >> >> > need to be discussed independent from the gateway implementation.
> >> >> >
> >> >> > Any audio-api.txt stuff should also go separately since that has to be
> >> >> > discussed. Also we can't send PCM data over D-Bus. It just doesn't work
> >> >> > like that. We do have the internal IPC for that and plugins for ALSA,
> >> >> > GStreamer and PulseAudio that should be used.
> >> >> >
> >> >> > However the most important part is that you follow the coding style and
> >> >> > that is the kernel coding style. You make it really hard for us to
> >> >> > review the code like this and it can't be applied. I really want you to
> >> >> > add support for the gateway role to BlueZ, but the overall code in the
> >> >> > project needs to follow the same rules.
> >> >> >
> >> >> > So please fix these issues first and then we do a deep review of it.
> >> >>
> >> >> Here is reworked and improved patches as you suggested with IPC support.
> >> >>
> >> >> But I have some doubts and questions:
> >> >> 1. to distinguish between headset and gateway I've added one more alsa
> >> >> config option "role" which could be master (for gateway and probably
> >> >> a2dp source) or slave (which is default and works for headset and a2dp
> >> >> sink). I don't really like this approach so if you have any other idea
> >> >> it would be great.
> >> >
> >> > we should use the terms "headset", "gateway", "sink" and "source" as
> >> > these are used through the specs.
> >> >
> >>
> >> But current configuration contains "type" which is either voice or
> >> hifi. So should I set 4 mentioned above for type field? I mean this
> >> will break backward compatibility.
> >
> > you could actually and still have "sink" == "hifi" and then also
> > "headset" == "voice" for example.
> >
> >> >> 2. my cell phone closes SCO connection when it doesn't need one,
> >> >> probably others act like this as well. SCO close results in
> >> >> bluetooth_hsp_write returning an error. What would be the best way to
> >> >> overcome this?
> >> >
> >> > No idea what's the problem here. You should already get a notice of the
> >> > IPC that the channel is closed. On closed channels we have to discard
> >> > any kind of PCM data from the PC.
> >> >
> >>
> >> That is how it should work with current implementation but it would
> >> not be very nice for application developers as e.g. pause of the
> >> player on the phone will result in snd_pcm_read (or how is it named)
> >> returning an error. I've tested using pyalsaaudio which raises an
> >> exception in this case.
> >> If I would develop an application over such api I would say several bad words :)
> >
> > Obviously the ALSA plugin (or GStreamer or PluseAudio for that matter)
> > need to hide that fact and make it return a proper value instead of an
> > error. While for playback we can just discard the audio data, for
> > capture we might have to produce silence.
> >
> >> >> 3. I've noticed that ipc interface duplicate dbus interface to some
> >> >> extent. Why can't pcm_bluetooth work over dbus directly?
> >> >
> >> > D-Bus can't handle massive amount of PCM data payload. Also the ALSA
> >> > plugins don't really like dealing with a D-Bus mainloop. Hence we do
> >> > have the IPC as an alternate way of dealing with audio. We don't like to
> >> > do it, but we have to.
> >> >
> >>
> >> You can add one more DBus call which will create socket and send audio
> >> connection over it.
> >
> > It just doesn't work that way. You will be killing your performance and
> > creating memory pressure. Especially on small and embedded systems. Also
> > the latency is pretty bad. Trust me here.
>
> I didn't meant to send audio data over dbus. My idea was like this:
>
> +-------+ +------+
> | bluez | | alsa |
> +-------+ +------+
> | get unix socket |
> | <-----------------|
> create |
> unix socket |
> | |
> | send unix |
> | socket name |
> |------------------>|
> | listen
> | for fd
> | |
> | send sco fd |
> |------------------>|
>
> first call is over Dbus and socket is sent over domain socket.

then you still have to handle the integration of D-Bus mainloop (or call
it message parsing/handling) into an ALSA plugin. ALSA is just a total
broken concept when it comes to virtual sound cards. It works great for
actual physical devices, but that is it.

Regards

Marcel



2009-01-19 10:02:11

by event

[permalink] [raw]
Subject: Re: [PATCH] Gateway profile

On Mon, Jan 19, 2009 at 11:20, Marcel Holtmann <[email protected]> wrote:
> Hi,
>
>> >> >> I've implemented gateway profile. I've tested basic things, like
>> >> >> place/cancel/answer call. Others are in development. Some could not be
>> >> >> tested as my carrier doesn't provide corresponding services (like
>> >> >> 3-way call, etc.) so any help welcome.
>> >> >
>> >> > thanks for the works, but can you please base the patch against the
>> >> > latest GIT tree. It is kinda hard to review things that might already
>> >> > have been implemented like sco_listen.
>> >> >
>> >> > audio/audio-api.txt | 94 +++++
>> >> > audio/device.h | 7
>> >> > audio/gateway.c | 938 +++++++++++++++++++++++++++++++++++++++++++++++++++
>> >> > audio/gateway.h | 11
>> >> > audio/manager.c | 124 ++++--
>> >> > common/glib-helper.c | 85 +++-
>> >> > common/glib-helper.h | 1
>> >> > 7 files changed, 1205 insertions(+), 55 deletions(-)
>> >> >
>> >> > So any changes to glib-helper.[ch] have to be in a separate patch and
>> >> > need to be discussed independent from the gateway implementation.
>> >> >
>> >> > Any audio-api.txt stuff should also go separately since that has to be
>> >> > discussed. Also we can't send PCM data over D-Bus. It just doesn't work
>> >> > like that. We do have the internal IPC for that and plugins for ALSA,
>> >> > GStreamer and PulseAudio that should be used.
>> >> >
>> >> > However the most important part is that you follow the coding style and
>> >> > that is the kernel coding style. You make it really hard for us to
>> >> > review the code like this and it can't be applied. I really want you to
>> >> > add support for the gateway role to BlueZ, but the overall code in the
>> >> > project needs to follow the same rules.
>> >> >
>> >> > So please fix these issues first and then we do a deep review of it.
>> >>
>> >> Here is reworked and improved patches as you suggested with IPC support.
>> >>
>> >> But I have some doubts and questions:
>> >> 1. to distinguish between headset and gateway I've added one more alsa
>> >> config option "role" which could be master (for gateway and probably
>> >> a2dp source) or slave (which is default and works for headset and a2dp
>> >> sink). I don't really like this approach so if you have any other idea
>> >> it would be great.
>> >
>> > we should use the terms "headset", "gateway", "sink" and "source" as
>> > these are used through the specs.
>> >
>>
>> But current configuration contains "type" which is either voice or
>> hifi. So should I set 4 mentioned above for type field? I mean this
>> will break backward compatibility.
>
> you could actually and still have "sink" == "hifi" and then also
> "headset" == "voice" for example.
>
>> >> 2. my cell phone closes SCO connection when it doesn't need one,
>> >> probably others act like this as well. SCO close results in
>> >> bluetooth_hsp_write returning an error. What would be the best way to
>> >> overcome this?
>> >
>> > No idea what's the problem here. You should already get a notice of the
>> > IPC that the channel is closed. On closed channels we have to discard
>> > any kind of PCM data from the PC.
>> >
>>
>> That is how it should work with current implementation but it would
>> not be very nice for application developers as e.g. pause of the
>> player on the phone will result in snd_pcm_read (or how is it named)
>> returning an error. I've tested using pyalsaaudio which raises an
>> exception in this case.
>> If I would develop an application over such api I would say several bad words :)
>
> Obviously the ALSA plugin (or GStreamer or PluseAudio for that matter)
> need to hide that fact and make it return a proper value instead of an
> error. While for playback we can just discard the audio data, for
> capture we might have to produce silence.
>
>> >> 3. I've noticed that ipc interface duplicate dbus interface to some
>> >> extent. Why can't pcm_bluetooth work over dbus directly?
>> >
>> > D-Bus can't handle massive amount of PCM data payload. Also the ALSA
>> > plugins don't really like dealing with a D-Bus mainloop. Hence we do
>> > have the IPC as an alternate way of dealing with audio. We don't like to
>> > do it, but we have to.
>> >
>>
>> You can add one more DBus call which will create socket and send audio
>> connection over it.
>
> It just doesn't work that way. You will be killing your performance and
> creating memory pressure. Especially on small and embedded systems. Also
> the latency is pretty bad. Trust me here.

I didn't meant to send audio data over dbus. My idea was like this:

+-------+ +------+
| bluez | | alsa |
+-------+ +------+
| get unix socket |
| <-----------------|
create |
unix socket |
| |
| send unix |
| socket name |
|------------------>|
| listen
| for fd
| |
| send sco fd |
|------------------>|

first call is over Dbus and socket is sent over domain socket.
>
> Regards
>
> Marcel
>
>
>


Vale,
Leonid Movshovich

2009-01-19 09:20:57

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] Gateway profile

Hi,

> >> >> I've implemented gateway profile. I've tested basic things, like
> >> >> place/cancel/answer call. Others are in development. Some could not be
> >> >> tested as my carrier doesn't provide corresponding services (like
> >> >> 3-way call, etc.) so any help welcome.
> >> >
> >> > thanks for the works, but can you please base the patch against the
> >> > latest GIT tree. It is kinda hard to review things that might already
> >> > have been implemented like sco_listen.
> >> >
> >> > audio/audio-api.txt | 94 +++++
> >> > audio/device.h | 7
> >> > audio/gateway.c | 938 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >> > audio/gateway.h | 11
> >> > audio/manager.c | 124 ++++--
> >> > common/glib-helper.c | 85 +++-
> >> > common/glib-helper.h | 1
> >> > 7 files changed, 1205 insertions(+), 55 deletions(-)
> >> >
> >> > So any changes to glib-helper.[ch] have to be in a separate patch and
> >> > need to be discussed independent from the gateway implementation.
> >> >
> >> > Any audio-api.txt stuff should also go separately since that has to be
> >> > discussed. Also we can't send PCM data over D-Bus. It just doesn't work
> >> > like that. We do have the internal IPC for that and plugins for ALSA,
> >> > GStreamer and PulseAudio that should be used.
> >> >
> >> > However the most important part is that you follow the coding style and
> >> > that is the kernel coding style. You make it really hard for us to
> >> > review the code like this and it can't be applied. I really want you to
> >> > add support for the gateway role to BlueZ, but the overall code in the
> >> > project needs to follow the same rules.
> >> >
> >> > So please fix these issues first and then we do a deep review of it.
> >>
> >> Here is reworked and improved patches as you suggested with IPC support.
> >>
> >> But I have some doubts and questions:
> >> 1. to distinguish between headset and gateway I've added one more alsa
> >> config option "role" which could be master (for gateway and probably
> >> a2dp source) or slave (which is default and works for headset and a2dp
> >> sink). I don't really like this approach so if you have any other idea
> >> it would be great.
> >
> > we should use the terms "headset", "gateway", "sink" and "source" as
> > these are used through the specs.
> >
>
> But current configuration contains "type" which is either voice or
> hifi. So should I set 4 mentioned above for type field? I mean this
> will break backward compatibility.

you could actually and still have "sink" == "hifi" and then also
"headset" == "voice" for example.

> >> 2. my cell phone closes SCO connection when it doesn't need one,
> >> probably others act like this as well. SCO close results in
> >> bluetooth_hsp_write returning an error. What would be the best way to
> >> overcome this?
> >
> > No idea what's the problem here. You should already get a notice of the
> > IPC that the channel is closed. On closed channels we have to discard
> > any kind of PCM data from the PC.
> >
>
> That is how it should work with current implementation but it would
> not be very nice for application developers as e.g. pause of the
> player on the phone will result in snd_pcm_read (or how is it named)
> returning an error. I've tested using pyalsaaudio which raises an
> exception in this case.
> If I would develop an application over such api I would say several bad words :)

Obviously the ALSA plugin (or GStreamer or PluseAudio for that matter)
need to hide that fact and make it return a proper value instead of an
error. While for playback we can just discard the audio data, for
capture we might have to produce silence.

> >> 3. I've noticed that ipc interface duplicate dbus interface to some
> >> extent. Why can't pcm_bluetooth work over dbus directly?
> >
> > D-Bus can't handle massive amount of PCM data payload. Also the ALSA
> > plugins don't really like dealing with a D-Bus mainloop. Hence we do
> > have the IPC as an alternate way of dealing with audio. We don't like to
> > do it, but we have to.
> >
>
> You can add one more DBus call which will create socket and send audio
> connection over it.

It just doesn't work that way. You will be killing your performance and
creating memory pressure. Especially on small and embedded systems. Also
the latency is pretty bad. Trust me here.

Regards

Marcel



2009-01-19 08:37:05

by event

[permalink] [raw]
Subject: Re: [PATCH] Gateway profile

On Sun, Jan 18, 2009 at 18:07, Marcel Holtmann <[email protected]> wrote:
> Hi,
>
>> >> I've implemented gateway profile. I've tested basic things, like
>> >> place/cancel/answer call. Others are in development. Some could not be
>> >> tested as my carrier doesn't provide corresponding services (like
>> >> 3-way call, etc.) so any help welcome.
>> >
>> > thanks for the works, but can you please base the patch against the
>> > latest GIT tree. It is kinda hard to review things that might already
>> > have been implemented like sco_listen.
>> >
>> > audio/audio-api.txt | 94 +++++
>> > audio/device.h | 7
>> > audio/gateway.c | 938 +++++++++++++++++++++++++++++++++++++++++++++++++++
>> > audio/gateway.h | 11
>> > audio/manager.c | 124 ++++--
>> > common/glib-helper.c | 85 +++-
>> > common/glib-helper.h | 1
>> > 7 files changed, 1205 insertions(+), 55 deletions(-)
>> >
>> > So any changes to glib-helper.[ch] have to be in a separate patch and
>> > need to be discussed independent from the gateway implementation.
>> >
>> > Any audio-api.txt stuff should also go separately since that has to be
>> > discussed. Also we can't send PCM data over D-Bus. It just doesn't work
>> > like that. We do have the internal IPC for that and plugins for ALSA,
>> > GStreamer and PulseAudio that should be used.
>> >
>> > However the most important part is that you follow the coding style and
>> > that is the kernel coding style. You make it really hard for us to
>> > review the code like this and it can't be applied. I really want you to
>> > add support for the gateway role to BlueZ, but the overall code in the
>> > project needs to follow the same rules.
>> >
>> > So please fix these issues first and then we do a deep review of it.
>>
>> Here is reworked and improved patches as you suggested with IPC support.
>>
>> But I have some doubts and questions:
>> 1. to distinguish between headset and gateway I've added one more alsa
>> config option "role" which could be master (for gateway and probably
>> a2dp source) or slave (which is default and works for headset and a2dp
>> sink). I don't really like this approach so if you have any other idea
>> it would be great.
>
> we should use the terms "headset", "gateway", "sink" and "source" as
> these are used through the specs.
>

But current configuration contains "type" which is either voice or
hifi. So should I set 4 mentioned above for type field? I mean this
will break backward compatibility.

>> 2. my cell phone closes SCO connection when it doesn't need one,
>> probably others act like this as well. SCO close results in
>> bluetooth_hsp_write returning an error. What would be the best way to
>> overcome this?
>
> No idea what's the problem here. You should already get a notice of the
> IPC that the channel is closed. On closed channels we have to discard
> any kind of PCM data from the PC.
>

That is how it should work with current implementation but it would
not be very nice for application developers as e.g. pause of the
player on the phone will result in snd_pcm_read (or how is it named)
returning an error. I've tested using pyalsaaudio which raises an
exception in this case.
If I would develop an application over such api I would say several bad words :)

>> 3. I've noticed that ipc interface duplicate dbus interface to some
>> extent. Why can't pcm_bluetooth work over dbus directly?
>
> D-Bus can't handle massive amount of PCM data payload. Also the ALSA
> plugins don't really like dealing with a D-Bus mainloop. Hence we do
> have the IPC as an alternate way of dealing with audio. We don't like to
> do it, but we have to.
>

You can add one more DBus call which will create socket and send audio
connection over it.

>> 4. Aren't there any legal issues with implementing bluez? As I noticed
>> in the spec that all the rights belong to Bluetooth SIG so it does
>> look neither "free as in free beer" nor "free as in freedom" :)
>
> BlueZ is a fully qualified host stack that complies with the SIG's
> requirements. We do have to update the listing with a headset gateway
> role at some point, but only once we have it fully working and actually
> used in a product.
>
> Regards
>
> Marcel
>
>
>


Vale,
Leonid Movshovich

2009-01-18 16:07:08

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] Gateway profile

Hi,

> >> I've implemented gateway profile. I've tested basic things, like
> >> place/cancel/answer call. Others are in development. Some could not be
> >> tested as my carrier doesn't provide corresponding services (like
> >> 3-way call, etc.) so any help welcome.
> >
> > thanks for the works, but can you please base the patch against the
> > latest GIT tree. It is kinda hard to review things that might already
> > have been implemented like sco_listen.
> >
> > audio/audio-api.txt | 94 +++++
> > audio/device.h | 7
> > audio/gateway.c | 938 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > audio/gateway.h | 11
> > audio/manager.c | 124 ++++--
> > common/glib-helper.c | 85 +++-
> > common/glib-helper.h | 1
> > 7 files changed, 1205 insertions(+), 55 deletions(-)
> >
> > So any changes to glib-helper.[ch] have to be in a separate patch and
> > need to be discussed independent from the gateway implementation.
> >
> > Any audio-api.txt stuff should also go separately since that has to be
> > discussed. Also we can't send PCM data over D-Bus. It just doesn't work
> > like that. We do have the internal IPC for that and plugins for ALSA,
> > GStreamer and PulseAudio that should be used.
> >
> > However the most important part is that you follow the coding style and
> > that is the kernel coding style. You make it really hard for us to
> > review the code like this and it can't be applied. I really want you to
> > add support for the gateway role to BlueZ, but the overall code in the
> > project needs to follow the same rules.
> >
> > So please fix these issues first and then we do a deep review of it.
>
> Here is reworked and improved patches as you suggested with IPC support.
>
> But I have some doubts and questions:
> 1. to distinguish between headset and gateway I've added one more alsa
> config option "role" which could be master (for gateway and probably
> a2dp source) or slave (which is default and works for headset and a2dp
> sink). I don't really like this approach so if you have any other idea
> it would be great.

we should use the terms "headset", "gateway", "sink" and "source" as
these are used through the specs.

> 2. my cell phone closes SCO connection when it doesn't need one,
> probably others act like this as well. SCO close results in
> bluetooth_hsp_write returning an error. What would be the best way to
> overcome this?

No idea what's the problem here. You should already get a notice of the
IPC that the channel is closed. On closed channels we have to discard
any kind of PCM data from the PC.

> 3. I've noticed that ipc interface duplicate dbus interface to some
> extent. Why can't pcm_bluetooth work over dbus directly?

D-Bus can't handle massive amount of PCM data payload. Also the ALSA
plugins don't really like dealing with a D-Bus mainloop. Hence we do
have the IPC as an alternate way of dealing with audio. We don't like to
do it, but we have to.

> 4. Aren't there any legal issues with implementing bluez? As I noticed
> in the spec that all the rights belong to Bluetooth SIG so it does
> look neither "free as in free beer" nor "free as in freedom" :)

BlueZ is a fully qualified host stack that complies with the SIG's
requirements. We do have to update the listing with a headset gateway
role at some point, but only once we have it fully working and actually
used in a product.

Regards

Marcel



2009-01-16 13:22:42

by event

[permalink] [raw]
Subject: Re: [PATCH] Gateway profile

On Wed, Dec 17, 2008 at 04:48, Marcel Holtmann <[email protected]> wrote:
> Hi Leonid,
>
>> I've implemented gateway profile. I've tested basic things, like
>> place/cancel/answer call. Others are in development. Some could not be
>> tested as my carrier doesn't provide corresponding services (like
>> 3-way call, etc.) so any help welcome.
>
> thanks for the works, but can you please base the patch against the
> latest GIT tree. It is kinda hard to review things that might already
> have been implemented like sco_listen.
>
> audio/audio-api.txt | 94 +++++
> audio/device.h | 7
> audio/gateway.c | 938 +++++++++++++++++++++++++++++++++++++++++++++++++++
> audio/gateway.h | 11
> audio/manager.c | 124 ++++--
> common/glib-helper.c | 85 +++-
> common/glib-helper.h | 1
> 7 files changed, 1205 insertions(+), 55 deletions(-)
>
> So any changes to glib-helper.[ch] have to be in a separate patch and
> need to be discussed independent from the gateway implementation.
>
> Any audio-api.txt stuff should also go separately since that has to be
> discussed. Also we can't send PCM data over D-Bus. It just doesn't work
> like that. We do have the internal IPC for that and plugins for ALSA,
> GStreamer and PulseAudio that should be used.
>
> However the most important part is that you follow the coding style and
> that is the kernel coding style. You make it really hard for us to
> review the code like this and it can't be applied. I really want you to
> add support for the gateway role to BlueZ, but the overall code in the
> project needs to follow the same rules.
>
> So please fix these issues first and then we do a deep review of it.
>
> Regards
>
> Marcel
>
>
>

Hello Marcel,

Here is reworked and improved patches as you suggested with IPC support.

But I have some doubts and questions:
1. to distinguish between headset and gateway I've added one more alsa
config option "role" which could be master (for gateway and probably
a2dp source) or slave (which is default and works for headset and a2dp
sink). I don't really like this approach so if you have any other idea
it would be great.
2. my cell phone closes SCO connection when it doesn't need one,
probably others act like this as well. SCO close results in
bluetooth_hsp_write returning an error. What would be the best way to
overcome this?
3. I've noticed that ipc interface duplicate dbus interface to some
extent. Why can't pcm_bluetooth work over dbus directly?
4. Aren't there any legal issues with implementing bluez? As I noticed
in the spec that all the rights belong to Bluetooth SIG so it does
look neither "free as in free beer" nor "free as in freedom" :)

Vale,
Leonid Movshovich


Attachments:
(No filename) (2.65 kB)
audio_api.txt.patch (3.30 kB)
gateway_profile.patch (52.03 kB)
Download all attachments

2009-02-06 12:08:15

by Thierry Pierret

[permalink] [raw]
Subject: Re: [PATCH] Gateway profile

Hi Leonid, hi to all,

I know this patch is not perfect and still has some issues. But since
there is currently no alternative, I would like to use it on my iMX31
platform. And if afterwards I can help with some improvements, it's
worthy.
In the meantime, I have to dive in your code, the bluez code and in the
buetooth specifications. Since I'm not an expert, neither for bluetooth
and bluez, neither for sound, I would earn some time if I could get some
answers already.

I installed the "Gateway Profile" patch on BlueZ 4.28 (with some minor
corrections for the compilation).

1. First issue, only the "Handsfree" service is exported :

$ sdptool browse local
Browsing FF:FF:FF:00:00:00 ...
Service Name: Hands-free
Service RecHandle: 0x10000
Service Class ID List:
"Handsfree" (0x111e)
"Generic Audio" (0x1203)
Protocol Descriptor List:
"L2CAP" (0x0100)
"RFCOMM" (0x0003)
Channel: 6
Profile Descriptor List:
"Handsfree" (0x111e)
Version: 0x0100

Here is the audio.conf file I use :

[General]
Master=true
Enable=Gateway
Disable=Headset;Sink;Source;Control
SCORouting=PCM

[Headset]
HFP=true
MaxConnected=1

I certainly miss something in my configuration in order to also get the
Headset service exported.

2. Second issue, any attempt to connect the handsfree service from a
mobile failed. The mobile ends in the paired state, but the handsfree
device connection failed. Here is the full (annotated) ouput of the
bluetoothd daemon.

bluetoothd[1772]: Bluetooth daemon
bluetoothd[1772]: Enabling debug information
bluetoothd[1772]: parsing main.conf
bluetoothd[1772]: discovto=0
bluetoothd[1772]: pairto=0
bluetoothd[1772]: pageto=8192
bluetoothd[1772]: name=%h-%d
bluetoothd[1772]: class=0x000100
bluetoothd[1772]: inqmode=0
bluetoothd[1772]: Key file does not have key 'DeviceID'
bluetoothd[1772]: Starting SDP server
bluetoothd[1772]: Loading plugins /usr/lib/bluetooth/plugins
bluetoothd[1772]: register_interface: path /org/bluez/1772/any
bluetoothd[1772]: Registered interface org.bluez.Service on path
/org/bluez/1772/any
bluetoothd[1772]: Unix socket created: 10
bluetoothd[1772]: HCI dev 0 registered
bluetoothd[1772]: child 1773 forked
bluetoothd[1772]: HCI dev 0 already up
bluetoothd[1772]: Starting security manager 0
bluetoothd[1772]: register_interface: path /org/bluez/1772/hci0
bluetoothd[1772]: Registered interface org.bluez.Service on path
/org/bluez/1772/hci0
bluetoothd[1772]: gateway_server_probe: path /org/bluez/1772/hci0
bluetoothd[1772]: Adding record with handle 0x10000
bluetoothd[1772]: Record pattern UUID 00000003-0000-1000-8000-00805f9
bluetoothd[1772]: Record pattern UUID 00000100-0000-1000-8000-00805f9
bluetoothd[1772]: Record pattern UUID 00001002-0000-1000-8000-00805f9
bluetoothd[1772]: Record pattern UUID 0000111e-0000-1000-8000-00805f9
bluetoothd[1772]: Record pattern UUID 00001203-0000-1000-8000-00805f9
bluetoothd[1772]: proxy_probe: path /org/bluez/1772/hci0
bluetoothd[1772]: Registered interface org.bluez.SerialProxyManager on
path /org/bluez/1772/hci0
bluetoothd[1772]: Creating device
/org/bluez/1772/hci0/dev_00_0E_ED_01_DA_66
bluetoothd[1772]: Probe drivers for
/org/bluez/1772/hci0/dev_00_0E_ED_01_DA_66
bluetoothd[1772]: adapter_get_device(00:0E:ED:01:DA:66)
bluetoothd[1772]: audio handle_uuid: server not enabled for
00001112-0000-1000-8000-00805F9B34FB (0x1112)
bluetoothd[1772]: Found Handsfree AG record
bluetoothd[1772]: serial_probe: path
/org/bluez/1772/hci0/dev_00_0E_ED_01_DA_66:
00000002-0000-1000-8000-0002ee000002
bluetoothd[1772]: Registered interface org.bluez.Serial on path
/org/bluez/1772/hci0/dev_00_0E_ED_01_DA_66
bluetoothd[1772]: serial_probe: path
/org/bluez/1772/hci0/dev_00_0E_ED_01_DA_66:
00001101-0000-1000-8000-00805f9b34fb
bluetoothd[1772]: serial_probe: path
/org/bluez/1772/hci0/dev_00_0E_ED_01_DA_66:
00001103-0000-1000-8000-00805f9b34fb
bluetoothd[1772]: serial_probe: path
/org/bluez/1772/hci0/dev_00_0E_ED_01_DA_66:
00001105-0000-1000-8000-00805f9b34fb
bluetoothd[1772]: serial_probe: path
/org/bluez/1772/hci0/dev_00_0E_ED_01_DA_66:
00001106-0000-1000-8000-00805f9b34fb
bluetoothd[1772]: serial_probe: path
/org/bluez/1772/hci0/dev_00_0E_ED_01_DA_66:
00001112-0000-1000-8000-00805F9B34FB
bluetoothd[1772]: serial_probe: path
/org/bluez/1772/hci0/dev_00_0E_ED_01_DA_66:
0000111f-0000-1000-8000-00805f9b34fb
bluetoothd[1772]: serial_probe: path
/org/bluez/1772/hci0/dev_00_0E_ED_01_DA_66:
0000112d-0000-1000-8000-00805f9b34fb
bluetoothd[1772]: serial_probe: path
/org/bluez/1772/hci0/dev_00_0E_ED_01_DA_66:
00005002-0000-1000-8000-0002ee000001
bluetoothd[1772]: Adapter /org/bluez/1772/hci0 has been enabled
bluetoothd[1772]: child 1773 exited

# Passkey agent started
bluetoothd[1772]: Agent registered for hci0 at :1.4:/org/bluez/agent_1782

# Attempt from a mobile (Nokia 6810) to connect the handsfree device
bluetoothd[1772]: adapter_get_device(00:0E:ED:01:DA:66)
bluetoothd[1772]: adapter_create_device(00:0E:ED:01:DA:66)
bluetoothd[1772]: Creating device
/org/bluez/1772/hci0/dev_00_0E_ED_01_DA_66
bluetoothd[1772]: pin_code_request (sba=00:02:5B:00:A5:A5,
dba=00:0E:ED:01:DA:66)
bluetoothd[1772]: adapter_get_device(00:0E:ED:01:DA:66)
bluetoothd[1772]: /org/bluez/1772/hci0/dev_00_0E_ED_01_DA_66: requesting
agent authentication
Agent_message was called!
Agent called: RequestPinCode
Device path = /org/bluez/1772/hci0/dev_00_0E_ED_01_DA_66
Agent has been released
bluetoothd[1772]: link_key_notify (sba=00:02:5B:00:A5:A5,
dba=00:0E:ED:01:DA:66)
bluetoothd[1772]: adapter_get_device(00:0E:ED:01:DA:66)
bluetoothd[1772]: hcid_dbus_bonding_process_complete: status=00
bluetoothd[1772]: setting timer for reverse service discovery
bluetoothd[1772]: adapter_get_device(00:0E:ED:01:DA:66)
bluetoothd[1772]: Probe drivers for
/org/bluez/1772/hci0/dev_00_0E_ED_01_DA_66
bluetoothd[1772]: audio handle_uuid: server not enabled for
00001112-0000-1000-8000-00805F9B34FB (0x1112)
bluetoothd[1772]: serial_probe: path
/org/bluez/1772/hci0/dev_00_0E_ED_01_DA_66:
00001112-0000-1000-8000-00805F9B34FB
bluetoothd[1772]: Registered interface org.bluez.Serial on path
/org/bluez/1772/hci0/dev_00_0E_ED_01_DA_66
bluetoothd[1772]: Probe drivers for
/org/bluez/1772/hci0/dev_00_0E_ED_01_DA_66
bluetoothd[1772]: audio handle_uuid: server not enabled for
00001112-0000-1000-8000-00805f9b34fb (0x1112)
bluetoothd[1772]: Found Handsfree AG record
bluetoothd[1772]: serial_probe: path
/org/bluez/1772/hci0/dev_00_0E_ED_01_DA_66:
00001105-0000-1000-8000-00805f9b34fb
bluetoothd[1772]: serial_probe: path
/org/bluez/1772/hci0/dev_00_0E_ED_01_DA_66:
00001106-0000-1000-8000-00805f9b34fb
bluetoothd[1772]: serial_probe: path
/org/bluez/1772/hci0/dev_00_0E_ED_01_DA_66:
00001103-0000-1000-8000-00805f9b34fb
bluetoothd[1772]: serial_probe: path
/org/bluez/1772/hci0/dev_00_0E_ED_01_DA_66:
00001101-0000-1000-8000-00805f9b34fb
bluetoothd[1772]: serial_probe: path
/org/bluez/1772/hci0/dev_00_0E_ED_01_DA_66:
00001101-0000-1000-8000-00805f9b34fb
bluetoothd[1772]: serial_probe: path
/org/bluez/1772/hci0/dev_00_0E_ED_01_DA_66:
0000111f-0000-1000-8000-00805f9b34fb
bluetoothd[1772]: serial_probe: path
/org/bluez/1772/hci0/dev_00_0E_ED_01_DA_66:
00001112-0000-1000-8000-00805f9b34fb
bluetoothd[1772]: serial_probe: path
/org/bluez/1772/hci0/dev_00_0E_ED_01_DA_66:
00000002-0000-1000-8000-0002ee000002
bluetoothd[1772]: serial_probe: path
/org/bluez/1772/hci0/dev_00_0E_ED_01_DA_66:
00005002-0000-1000-8000-0002ee000001
bluetoothd[1772]: serial_probe: path
/org/bluez/1772/hci0/dev_00_0E_ED_01_DA_66:
0000112d-0000-1000-8000-00805f9b34fb

What's wrong ? I need support.

Thanks in advance for your help.
Regards.

Thierry