Closed Bug 878728 Opened 11 years ago Closed 11 years ago

Support in-band ringtone

Categories

(Firefox OS Graveyard :: Bluetooth, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: echou, Assigned: ben.tian)

Details

(Whiteboard: [fixed-in-birch])

Attachments

(1 file, 2 obsolete files)

In-band ringtone is an optional feature defined in HFP sepc 1.5 & 1.6. Once it is done, the connected remote headset could hear the ringtone sent from FFOS phone.

To implement this feature, several things need to be done:

(1) Modify BRSF value
(2) Change the timing of initializing SCO connection. The whole process is defined in section 4.13.1 "Answer Incoming Call from the HF – In-Band Ringing"
(3) Implement result code: BSIR for further use

This is a good-to-have feature.
Assignee: nobody → btian
No longer blocks: 878745
The patch implements in-band ring tone in dom bluetooth but requires OEM implement BT SCO+SPEAKER ouput audio path in audio driver. OEM can enable BSIR by setting mBSIR=true in function BluetoothHfpManager::Reset(). 

Also the patch defines BRSF bitmasks for AG supported features.
Attachment #758361 - Flags: review?(echou)
Comment on attachment 758361 [details] [diff] [review]
Implement in-band ring tone and BRSF bitmask

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

Overall looks good. Please have me review again after revision. Thanks.

::: dom/bluetooth/BluetoothHfpManager.cpp
@@ +38,5 @@
> + */
> +#define BRSF_BIT_THREE_WAY_CALLING        1
> +#define BRSF_BIT_IN_BAND_RING_TONE        (1 << 3)
> +#define BRSF_BIT_ABILITY_TO_REJECT_CALL   (1 << 5)
> +#define BRSF_BIT_ENHANCED_CALL_STATUS     (1 << 6)

This is great. Let's complete all feature bits.

@@ +726,5 @@
>  
>    // For more information, please refer to 4.34.1 "Bluetooth Defined AT
>    // Capabilities" in Bluetooth hands-free profile 1.6
>    if (msg.Find("AT+BRSF=") != -1) {
> +    uint8_t brsf = BRSF_BIT_THREE_WAY_CALLING |

According to spec, BRSF is actually a 32-bit unsigned integer bitmap. So please use uint32_t. In addition, you'll need to revise BluetoothHfpManager::SendCommand() because the second argument is uint8_t now.
Add bitmask definition and revise BluetoothHfpManager::SendCommand().
Attachment #758361 - Attachment is obsolete: true
Attachment #758361 - Flags: review?(echou)
Attachment #758389 - Flags: review?(echou)
Comment on attachment 758389 [details] [diff] [review]
Implement in-band ring tone and BRSF bitmask v2

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

Almost there. Happy to review again once it's done. :)

::: dom/bluetooth/BluetoothHfpManager.cpp
@@ +43,5 @@
> +#define BRSF_BIT_ATTACH_NUM_TO_VOICE_TAG  (1 << 4)
> +#define BRSF_BIT_ABILITY_TO_REJECT_CALL   (1 << 5)
> +#define BRSF_BIT_ENHANCED_CALL_STATUS     (1 << 6)
> +#define BRSF_BIT_ENHANCED_CALL_CONTROL    (1 << 7)
> +#define BRSF_BIT_EXTENDED_ERR_RESULT_CODE (1 << 8)

nit: CODE'S'

@@ +733,5 @@
>    // For more information, please refer to 4.34.1 "Bluetooth Defined AT
>    // Capabilities" in Bluetooth hands-free profile 1.6
>    if (msg.Find("AT+BRSF=") != -1) {
> +    uint32_t brsf = BRSF_BIT_THREE_WAY_CALLING |
> +                   BRSF_BIT_ABILITY_TO_REJECT_CALL |

nit: weird alignment

@@ +1138,5 @@
>      return false;
>    }
>  
>    nsAutoCString message;
>    int value = aValue;

This variable 'value' is only used once in the function. Could you assist with removing it?
Remove variable 'value' in SendCommand() and revise naming and code alignment.
Attachment #758389 - Attachment is obsolete: true
Attachment #758389 - Flags: review?(echou)
Attachment #758449 - Flags: review?(echou)
Comment on attachment 758449 [details] [diff] [review]
Implement in-band ring tone and BRSF bitmask v3

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

Thanks!
Attachment #758449 - Flags: review?(echou) → review+
birch: https://hg.mozilla.org/projects/birch/rev/c24bbcfd3d53
Whiteboard: [fixed-in-birch]
https://hg.mozilla.org/mozilla-central/rev/c24bbcfd3d53
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: