911635 - HFP 1.6 optional item: Wideband Speech

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Description

[Eric Chou [:ericchou] [:echou]]

We have implemented mandatory items for HFP 1.6 (bug 879635), and now we can start to see if it's possible to support Wideband Speech in the next version.

Comment 1

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Hi Will,
Please make sure WBS can correctly work on 8909 platform.

Comment 2

[Will Wang [:WillWang]]

Attachment: 0001-WBS-git-patch.patch

Hi Shawn

Could you help to review this patch?
Thanks!

Comment 3

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Comment on attachment 8655370 [details] [diff] [review]
0001-WBS-git-patch.patch

Review of attachment 8655370 [details] [diff] [review]:
-----------------------------------------------------------------

You missed the change for fallback folder.

::: dom/bluetooth/bluedroid/b2g_bdroid_buildcfg.h
@@ +35,4 @@
>                               BTA_AG_FEAT_ECNR | \
>                               BTA_AG_FEAT_REJECT | \
>                               BTA_AG_FEAT_ECS    | \
> +                             BTA_AG_FEAT_CODEC  | \

This file will be removed. You can consult with tzimmermann to land the configuration into another repository.

::: dom/bluetooth/bluedroid/hfp/BluetoothHfpManager.cpp
@@ +1516,5 @@
> +  nsCOMPtr<nsIObserverService> obs = services::GetObserverService();
> +  NS_ENSURE_TRUE_VOID(obs);
> +
> +  if (HFP_WBS_YES == aWbs) {
> +  mWbsEnabled = true;

nit: Fix the coding style.

@@ +1518,5 @@
> +
> +  if (HFP_WBS_YES == aWbs) {
> +  mWbsEnabled = true;
> +  } else {
> +  mWbsEnabled = false;

Ditto.

::: dom/system/gonk/AudioManager.cpp
@@ +392,5 @@
> +          AudioSystem::setParameters(0, cmd);
> +      } else {
> +          cmd.setTo("bt_wbs=off");
> +          AudioSystem::setParameters(0, cmd);
> +      }

nit: Fix the indentation.

Comment 4

[Ben Tian (inactive)]

Comment on attachment 8655370 [details] [diff] [review]
0001-WBS-git-patch.patch

Review of attachment 8655370 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bluetooth/bluedroid/hfp/BluetoothHfpManager.cpp
@@ +1515,5 @@
> +  // Notify Gecko observers
> +  nsCOMPtr<nsIObserverService> obs = services::GetObserverService();
> +  NS_ENSURE_TRUE_VOID(obs);
> +
> +  if (HFP_WBS_YES == aWbs) {

Simplify as

  mWbsEnabled = (aWbs == HFP_WBS_YES);

@@ +1527,5 @@
> +                                     BLUETOOTH_HFP_WBS_STATUS_CHANGED_ID,
> +                                     mDeviceAddress.get()))) {
> +    BT_WARNING("Failed to notify bluetooth-hfp-wbs-status-changed observsers!");
> +  }
> +

nit: remove this extra newline.

::: dom/bluetooth/bluedroid/hfp/BluetoothHfpManager.h
@@ +131,5 @@
>                          const nsAString& aBdAddress) override;
>    void NRECNotification(BluetoothHandsfreeNRECState aNrec,
>                          const nsAString& aBdAddr) override;
> +  void WbsNotification(BluetoothHandsfreeWbsConfig aWbs,
> +                  const nsAString& aBdAddr) override;

nit: alignment.

Comment 5

[Will Wang [:WillWang]]

Attachment: Patch: Add WBS support for branch 2.2r

Thank you Shawn, Ben.

Hi Shawn,

Could you help to review again this patch?
I rebased it to branch 2.2r, and ran the PTS test TC_AG_WBS_BV_01_I,
Thanks!

Comment 6

[Will Wang [:WillWang]]

Note:

To enable WBS, a define of stack bluedroid named BTIF_HF_WBS_PREFERRED[1] must be TRUE,
then the preferred codec will be overridden as following:
09-03 07:26:44.202 I/bt-btif ( 1458): btif_hf_upstreams_evt btif_hf override-Preferred Codec to MSBC

and codec status will be set to MSBC instead of original CVSD:
09-03 07:26:44.202 D/bt-btif ( 1458): BTA_AG_WBS_EVT Set codec status 0 codec 2 1=CVSD 2=MSBC

This will be reflected in my incoming gecko patch.

Besides,
another define BTM_WBS_INCLUDED[2] of bluedroid must be TRUE as well.


[1] file path: external/bluetooth/bluedroid/btif/src/btif_hf.c
[2] file path: external/bluetooth/bluedroid/include/bt_target.h

Comment 7

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Comment on attachment 8656523 [details] [diff] [review]
Patch: Add WBS support for branch 2.2r

Review of attachment 8656523 [details] [diff] [review]:
-----------------------------------------------------------------

Remove PATCH 2/2 from subject, also you should add hfp fallback. Do you plan to add it in this patch or another bug?

Comment 8

[Will Wang [:WillWang]]

Attachment: Patch: Add HFP Wideband Speech(WBS) support. (for branch 2.2r)

Revised patch based on Shawn's review.
- add support for hfp-fallback.
- rebase for another HFP NREC patch in Bug 1197815.


Hi Shawn,

Could you help to review?
Thanks!

Comment 9

[Will Wang [:WillWang]]

Also, above patch move declaration into BT_DECL_HFP_MGR_BASE as Bug 1197815 did.

Comment 10

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Comment on attachment 8658113 [details] [diff] [review]
Patch: Add HFP Wideband Speech(WBS) support. (for branch 2.2r)

Review of attachment 8658113 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/system/gonk/AudioManager.cpp
@@ +367,5 @@
> +    String8 cmd;
> +    BluetoothHfpManagerBase* hfp =
> +      static_cast<BluetoothHfpManagerBase*>(aSubject);
> +    if (hfp->IsNrecEnabled()) {
> +      cmd.setTo("bt_headset_name=<unknown>;bt_headset_nrec=on");

nit: Please add "TODO" note, to fix <unknown> name.

Comment 11

[Will Wang [:WillWang]]

Attachment: (for branch 2.2r) Patch: Add HFP Wideband Speech(WBS) support.

Hi Shawn

Thanks for your review :)
Revised patch is as attached.
- Add TODO comment. (ref: Bug 880785)
- Add a newline \n to end of file for POSIX standard.

Carry r+ from previous patch.

Comment 12

[Will Wang [:WillWang]]

Hi Wesley

Could you help to handle this for branch 2.2r,
thanks!

Comment 13

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

it has been flagged with feature-b2g:2.2r and check-in needed. 
Sheriff will uplift it.

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2r/rev/86954a963254