---
audio/unix.c | 55 +++++++++++++++++++++++++++++++++++++++++--------------
1 files changed, 41 insertions(+), 14 deletions(-)
diff --git a/audio/unix.c b/audio/unix.c
index 3fad129..23e2382 100644
--- a/audio/unix.c
+++ b/audio/unix.c
@@ -168,21 +168,28 @@ static void unix_ipc_error(struct unix_client *client, uint8_t name, int err)
rsp->posix_errno = err;
+ debug("sending error %s(%d)", strerror(err), err);
unix_ipc_sendmsg(client, &rsp->h);
}
static service_type_t select_service(struct audio_device *dev, const char *interface)
{
if (!interface) {
- if (dev->sink && avdtp_is_connected(&dev->src, &dev->dst))
+ if (dev->source && avdtp_is_connected(&dev->src, &dev->dst))
+ return TYPE_SOURCE;
+ else if (dev->sink && avdtp_is_connected(&dev->src, &dev->dst))
return TYPE_SINK;
else if (dev->headset && headset_is_active(dev))
return TYPE_HEADSET;
+ else if (dev->source)
+ return TYPE_SOURCE;
else if (dev->sink)
return TYPE_SINK;
else if (dev->headset)
return TYPE_HEADSET;
- } else if (!strcmp(interface, AUDIO_SINK_INTERFACE) && dev->sink)
+ } else if (!strcmp(interface, AUDIO_SOURCE_INTERFACE) && dev->source)
+ return TYPE_SOURCE;
+ else if (!strcmp(interface, AUDIO_SINK_INTERFACE) && dev->sink)
return TYPE_SINK;
else if (!strcmp(interface, AUDIO_HEADSET_INTERFACE) && dev->headset)
return TYPE_HEADSET;
@@ -487,6 +494,7 @@ static void print_sbc(struct sbc_codec_cap *sbc)
static int a2dp_append_codec(struct bt_get_capabilities_rsp *rsp,
struct avdtp_service_capability *cap,
uint8_t seid,
+ uint8_t type,
uint8_t configured,
uint8_t lock)
{
@@ -518,7 +526,10 @@ static int a2dp_append_codec(struct bt_get_capabilities_rsp *rsp,
sbc->max_bitpool = sbc_cap->max_bitpool;
print_sbc(sbc_cap);
- codec->type = BT_A2DP_SBC_SINK;
+ if (type == AVDTP_SEP_TYPE_SINK)
+ codec->type = BT_A2DP_SBC_SINK;
+ else if (type == AVDTP_SEP_TYPE_SOURCE)
+ codec->type = BT_A2DP_SBC_SOURCE;
} else if (codec_cap->media_codec_type == A2DP_CODEC_MPEG12) {
struct mpeg_codec_cap *mpeg_cap = (void *) codec_cap;
mpeg_capabilities_t *mpeg = (void *) codec;
@@ -536,7 +547,10 @@ static int a2dp_append_codec(struct bt_get_capabilities_rsp *rsp,
mpeg->bitrate = mpeg_cap->bitrate;
print_mpeg12(mpeg_cap);
- codec->type = BT_A2DP_MPEG12_SINK;
+ if (type == AVDTP_SEP_TYPE_SINK)
+ codec->type = BT_A2DP_MPEG12_SINK;
+ else if (type == AVDTP_SEP_TYPE_SOURCE)
+ codec->type = BT_A2DP_MPEG12_SOURCE;
} else {
size_t codec_length, type_length, total_length;
@@ -553,7 +567,10 @@ static int a2dp_append_codec(struct bt_get_capabilities_rsp *rsp,
memcpy(codec->data, &codec_cap->media_codec_type, type_length);
memcpy(codec->data + type_length, codec_cap->data,
codec_length);
- codec->type = BT_A2DP_UNKNOWN_SINK;
+ if (type == AVDTP_SEP_TYPE_SINK)
+ codec->type = BT_A2DP_UNKNOWN_SINK;
+ else if (type == AVDTP_SEP_TYPE_SOURCE)
+ codec->type = BT_A2DP_UNKNOWN_SOURCE;
}
codec->seid = seid;
@@ -606,7 +623,8 @@ static void a2dp_discovery_complete(struct avdtp *session, GSList *seps,
type = avdtp_get_type(rsep);
- if (type != AVDTP_SEP_TYPE_SINK)
+ if (type != AVDTP_SEP_TYPE_SINK &&
+ type != AVDTP_SEP_TYPE_SOURCE)
continue;
cap = avdtp_get_codec(rsep);
@@ -641,7 +659,7 @@ static void a2dp_discovery_complete(struct avdtp *session, GSList *seps,
if (sep && a2dp_sep_get_lock(sep))
lock = BT_WRITE_LOCK;
- a2dp_append_codec(rsp, cap, seid, configured, lock);
+ a2dp_append_codec(rsp, cap, seid, type, configured, lock);
}
unix_ipc_sendmsg(client, &rsp->h);
@@ -819,6 +837,7 @@ static void start_discovery(struct audio_device *dev, struct unix_client *client
switch (client->type) {
case TYPE_SINK:
+ case TYPE_SOURCE:
a2dp = &client->d.a2dp;
if (!a2dp->session)
@@ -1252,10 +1271,12 @@ static void handle_getcapabilities_req(struct unix_client *client,
dev = manager_find_device(req->object, &src, &dst,
client->interface, FALSE);
- if (!dev && req->transport == BT_CAPABILITIES_TRANSPORT_SCO) {
+ if (!dev) {
g_free(client->interface);
- client->interface = g_strdup(AUDIO_GATEWAY_INTERFACE);
-
+ if (req->transport == BT_CAPABILITIES_TRANSPORT_SCO)
+ client->interface = g_strdup(AUDIO_GATEWAY_INTERFACE);
+ else if (req->transport == BT_CAPABILITIES_TRANSPORT_A2DP)
+ client->interface = g_strdup(AUDIO_SOURCE_INTERFACE);
dev = manager_find_device(req->object, &src, &dst,
client->interface, TRUE);
if (!dev && (req->flags & BT_FLAG_AUTOCONNECT))
@@ -1305,8 +1326,10 @@ static int handle_sco_open(struct unix_client *client, struct bt_open_req *req)
static int handle_a2dp_open(struct unix_client *client, struct bt_open_req *req)
{
if (!client->interface)
+ /* FIXME: are we treating a sink or a source? */
client->interface = g_strdup(AUDIO_SINK_INTERFACE);
- else if (!g_str_equal(client->interface, AUDIO_SINK_INTERFACE))
+ else if (!g_str_equal(client->interface, AUDIO_SINK_INTERFACE) &&
+ !g_str_equal(client->interface, AUDIO_SOURCE_INTERFACE))
return -EIO;
debug("open a2dp - object=%s source=%s destination=%s lock=%s%s",
@@ -1398,8 +1421,10 @@ static int handle_a2dp_transport(struct unix_client *client,
struct mpeg_codec_cap mpeg_cap;
if (!client->interface)
+ /* FIXME: are we treating a sink or a source? */
client->interface = g_strdup(AUDIO_SINK_INTERFACE);
- else if (!g_str_equal(client->interface, AUDIO_SINK_INTERFACE))
+ else if (!g_str_equal(client->interface, AUDIO_SINK_INTERFACE) &&
+ !g_str_equal(client->interface, AUDIO_SOURCE_INTERFACE))
return -EIO;
if (client->caps) {
@@ -1413,7 +1438,8 @@ static int handle_a2dp_transport(struct unix_client *client,
client->caps = g_slist_append(client->caps, media_transport);
- if (req->codec.type == BT_A2DP_MPEG12_SINK) {
+ if (req->codec.type == BT_A2DP_MPEG12_SINK ||
+ req->codec.type == BT_A2DP_MPEG12_SOURCE) {
mpeg_capabilities_t *mpeg = (void *) &req->codec;
memset(&mpeg_cap, 0, sizeof(mpeg_cap));
@@ -1431,7 +1457,8 @@ static int handle_a2dp_transport(struct unix_client *client,
sizeof(mpeg_cap));
print_mpeg12(&mpeg_cap);
- } else if (req->codec.type == BT_A2DP_SBC_SINK) {
+ } else if (req->codec.type == BT_A2DP_SBC_SINK ||
+ req->codec.type == BT_A2DP_SBC_SOURCE) {
sbc_capabilities_t *sbc = (void *) &req->codec;
memset(&sbc_cap, 0, sizeof(sbc_cap));
--
1.6.0.4
2009/8/16 Johan Hedberg <[email protected]>:
> Hi,
>
> On Sun, Aug 16, 2009, Johan Hedberg wrote:
>> There are a few issues with this patch:
>
> I went ahead and fixed the issues myself so we can proceed with the 4.48
> release. The patches are now pushed upstream.
>
> Johan
>
Sorry for the late reply, didn't check email after very early in the
morning today (GMT-3) because of Mom's birthday.
Thanks for the review and the fixes!
--
João Paulo Rechi Vita
MSc Computer Science Student
Computer Engineer
IC / Unicamp
http://jprvita.wordpress.com/
Hi,
On Sun, Aug 16, 2009, Johan Hedberg wrote:
> There are a few issues with this patch:
I went ahead and fixed the issues myself so we can proceed with the 4.48
release. The patches are now pushed upstream.
Johan
Hi Luiz,
On Sun, Aug 16, 2009, Luiz Augusto von Dentz wrote:
> >> >> ? ? ? if (!interface) {
> >> >> - ? ? ? ? ? ? if (dev->sink && avdtp_is_connected(&dev->src, &dev->dst))
> >> >> + ? ? ? ? ? ? if (dev->source && avdtp_is_connected(&dev->src, &dev->dst))
> >> >> + ? ? ? ? ? ? ? ? ? ? return TYPE_SOURCE;
> >> >> + ? ? ? ? ? ? else if (dev->sink && avdtp_is_connected(&dev->src, &dev->dst))
> >> >> ? ? ? ? ? ? ? ? ? ? ? return TYPE_SINK;
> >> >
> >> > I think the dev->source check should be in an "else if" clause while the
> >> > dev->sink check should be first. I.e. if a device happens to support both
> >> > roles of A2DP we default to dev->sink.
> >>
> >> Good catches, perhaps we should consider doing avdtp_is_connected
> >> check before checking for sink/source pointers. What do you think?
> >
> > What exactly do you mean? Could you perhaps give an example code snippet
> > of how you'd like the code to look like?
> >
> > Johan
> >
>
> Something like this:
>
> if (avdtp_is_connected(...)) {
> if (dev->sink) return TYPE_SINK;
> else if (dev->source) return TYPE_SOURCE;
> else...
> }
Sure, but take a look at the bigger context, i.e. the whole select_service
function. You need to be careful not to change it's current behavior e.g.
in the case that avdtp_is_connected returns TRUE but both dev->sink and
dev->source are NULL (not sure how that could happen but your change would
change the logic for that case).
Johan
Hi Johan,
2009/8/16 Johan Hedberg <[email protected]>:
> Hi Luiz,
>
> On Sun, Aug 16, 2009, Luiz Augusto von Dentz wrote:
>> > On Sat, Aug 15, 2009, Jo?o Paulo Rechi Vita wrote:
>> >> ? ? ? if (!interface) {
>> >> - ? ? ? ? ? ? if (dev->sink && avdtp_is_connected(&dev->src, &dev->dst))
>> >> + ? ? ? ? ? ? if (dev->source && avdtp_is_connected(&dev->src, &dev->dst))
>> >> + ? ? ? ? ? ? ? ? ? ? return TYPE_SOURCE;
>> >> + ? ? ? ? ? ? else if (dev->sink && avdtp_is_connected(&dev->src, &dev->dst))
>> >> ? ? ? ? ? ? ? ? ? ? ? return TYPE_SINK;
>> >
>> > I think the dev->source check should be in an "else if" clause while the
>> > dev->sink check should be first. I.e. if a device happens to support both
>> > roles of A2DP we default to dev->sink.
>>
>> Good catches, perhaps we should consider doing avdtp_is_connected
>> check before checking for sink/source pointers. What do you think?
>
> What exactly do you mean? Could you perhaps give an example code snippet
> of how you'd like the code to look like?
>
> Johan
>
Something like this:
if (avdtp_is_connected(...)) {
if (dev->sink) return TYPE_SINK;
else if (dev->source) return TYPE_SOURCE;
else...
}
--
Luiz Augusto von Dentz
Engenheiro de Computa??o
Hi Luiz,
On Sun, Aug 16, 2009, Luiz Augusto von Dentz wrote:
> > On Sat, Aug 15, 2009, Jo?o Paulo Rechi Vita wrote:
> >> ? ? ? if (!interface) {
> >> - ? ? ? ? ? ? if (dev->sink && avdtp_is_connected(&dev->src, &dev->dst))
> >> + ? ? ? ? ? ? if (dev->source && avdtp_is_connected(&dev->src, &dev->dst))
> >> + ? ? ? ? ? ? ? ? ? ? return TYPE_SOURCE;
> >> + ? ? ? ? ? ? else if (dev->sink && avdtp_is_connected(&dev->src, &dev->dst))
> >> ? ? ? ? ? ? ? ? ? ? ? return TYPE_SINK;
> >
> > I think the dev->source check should be in an "else if" clause while the
> > dev->sink check should be first. I.e. if a device happens to support both
> > roles of A2DP we default to dev->sink.
>
> Good catches, perhaps we should consider doing avdtp_is_connected
> check before checking for sink/source pointers. What do you think?
What exactly do you mean? Could you perhaps give an example code snippet
of how you'd like the code to look like?
Johan
Hi,
2009/8/16 Johan Hedberg <[email protected]>:
> Hi,
>
> There are a few issues with this patch:
>
> On Sat, Aug 15, 2009, Jo?o Paulo Rechi Vita wrote:
>> ? ? ? if (!interface) {
>> - ? ? ? ? ? ? if (dev->sink && avdtp_is_connected(&dev->src, &dev->dst))
>> + ? ? ? ? ? ? if (dev->source && avdtp_is_connected(&dev->src, &dev->dst))
>> + ? ? ? ? ? ? ? ? ? ? return TYPE_SOURCE;
>> + ? ? ? ? ? ? else if (dev->sink && avdtp_is_connected(&dev->src, &dev->dst))
>> ? ? ? ? ? ? ? ? ? ? ? return TYPE_SINK;
>
> I think the dev->source check should be in an "else if" clause while the
> dev->sink check should be first. I.e. if a device happens to support both
> roles of A2DP we default to dev->sink.
Good catches, perhaps we should consider doing avdtp_is_connected
check before checking for sink/source pointers. What do you think?
--
Luiz Augusto von Dentz
Engenheiro de Computa??o
Hi,
There are a few issues with this patch:
On Sat, Aug 15, 2009, Jo?o Paulo Rechi Vita wrote:
> if (!interface) {
> - if (dev->sink && avdtp_is_connected(&dev->src, &dev->dst))
> + if (dev->source && avdtp_is_connected(&dev->src, &dev->dst))
> + return TYPE_SOURCE;
> + else if (dev->sink && avdtp_is_connected(&dev->src, &dev->dst))
> return TYPE_SINK;
I think the dev->source check should be in an "else if" clause while the
dev->sink check should be first. I.e. if a device happens to support both
roles of A2DP we default to dev->sink.
> + else if (dev->source)
> + return TYPE_SOURCE;
> else if (dev->sink)
> return TYPE_SINK;
Same here. dev->sink check before dev->source.
> - codec->type = BT_A2DP_SBC_SINK;
> + if (type == AVDTP_SEP_TYPE_SINK)
> + codec->type = BT_A2DP_SBC_SINK;
> + else if (type == AVDTP_SEP_TYPE_SOURCE)
> + codec->type = BT_A2DP_SBC_SOURCE;
What about the "else" part? If type is neither you should probably bail
out of the function with -EINVAL.
> print_mpeg12(mpeg_cap);
> - codec->type = BT_A2DP_MPEG12_SINK;
> + if (type == AVDTP_SEP_TYPE_SINK)
> + codec->type = BT_A2DP_MPEG12_SINK;
> + else if (type == AVDTP_SEP_TYPE_SOURCE)
> + codec->type = BT_A2DP_MPEG12_SOURCE;
Same here.
> + if (type == AVDTP_SEP_TYPE_SINK)
> + codec->type = BT_A2DP_UNKNOWN_SINK;
> + else if (type == AVDTP_SEP_TYPE_SOURCE)
> + codec->type = BT_A2DP_UNKNOWN_SOURCE;
And again..
> }
>
> codec->seid = seid;
> @@ -606,7 +623,8 @@ static void a2dp_discovery_complete(struct avdtp *session, GSList *seps,
>
> type = avdtp_get_type(rsep);
>
> - if (type != AVDTP_SEP_TYPE_SINK)
> + if (type != AVDTP_SEP_TYPE_SINK &&
> + type != AVDTP_SEP_TYPE_SOURCE)
> continue;
Incorrect indentation for the continuation line of the if clause. It
should at least two tabs from the original to clearly separate it from the
"continue;" line.
> + if (!dev) {
> g_free(client->interface);
> - client->interface = g_strdup(AUDIO_GATEWAY_INTERFACE);
> -
> + if (req->transport == BT_CAPABILITIES_TRANSPORT_SCO)
> + client->interface = g_strdup(AUDIO_GATEWAY_INTERFACE);
> + else if (req->transport == BT_CAPABILITIES_TRANSPORT_A2DP)
> + client->interface = g_strdup(AUDIO_SOURCE_INTERFACE);
Again a missing "else" clause but this time the consequences are even more
serious. If req->transport doesn't match either of those values you'll
leave client->interface pointing to free'd memory which will probably
cause access to free'd memory or a double-free later. So at the very least
you should set client->interface to NULL here and maybe also return an
error from the function.
Johan
Check if we find the device before trying to define an interface,
so if we fail the if comparison plus the strdup are not necessary.
---
audio/unix.c | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/audio/unix.c b/audio/unix.c
index 23e2382..e0307e0 100644
--- a/audio/unix.c
+++ b/audio/unix.c
@@ -1257,14 +1257,13 @@ static void handle_getcapabilities_req(struct unix_client *client,
str2ba(req->source, &src);
str2ba(req->destination, &dst);
+ if (!manager_find_device(req->object, &src, &dst, NULL, FALSE))
+ goto failed;
+
if (req->transport == BT_CAPABILITIES_TRANSPORT_SCO)
client->interface = g_strdup(AUDIO_HEADSET_INTERFACE);
else if (req->transport == BT_CAPABILITIES_TRANSPORT_A2DP)
client->interface = g_strdup(AUDIO_SINK_INTERFACE);
-
- if (!manager_find_device(req->object, &src, &dst, NULL, FALSE))
- goto failed;
-
dev = manager_find_device(req->object, &src, &dst, client->interface,
TRUE);
if (!dev && (req->flags & BT_FLAG_AUTOCONNECT))
--
1.6.0.4