HFP 1.6 optional item: Wideband Speech

RESOLVED FIXED

Status

Firefox OS
Bluetooth
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: ericchou, Assigned: WillWang)

Tracking

unspecified
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(feature-b2g:2.2r+, b2g-v2.2r fixed)

Details

(Whiteboard: [red-tai][ETA=8/31])

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

5 years ago
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.
(Reporter)

Updated

5 years ago
Blocks: 879196
(Reporter)

Updated

5 years ago
No longer blocks: 879196
(Reporter)

Updated

5 years ago
Blocks: 879196
Assignee: nobody → wiwang
Whiteboard: [red-tai]
Hi Will,
Please make sure WBS can correctly work on 8909 platform.
(Assignee)

Updated

3 years ago
Blocks: 1181901
feature-b2g: --- → 2.2r+
Whiteboard: [red-tai] → [red-tai][ETA=8/31]
(Assignee)

Updated

3 years ago
Depends on: 1189315
(Assignee)

Comment 2

3 years ago
Created attachment 8655370 [details] [diff] [review]
0001-WBS-git-patch.patch

Hi Shawn

Could you help to review this patch?
Thanks!
Attachment #8655370 - Flags: review?(shuang)
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.
Attachment #8655370 - Flags: review?(shuang) → review-

Comment 4

3 years ago
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.
(Assignee)

Comment 5

3 years ago
Created attachment 8656523 [details] [diff] [review]
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!
Attachment #8655370 - Attachment is obsolete: true
Attachment #8656523 - Flags: review?(shuang)
(Assignee)

Comment 6

3 years ago
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 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?
Attachment #8656523 - Flags: review?(shuang) → review+
(Assignee)

Comment 8

3 years ago
Created attachment 8658113 [details] [diff] [review]
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!
Attachment #8656523 - Attachment is obsolete: true
Attachment #8658113 - Flags: review?(shuang)
(Assignee)

Comment 9

3 years ago
Also, above patch move declaration into BT_DECL_HFP_MGR_BASE as Bug 1197815 did.
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.
Attachment #8658113 - Flags: review?(shuang) → review+
(Assignee)

Comment 11

3 years ago
Created attachment 8658642 [details] [diff] [review]
(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.
Attachment #8658113 - Attachment is obsolete: true
Attachment #8658642 - Flags: review+
(Assignee)

Comment 12

3 years ago
Hi Wesley

Could you help to handle this for branch 2.2r,
thanks!
blocking-b2g: --- → 2.2r?
status-b2g-v2.2r: --- → affected
Flags: needinfo?(whuang)
Keywords: checkin-needed
it has been flagged with feature-b2g:2.2r and check-in needed. 
Sheriff will uplift it.
blocking-b2g: 2.2r? → ---
Flags: needinfo?(whuang)
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2r/rev/86954a963254
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-b2g-v2.2r: affected → fixed
Keywords: checkin-needed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.