[PATCH v2 1/7] hfpmodem: Watch for changes in the selected codec

[PATCH v2 1/7] hfpmodem: Watch for changes in the selected codec

[Vinicius Costa Gomes]

[-- Attachment #1: Type: text/plain, Size: 1875 bytes --]

This patch adds a function to monitor when the AG sends a new codec
before establishing the SCO connection.
---
 plugins/hfp_hf_bluez5.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index c63b1a2..efbb209 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -314,9 +314,60 @@ static struct ofono_modem_driver hfp_driver = {
 	.post_sim	= hfp_post_sim,
 };
 
+static void bcs_notify(GAtResult *result, gpointer user_data)
+{
+	struct hfp *hfp = user_data;
+	struct hfp_slc_info *info = &hfp->info;
+	GAtResultIter iter;
+	char str[32];
+	int value;
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "+BCS:"))
+		return;
+
+	if (!g_at_result_iter_next_number(&iter, &value))
+		return;
+
+	memset(str, 0, sizeof(str));
+
+	switch (value) {
+	case HFP_CODEC_MSBC:
+		if (!ofono_handsfree_audio_has_wideband()) {
+			/* No wideband speech support, so we only have CVSD */
+			sprintf(str, "AT+BAC=%d", HFP_CODEC_CVSD);
+			break;
+		}
+
+		/* fall through */
+
+	case HFP_CODEC_CVSD:
+		/* Confirm the codec */
+		sprintf(str, "AT+BCS=%d", value);
+		break;
+
+	default:
+		/* Unsupported codec, re-send our codecs */
+		if (ofono_handsfree_audio_has_wideband())
+			sprintf(str, "AT+BAC=%d,%d", HFP_CODEC_CVSD,
+							HFP_CODEC_MSBC);
+		else
+			sprintf(str, "AT+BAC=%d", HFP_CODEC_CVSD);
+	}
+
+	g_at_chat_send(info->chat, str, NULL, NULL, NULL, NULL);
+}
+
 static int hfp16_card_probe(struct ofono_handsfree_card *card,
 					unsigned int vendor, void *data)
 {
+	struct hfp *hfp = data;
+	struct hfp_slc_info *info = &hfp->info;
+
+	g_at_chat_register(info->chat, "+BCS:", bcs_notify, FALSE,
+								hfp, NULL);
+
 	return 0;
 }
 
-- 
1.8.2

[PATCH v2 2/7] include: ofono_handsfree_card_select_codec()

[Vinicius Costa Gomes]

[-- Attachment #1: Type: text/plain, Size: 768 bytes --]

This will be used by the drivers that a given codec was negotiated
for a card.
---
 include/handsfree-audio.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/handsfree-audio.h b/include/handsfree-audio.h
index 82d1312..2e58c43 100644
--- a/include/handsfree-audio.h
+++ b/include/handsfree-audio.h
@@ -48,6 +48,8 @@ struct ofono_handsfree_card *ofono_handsfree_card_create(unsigned int vendor,
 							void *data);
 int ofono_handsfree_card_register(struct ofono_handsfree_card *card);
 void ofono_handsfree_card_remove(struct ofono_handsfree_card *card);
+void ofono_handsfree_card_select_codec(struct ofono_handsfree_card *card,
+							unsigned char codec);
 
 ofono_bool_t ofono_handsfree_audio_has_wideband(void);
 
-- 
1.8.2

[PATCH v2 3/7] handsfree-audio: Implement ofono_handsfree_card_select_codec()

[Vinicius Costa Gomes]

[-- Attachment #1: Type: text/plain, Size: 802 bytes --]

---
 src/handsfree-audio.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
index 50be691..3d6804d 100644
--- a/src/handsfree-audio.c
+++ b/src/handsfree-audio.c
@@ -49,6 +49,7 @@ struct ofono_handsfree_card {
 	char *local;
 	char *path;
 	DBusMessage *msg;
+	unsigned char selected_codec;
 	const struct ofono_handsfree_card_driver *driver;
 	void *driver_data;
 };
@@ -535,6 +536,12 @@ void ofono_handsfree_card_remove(struct ofono_handsfree_card *card)
 	g_free(card);
 }
 
+void ofono_handsfree_card_select_codec(struct ofono_handsfree_card *card,
+							unsigned char codec)
+{
+	card->selected_codec = codec;
+}
+
 ofono_bool_t ofono_handsfree_audio_has_wideband(void)
 {
 	return has_wideband;
-- 
1.8.2

[PATCH v2 4/7] hfp_hf_bluez5: Set the audio codec in the card

[Vinicius Costa Gomes]

[-- Attachment #1: Type: text/plain, Size: 701 bytes --]

From: Claudio Takahasi <claudio.takahasi@openbossa.org>

This patch updates the handsfree audio card codec when the AG notifies
the codec for the succeeding incoming SCO connection.
---
 plugins/hfp_hf_bluez5.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index efbb209..2279041 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -343,6 +343,8 @@ static void bcs_notify(GAtResult *result, gpointer user_data)
 		/* fall through */
 
 	case HFP_CODEC_CVSD:
+		ofono_handsfree_card_select_codec(hfp->card, value);
+
 		/* Confirm the codec */
 		sprintf(str, "AT+BCS=%d", value);
 		break;
-- 
1.8.2

[PATCH v2 5/7] handsfree-audio: Send the selected codec

[Vinicius Costa Gomes]

[-- Attachment #1: Type: text/plain, Size: 1466 bytes --]

From: Claudio Takahasi <claudio.takahasi@openbossa.org>

This patch removes the hard-coded CVSD codec, and adds the selected
codec in the NewConnection method call, notifying the agent the codec
previously selected for the audio connection.
---
 src/handsfree-audio.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
index 3d6804d..8fc6f93 100644
--- a/src/handsfree-audio.c
+++ b/src/handsfree-audio.c
@@ -67,11 +67,10 @@ static guint sco_watch = 0;
 static GSList *drivers = 0;
 static ofono_bool_t has_wideband = FALSE;
 
-static void send_new_connection(const char *card, int fd)
+static void send_new_connection(const char *card, int fd, uint8_t codec)
 {
 	DBusMessage *msg;
 	DBusMessageIter iter;
-	uint8_t codec = HFP_CODEC_CVSD;
 
 	msg = dbus_message_new_method_call(agent->owner, agent->path,
 				HFP_AUDIO_AGENT_INTERFACE, "NewConnection");
@@ -150,7 +149,7 @@ static gboolean sco_accept(GIOChannel *io, GIOCondition cond,
 		return TRUE;
 	}
 
-	send_new_connection(card->path, nsk);
+	send_new_connection(card->path, nsk, card->selected_codec);
 	close(nsk);
 
 	return TRUE;
@@ -252,7 +251,7 @@ static gboolean sco_connect_cb(GIOChannel *io, GIOCondition cond,
 
 	sk = g_io_channel_unix_get_fd(io);
 
-	send_new_connection(card->path, sk);
+	send_new_connection(card->path, sk, card->selected_codec);
 
 	close(sk);
 
-- 
1.8.2

[PATCH v2 6/7] handsfree-audio: Set CVSD as default in the card

[Vinicius Costa Gomes]

[-- Attachment #1: Type: text/plain, Size: 716 bytes --]

From: Claudio Takahasi <claudio.takahasi@openbossa.org>

If the device doesn't support codec negotiation, the selected codec is
not being initialized. For this case set CVSD as default.
---
 src/handsfree-audio.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
index 8fc6f93..a332f68 100644
--- a/src/handsfree-audio.c
+++ b/src/handsfree-audio.c
@@ -342,6 +342,8 @@ struct ofono_handsfree_card *ofono_handsfree_card_create(unsigned int vendor,
 
 	card = g_new0(struct ofono_handsfree_card, 1);
 
+	card->selected_codec = HFP_CODEC_CVSD;
+
 	card_list = g_slist_prepend(card_list, card);
 
 	for (l = drivers; l; l = l->next) {
-- 
1.8.2

[PATCH v2 7/7] handsfree-audio: Enable wideband speech if defer is enabled

[Vinicius Costa Gomes]

[-- Attachment #1: Type: text/plain, Size: 1572 bytes --]

From: Claudio Takahasi <claudio.takahasi@openbossa.org>

Only enable support for wideband speech codecs, at this point mSBC, if
the kernel supports defer setup.
---
 src/handsfree-audio.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
index a332f68..da8e100 100644
--- a/src/handsfree-audio.c
+++ b/src/handsfree-audio.c
@@ -66,6 +66,7 @@ static GSList *card_list = 0;
 static guint sco_watch = 0;
 static GSList *drivers = 0;
 static ofono_bool_t has_wideband = FALSE;
+static int defer_setup = 1;
 
 static void send_new_connection(const char *card, int fd, uint8_t codec)
 {
@@ -159,7 +160,7 @@ static int sco_init(void)
 {
 	GIOChannel *sco_io;
 	struct sockaddr_sco saddr;
-	int sk, defer_setup = 1;
+	int sk;
 
 	sk = socket(PF_BLUETOOTH, SOCK_SEQPACKET | O_NONBLOCK | SOCK_CLOEXEC,
 								BTPROTO_SCO);
@@ -177,9 +178,11 @@ static int sco_init(void)
 	}
 
 	if (setsockopt(sk, SOL_BLUETOOTH, BT_DEFER_SETUP,
-				&defer_setup, sizeof(defer_setup)) < 0)
+				&defer_setup, sizeof(defer_setup)) < 0) {
+		defer_setup = 0;
 		ofono_warn("Can't enable deferred setup: %s (%d)",
 						strerror(errno), errno);
+	}
 
 	if (listen(sk, 5) < 0) {
 		close(sk);
@@ -656,6 +659,9 @@ static DBusMessage *am_agent_register(DBusConnection *conn,
 			has_cvsd = TRUE;
 		else if (codecs[i] != HFP_CODEC_MSBC)
 			return __ofono_error_invalid_args(msg);
+
+		if (defer_setup)
+			has_wideband = TRUE;
 	}
 
 	if (has_cvsd == FALSE) {
-- 
1.8.2

Re: [PATCH v2 1/7] hfpmodem: Watch for changes in the selected codec

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 2599 bytes --]

Hi Vinicius,

On 04/10/2013 02:37 PM, Vinicius Costa Gomes wrote:
> This patch adds a function to monitor when the AG sends a new codec
> before establishing the SCO connection.
> ---
>   plugins/hfp_hf_bluez5.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 51 insertions(+)
>
> diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
> index c63b1a2..efbb209 100644
> --- a/plugins/hfp_hf_bluez5.c
> +++ b/plugins/hfp_hf_bluez5.c
> @@ -314,9 +314,60 @@ static struct ofono_modem_driver hfp_driver = {
>   	.post_sim	= hfp_post_sim,
>   };
>
> +static void bcs_notify(GAtResult *result, gpointer user_data)
> +{
> +	struct hfp *hfp = user_data;
> +	struct hfp_slc_info *info =&hfp->info;
> +	GAtResultIter iter;
> +	char str[32];
> +	int value;
> +
> +	g_at_result_iter_init(&iter, result);
> +
> +	if (!g_at_result_iter_next(&iter, "+BCS:"))
> +		return;
> +
> +	if (!g_at_result_iter_next_number(&iter,&value))
> +		return;
> +
> +	memset(str, 0, sizeof(str));

Why do you need this exactly?

> +
> +	switch (value) {
> +	case HFP_CODEC_MSBC:
> +		if (!ofono_handsfree_audio_has_wideband()) {
> +			/* No wideband speech support, so we only have CVSD */
> +			sprintf(str, "AT+BAC=%d", HFP_CODEC_CVSD);
> +			break;
> +		}
> +
> +		/* fall through */
> +

This fall through and the default statement make this code extremely 
hard to read.  Why don't we move the codec selection logic into the core 
instead?

Then this becomes something like:

if (ofono_handsfree_audio_card_set_codec(card, value) == FALSE) {
	if (ofono_handsfree_audio_has_wideband())
		...
	else
		...

	goto done;
}

sprintf(str, "AT+BCS=%d", value);

done:
....


> +	case HFP_CODEC_CVSD:
> +		/* Confirm the codec */
> +		sprintf(str, "AT+BCS=%d", value);
> +		break;
> +
> +	default:
> +		/* Unsupported codec, re-send our codecs */
> +		if (ofono_handsfree_audio_has_wideband())
> +			sprintf(str, "AT+BAC=%d,%d", HFP_CODEC_CVSD,
> +							HFP_CODEC_MSBC);
> +		else
> +			sprintf(str, "AT+BAC=%d", HFP_CODEC_CVSD);
> +	}
> +
> +	g_at_chat_send(info->chat, str, NULL, NULL, NULL, NULL);
> +}
> +
>   static int hfp16_card_probe(struct ofono_handsfree_card *card,
>   					unsigned int vendor, void *data)
>   {
> +	struct hfp *hfp = data;
> +	struct hfp_slc_info *info =&hfp->info;
> +
> +	g_at_chat_register(info->chat, "+BCS:", bcs_notify, FALSE,
> +								hfp, NULL);
> +

You read my mind, I was going to suggest doing this exactly :)

>   	return 0;
>   }
>

Regards,
-Denis

Re: [PATCH v2 2/7] include: ofono_handsfree_card_select_codec()

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1130 bytes --]

Hi Vinicius,

On 04/10/2013 02:37 PM, Vinicius Costa Gomes wrote:
> This will be used by the drivers that a given codec was negotiated
> for a card.
> ---
>   include/handsfree-audio.h | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/include/handsfree-audio.h b/include/handsfree-audio.h
> index 82d1312..2e58c43 100644
> --- a/include/handsfree-audio.h
> +++ b/include/handsfree-audio.h
> @@ -48,6 +48,8 @@ struct ofono_handsfree_card *ofono_handsfree_card_create(unsigned int vendor,
>   							void *data);
>   int ofono_handsfree_card_register(struct ofono_handsfree_card *card);
>   void ofono_handsfree_card_remove(struct ofono_handsfree_card *card);
> +void ofono_handsfree_card_select_codec(struct ofono_handsfree_card *card,
> +							unsigned char codec);

As mentioned before, lets make this return ofono_bool_t and name it 
'set_codec'.

Also, do we have some idea whether codec ids are limited to 8 bits or 
they can be larger?  My reading of the spec would seem to indicate 8 
bits maximum.

>
>   ofono_bool_t ofono_handsfree_audio_has_wideband(void);
>

Regards,
-Denis

Re: [PATCH v2 6/7] handsfree-audio: Set CVSD as default in the card

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 889 bytes --]

Hi Vinicius,

On 04/10/2013 02:37 PM, Vinicius Costa Gomes wrote:
> From: Claudio Takahasi<claudio.takahasi@openbossa.org>
>
> If the device doesn't support codec negotiation, the selected codec is
> not being initialized. For this case set CVSD as default.
> ---
>   src/handsfree-audio.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
> index 8fc6f93..a332f68 100644
> --- a/src/handsfree-audio.c
> +++ b/src/handsfree-audio.c
> @@ -342,6 +342,8 @@ struct ofono_handsfree_card *ofono_handsfree_card_create(unsigned int vendor,
>
>   	card = g_new0(struct ofono_handsfree_card, 1);
>
> +	card->selected_codec = HFP_CODEC_CVSD;
> +
>   	card_list = g_slist_prepend(card_list, card);
>
>   	for (l = drivers; l; l = l->next) {

Please merge this patch with 'set_codec' implementation.

Regards,
-Denis

Re: [PATCH v2 7/7] handsfree-audio: Enable wideband speech if defer is enabled

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1997 bytes --]

Hi Vinicius,

On 04/10/2013 02:37 PM, Vinicius Costa Gomes wrote:
> From: Claudio Takahasi<claudio.takahasi@openbossa.org>
>
> Only enable support for wideband speech codecs, at this point mSBC, if
> the kernel supports defer setup.
> ---
>   src/handsfree-audio.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
> index a332f68..da8e100 100644
> --- a/src/handsfree-audio.c
> +++ b/src/handsfree-audio.c
> @@ -66,6 +66,7 @@ static GSList *card_list = 0;
>   static guint sco_watch = 0;
>   static GSList *drivers = 0;
>   static ofono_bool_t has_wideband = FALSE;
> +static int defer_setup = 1;
>
>   static void send_new_connection(const char *card, int fd, uint8_t codec)
>   {
> @@ -159,7 +160,7 @@ static int sco_init(void)
>   {
>   	GIOChannel *sco_io;
>   	struct sockaddr_sco saddr;
> -	int sk, defer_setup = 1;
> +	int sk;
>
>   	sk = socket(PF_BLUETOOTH, SOCK_SEQPACKET | O_NONBLOCK | SOCK_CLOEXEC,
>   								BTPROTO_SCO);
> @@ -177,9 +178,11 @@ static int sco_init(void)
>   	}
>
>   	if (setsockopt(sk, SOL_BLUETOOTH, BT_DEFER_SETUP,
> -				&defer_setup, sizeof(defer_setup))<  0)
> +				&defer_setup, sizeof(defer_setup))<  0) {
> +		defer_setup = 0;
>   		ofono_warn("Can't enable deferred setup: %s (%d)",
>   						strerror(errno), errno);
> +	}
>
>   	if (listen(sk, 5)<  0) {
>   		close(sk);
> @@ -656,6 +659,9 @@ static DBusMessage *am_agent_register(DBusConnection *conn,
>   			has_cvsd = TRUE;
>   		else if (codecs[i] != HFP_CODEC_MSBC)
>   			return __ofono_error_invalid_args(msg);
> +
> +		if (defer_setup)
> +			has_wideband = TRUE;

This patch seems to have unintended side-effects and is in general hard 
to read.

How about something like:

if (has_wideband && !defer_setup)
	has_wideband = FALSE;

outside of the for loop.  Other ideas welcome as well.

>   	}
>
>   	if (has_cvsd == FALSE) {

Regards,
-Denis

Re: [PATCH v2 1/7] hfpmodem: Watch for changes in the selected codec

[Vinicius Costa Gomes]

[-- Attachment #1: Type: text/plain, Size: 2899 bytes --]

Hi Denis,

On 18:07 Fri 12 Apr, Denis Kenzior wrote:
> Hi Vinicius,
> 
> On 04/10/2013 02:37 PM, Vinicius Costa Gomes wrote:
> >This patch adds a function to monitor when the AG sends a new codec
> >before establishing the SCO connection.
> >---
> >  plugins/hfp_hf_bluez5.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 51 insertions(+)
> >
> >diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
> >index c63b1a2..efbb209 100644
> >--- a/plugins/hfp_hf_bluez5.c
> >+++ b/plugins/hfp_hf_bluez5.c
> >@@ -314,9 +314,60 @@ static struct ofono_modem_driver hfp_driver = {
> >  	.post_sim	= hfp_post_sim,
> >  };
> >
> >+static void bcs_notify(GAtResult *result, gpointer user_data)
> >+{
> >+	struct hfp *hfp = user_data;
> >+	struct hfp_slc_info *info =&hfp->info;
> >+	GAtResultIter iter;
> >+	char str[32];
> >+	int value;
> >+
> >+	g_at_result_iter_init(&iter, result);
> >+
> >+	if (!g_at_result_iter_next(&iter, "+BCS:"))
> >+		return;
> >+
> >+	if (!g_at_result_iter_next_number(&iter,&value))
> >+		return;
> >+
> >+	memset(str, 0, sizeof(str));
> 
> Why do you need this exactly?

I don't. Will fix.

> 
> >+
> >+	switch (value) {
> >+	case HFP_CODEC_MSBC:
> >+		if (!ofono_handsfree_audio_has_wideband()) {
> >+			/* No wideband speech support, so we only have CVSD */
> >+			sprintf(str, "AT+BAC=%d", HFP_CODEC_CVSD);
> >+			break;
> >+		}
> >+
> >+		/* fall through */
> >+
> 
> This fall through and the default statement make this code extremely
> hard to read.  Why don't we move the codec selection logic into the
> core instead?
> 
> Then this becomes something like:
> 
> if (ofono_handsfree_audio_card_set_codec(card, value) == FALSE) {
> 	if (ofono_handsfree_audio_has_wideband())
> 		...
> 	else
> 		...
> 
> 	goto done;
> }
> 
> sprintf(str, "AT+BCS=%d", value);
> 
> done:
> ....

Sounds good. Let's see how it ends up looking.

> 
> 
> >+	case HFP_CODEC_CVSD:
> >+		/* Confirm the codec */
> >+		sprintf(str, "AT+BCS=%d", value);
> >+		break;
> >+
> >+	default:
> >+		/* Unsupported codec, re-send our codecs */
> >+		if (ofono_handsfree_audio_has_wideband())
> >+			sprintf(str, "AT+BAC=%d,%d", HFP_CODEC_CVSD,
> >+							HFP_CODEC_MSBC);
> >+		else
> >+			sprintf(str, "AT+BAC=%d", HFP_CODEC_CVSD);
> >+	}
> >+
> >+	g_at_chat_send(info->chat, str, NULL, NULL, NULL, NULL);
> >+}
> >+
> >  static int hfp16_card_probe(struct ofono_handsfree_card *card,
> >  					unsigned int vendor, void *data)
> >  {
> >+	struct hfp *hfp = data;
> >+	struct hfp_slc_info *info =&hfp->info;
> >+
> >+	g_at_chat_register(info->chat, "+BCS:", bcs_notify, FALSE,
> >+								hfp, NULL);
> >+
> 
> You read my mind, I was going to suggest doing this exactly :)
> 
> >  	return 0;
> >  }
> >
> 
> Regards,
> -Denis


Cheers,
-- 
Vinicius

Re: [PATCH v2 2/7] include: ofono_handsfree_card_select_codec()

[Vinicius Costa Gomes]

[-- Attachment #1: Type: text/plain, Size: 1411 bytes --]

Hi Denis,

On 18:12 Fri 12 Apr, Denis Kenzior wrote:
> Hi Vinicius,
> 
> On 04/10/2013 02:37 PM, Vinicius Costa Gomes wrote:
> >This will be used by the drivers that a given codec was negotiated
> >for a card.
> >---
> >  include/handsfree-audio.h | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> >diff --git a/include/handsfree-audio.h b/include/handsfree-audio.h
> >index 82d1312..2e58c43 100644
> >--- a/include/handsfree-audio.h
> >+++ b/include/handsfree-audio.h
> >@@ -48,6 +48,8 @@ struct ofono_handsfree_card *ofono_handsfree_card_create(unsigned int vendor,
> >  							void *data);
> >  int ofono_handsfree_card_register(struct ofono_handsfree_card *card);
> >  void ofono_handsfree_card_remove(struct ofono_handsfree_card *card);
> >+void ofono_handsfree_card_select_codec(struct ofono_handsfree_card *card,
> >+							unsigned char codec);
> 
> As mentioned before, lets make this return ofono_bool_t and name it
> 'set_codec'.
> 
> Also, do we have some idea whether codec ids are limited to 8 bits
> or they can be larger?  My reading of the spec would seem to
> indicate 8 bits maximum.

That is also my reading. From the spec, section 4.34.1, page 92: "The format
of the Codec IDs is 8 bit aliases that are defined in section 12".

> 
> >
> >  ofono_bool_t ofono_handsfree_audio_has_wideband(void);
> >
> 
> Regards,
> -Denis


Cheers,
-- 
Vinicius