From: Chan-yeol Park <[email protected]>
---
audio/a2dp.c | 8 ++++----
audio/audio.conf | 2 +-
audio/manager.c | 4 ++--
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/audio/a2dp.c b/audio/a2dp.c
index cc8f139..145dcf6 100644
--- a/audio/a2dp.c
+++ b/audio/a2dp.c
@@ -1406,9 +1406,9 @@ int a2dp_register(DBusConnection *conn, const bdaddr_t *src, GKeyFile *config)
g_clear_error(&err);
} else {
if (strstr(str, "Sink"))
- source = TRUE;
- if (strstr(str, "Source"))
sink = TRUE;
+ if (strstr(str, "Source"))
+ source = TRUE;
if (strstr(str, "Socket"))
socket = TRUE;
g_free(str);
@@ -1421,9 +1421,9 @@ int a2dp_register(DBusConnection *conn, const bdaddr_t *src, GKeyFile *config)
g_clear_error(&err);
} else {
if (strstr(str, "Sink"))
- source = FALSE;
- if (strstr(str, "Source"))
sink = FALSE;
+ if (strstr(str, "Source"))
+ source = FALSE;
if (strstr(str, "Socket"))
socket = FALSE;
g_free(str);
diff --git a/audio/audio.conf b/audio/audio.conf
index fd6092a..1abdab9 100644
--- a/audio/audio.conf
+++ b/audio/audio.conf
@@ -9,7 +9,7 @@
# If we want to disable support for specific services
# Defaults to supporting all implemented services
-#Disable=Gateway,Source,Socket
+#Disable=Gateway,Sink,Socket
# SCO routing. Either PCM or HCI (in which case audio is routed to/from ALSA)
# Defaults to HCI
diff --git a/audio/manager.c b/audio/manager.c
index 9c7d0d3..9e5c71c 100644
--- a/audio/manager.c
+++ b/audio/manager.c
@@ -114,8 +114,8 @@ static struct enabled_interfaces enabled = {
.hfp = TRUE,
.headset = TRUE,
.gateway = FALSE,
- .sink = TRUE,
- .source = FALSE,
+ .sink = FALSE,
+ .source = TRUE,
.control = TRUE,
.socket = FALSE,
.media = TRUE,
--
1.7.9.5
Hi,
On Fri, May 18, 2012, Johan Hedberg wrote:
> On Fri, May 18, 2012, Lucas De Marchi wrote:
> > >> --- a/audio/avrcp.c
> > >> +++ b/audio/avrcp.c
> > >> @@ -1272,8 +1272,11 @@ void avrcp_unregister(const bdaddr_t *src)
> > >>
> > >> ? ? ? servers = g_slist_remove(servers, server);
> > >>
> > >> - ? ? remove_record_from_server(server->ct_record_id);
> > >> - ? ? remove_record_from_server(server->tg_record_id);
> > >> + ? ? if (server->ct_record_id)
> > >> + ? ? ? ? ? ? remove_record_from_server(server->ct_record_id);
> > >> +
> > >> + ? ? if (server->tg_record_id)
> > >> + ? ? ? ? ? ? remove_record_from_server(server->tg_record_id);
> > >>
> > >> ? ? ? avctp_unregister(&server->src);
> > >> ? ? ? g_free(server);
> > >
> > > I don't think the commit message for this patch is truthful. If you look
> > > at the code the remove_record_from_server will return ENOENT if you pass
> > > it a non-existent handle. I.e. it will not crash. Please fix the commit
> > > message to reflect what exactly is being fixed (i.e. an unnecessary call
> > > to remove_record_from_server if there is no record handle).
> >
> > Then I don't see a reason: just let remove_record_from_server() fail ?
> >
> > Chan, what are you fixing here?
>
> As discussed on IRC (and Chanyeol should really have explained this in
> the commit message) handle 0 is actually the SDP server's own "master"
> record. So it is plausible that weird behavior might occur if trying to
> remove it. Since this is a SDP server internal detail and it doesn't
> really make sense to remove it I'm thinking that
> remove_record_from_server should return EINVAL if passed a 0 value.
I already pushed a patch for this so no need to work on it anymore.
Johan
Hi Lucas,
On Fri, May 18, 2012, Lucas De Marchi wrote:
> >> --- a/audio/avrcp.c
> >> +++ b/audio/avrcp.c
> >> @@ -1272,8 +1272,11 @@ void avrcp_unregister(const bdaddr_t *src)
> >>
> >> ? ? ? servers = g_slist_remove(servers, server);
> >>
> >> - ? ? remove_record_from_server(server->ct_record_id);
> >> - ? ? remove_record_from_server(server->tg_record_id);
> >> + ? ? if (server->ct_record_id)
> >> + ? ? ? ? ? ? remove_record_from_server(server->ct_record_id);
> >> +
> >> + ? ? if (server->tg_record_id)
> >> + ? ? ? ? ? ? remove_record_from_server(server->tg_record_id);
> >>
> >> ? ? ? avctp_unregister(&server->src);
> >> ? ? ? g_free(server);
> >
> > I don't think the commit message for this patch is truthful. If you look
> > at the code the remove_record_from_server will return ENOENT if you pass
> > it a non-existent handle. I.e. it will not crash. Please fix the commit
> > message to reflect what exactly is being fixed (i.e. an unnecessary call
> > to remove_record_from_server if there is no record handle).
>
> Then I don't see a reason: just let remove_record_from_server() fail ?
>
> Chan, what are you fixing here?
As discussed on IRC (and Chanyeol should really have explained this in
the commit message) handle 0 is actually the SDP server's own "master"
record. So it is plausible that weird behavior might occur if trying to
remove it. Since this is a SDP server internal detail and it doesn't
really make sense to remove it I'm thinking that
remove_record_from_server should return EINVAL if passed a 0 value.
Johan
On Fri, May 18, 2012 at 8:40 AM, Johan Hedberg <[email protected]> wrote:
> Hi Chanyeol,
>
> On Fri, May 18, 2012, [email protected] wrote:
>> From: Chan-yeol Park <[email protected]>
>>
>> ---
>> ?audio/avrcp.c | ? ?7 +++++--
>> ?1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/audio/avrcp.c b/audio/avrcp.c
>> index df39d04..2e946c1 100644
>> --- a/audio/avrcp.c
>> +++ b/audio/avrcp.c
>> @@ -1272,8 +1272,11 @@ void avrcp_unregister(const bdaddr_t *src)
>>
>> ? ? ? servers = g_slist_remove(servers, server);
>>
>> - ? ? remove_record_from_server(server->ct_record_id);
>> - ? ? remove_record_from_server(server->tg_record_id);
>> + ? ? if (server->ct_record_id)
>> + ? ? ? ? ? ? remove_record_from_server(server->ct_record_id);
>> +
>> + ? ? if (server->tg_record_id)
>> + ? ? ? ? ? ? remove_record_from_server(server->tg_record_id);
>>
>> ? ? ? avctp_unregister(&server->src);
>> ? ? ? g_free(server);
>
> I don't think the commit message for this patch is truthful. If you look
> at the code the remove_record_from_server will return ENOENT if you pass
> it a non-existent handle. I.e. it will not crash. Please fix the commit
> message to reflect what exactly is being fixed (i.e. an unnecessary call
> to remove_record_from_server if there is no record handle).
Then I don't see a reason: just let remove_record_from_server() fail ?
Chan, what are you fixing here?
Lucas De Marchi
Hi Chanyeol,
On Fri, May 18, 2012, [email protected] wrote:
> From: Chan-yeol Park <[email protected]>
>
> ---
> audio/avrcp.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/audio/avrcp.c b/audio/avrcp.c
> index df39d04..2e946c1 100644
> --- a/audio/avrcp.c
> +++ b/audio/avrcp.c
> @@ -1272,8 +1272,11 @@ void avrcp_unregister(const bdaddr_t *src)
>
> servers = g_slist_remove(servers, server);
>
> - remove_record_from_server(server->ct_record_id);
> - remove_record_from_server(server->tg_record_id);
> + if (server->ct_record_id)
> + remove_record_from_server(server->ct_record_id);
> +
> + if (server->tg_record_id)
> + remove_record_from_server(server->tg_record_id);
>
> avctp_unregister(&server->src);
> g_free(server);
I don't think the commit message for this patch is truthful. If you look
at the code the remove_record_from_server will return ENOENT if you pass
it a non-existent handle. I.e. it will not crash. Please fix the commit
message to reflect what exactly is being fixed (i.e. an unnecessary call
to remove_record_from_server if there is no record handle).
Johan
Hi Chanyeol,
On Fri, May 18, 2012, [email protected] wrote:
> From: Chan-yeol Park <[email protected]>
>
> ---
> audio/a2dp.c | 8 ++++----
> audio/audio.conf | 2 +-
> audio/manager.c | 4 ++--
> 3 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/audio/a2dp.c b/audio/a2dp.c
> index cc8f139..145dcf6 100644
> --- a/audio/a2dp.c
> +++ b/audio/a2dp.c
> @@ -1406,9 +1406,9 @@ int a2dp_register(DBusConnection *conn, const bdaddr_t *src, GKeyFile *config)
> g_clear_error(&err);
> } else {
> if (strstr(str, "Sink"))
> - source = TRUE;
> - if (strstr(str, "Source"))
> sink = TRUE;
> + if (strstr(str, "Source"))
> + source = TRUE;
> if (strstr(str, "Socket"))
> socket = TRUE;
> g_free(str);
> @@ -1421,9 +1421,9 @@ int a2dp_register(DBusConnection *conn, const bdaddr_t *src, GKeyFile *config)
> g_clear_error(&err);
> } else {
> if (strstr(str, "Sink"))
> - source = FALSE;
> - if (strstr(str, "Source"))
> sink = FALSE;
> + if (strstr(str, "Source"))
> + source = FALSE;
> if (strstr(str, "Socket"))
> socket = FALSE;
> g_free(str);
This doesn't look right to me. The config file values have (for
historical reasons) been referring to the remote device object D-Bus
interface names whereas these gboolean variables are used to determine
what kind of local stream endpoints need to be registered. So having
e.g. support for "Source" on a remote device requires registering a
local sink stream endpoint.
Johan
From: Chan-yeol Park <[email protected]>
---
audio/avrcp.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/audio/avrcp.c b/audio/avrcp.c
index df39d04..2e946c1 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -1272,8 +1272,11 @@ void avrcp_unregister(const bdaddr_t *src)
servers = g_slist_remove(servers, server);
- remove_record_from_server(server->ct_record_id);
- remove_record_from_server(server->tg_record_id);
+ if (server->ct_record_id)
+ remove_record_from_server(server->ct_record_id);
+
+ if (server->tg_record_id)
+ remove_record_from_server(server->tg_record_id);
avctp_unregister(&server->src);
g_free(server);
--
1.7.9.5
From: Chan-yeol Park <[email protected]>
This patch adds logic for activating configured AVRCP role.
---
audio/audio.conf | 2 +-
audio/avrcp.c | 107 ++++++++++++++++++++++++++++++++++++------------------
2 files changed, 73 insertions(+), 36 deletions(-)
diff --git a/audio/audio.conf b/audio/audio.conf
index 1abdab9..75dad56 100644
--- a/audio/audio.conf
+++ b/audio/audio.conf
@@ -9,7 +9,7 @@
# If we want to disable support for specific services
# Defaults to supporting all implemented services
-#Disable=Gateway,Sink,Socket
+#Disable=Gateway,Sink,Control,Socket
# SCO routing. Either PCM or HCI (in which case audio is routed to/from ALSA)
# Defaults to HCI
diff --git a/audio/avrcp.c b/audio/avrcp.c
index 2e946c1..1b781a1 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -1183,57 +1183,94 @@ void avrcp_disconnect(struct audio_device *dev)
int avrcp_register(DBusConnection *conn, const bdaddr_t *src, GKeyFile *config)
{
sdp_record_t *record;
- gboolean tmp, master = TRUE;
+ gboolean tmp, master = TRUE, target = TRUE, control = FALSE;
+ char *str;
GError *err = NULL;
struct avrcp_server *server;
- if (config) {
- tmp = g_key_file_get_boolean(config, "General",
- "Master", &err);
- if (err) {
- DBG("audio.conf: %s", err->message);
- g_error_free(err);
- } else
- master = tmp;
+ if (!config)
+ goto proceed;
+
+ tmp = g_key_file_get_boolean(config, "General",
+ "Master", &err);
+ if (err) {
+ DBG("audio.conf: %s", err->message);
+ g_clear_error(&err);
+ } else
+ master = tmp;
+
+ str = g_key_file_get_string(config, "General", "Enable", &err);
+
+ if (err) {
+ DBG("audio.conf: %s", err->message);
+ g_clear_error(&err);
+ } else {
+ if (strstr(str, "Target"))
+ target = TRUE;
+ if (strstr(str, "Control"))
+ control = TRUE;
+ g_free(str);
}
+ str = g_key_file_get_string(config, "General", "Disable", &err);
+
+ if (err) {
+ DBG("audio.conf: %s", err->message);
+ g_clear_error(&err);
+ } else {
+ if (strstr(str, "Target"))
+ target = FALSE;
+ if (strstr(str, "Control"))
+ control = FALSE;
+ g_free(str);
+ }
+
+
+proceed:
server = g_new0(struct avrcp_server, 1);
if (!server)
return -ENOMEM;
- record = avrcp_tg_record();
- if (!record) {
- error("Unable to allocate new service record");
- g_free(server);
- return -1;
- }
+ if (target) {
+ record = avrcp_tg_record();
+ if (!record) {
+ error("Unable to allocate new service record");
+ g_free(server);
+ return -1;
+ }
- if (add_record_to_server(src, record) < 0) {
- error("Unable to register AVRCP target service record");
- g_free(server);
- sdp_record_free(record);
- return -1;
- }
- server->tg_record_id = record->handle;
+ if (add_record_to_server(src, record) < 0) {
+ error("Unable to register AVRCP target service record");
+ g_free(server);
+ sdp_record_free(record);
+ return -1;
+ }
+ server->tg_record_id = record->handle;
- record = avrcp_ct_record();
- if (!record) {
- error("Unable to allocate new service record");
- g_free(server);
- return -1;
}
- if (add_record_to_server(src, record) < 0) {
- error("Unable to register AVRCP service record");
- sdp_record_free(record);
- g_free(server);
- return -1;
+ if (control) {
+ record = avrcp_ct_record();
+ if (!record) {
+ error("Unable to allocate new service record");
+ g_free(server);
+ return -1;
+ }
+
+ if (add_record_to_server(src, record) < 0) {
+ error("Unable to register AVRCP service record");
+ sdp_record_free(record);
+ g_free(server);
+ return -1;
+ }
+ server->ct_record_id = record->handle;
}
- server->ct_record_id = record->handle;
if (avctp_register(src, master) < 0) {
- remove_record_from_server(server->ct_record_id);
- remove_record_from_server(server->tg_record_id);
+ if (control)
+ remove_record_from_server(server->ct_record_id);
+ if (target)
+ remove_record_from_server(server->tg_record_id);
g_free(server);
return -1;
}
--
1.7.9.5