[Tarako][FM][AudioChannel] No sound from the earphone, BT headset or phone after answering incoming call and FM app gets killed

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: bli, Assigned: jesse.ji)

Tracking

unspecified
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:1.3T+, b2g-v1.3T fixed)

Details

(Whiteboard: [POVB][depend992478][tarako_only])

Attachments

(1 attachment, 2 obsolete attachments)

Environment:
------------------
Gaia 14ef4fcdf9199f04f7678755c917dc77f51e13ba
Gecko https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/b574a7967338
BuildID 20140329004002
Version 28.1
ro.build.version.incremental=70
ro.build.date=Fri Mar 28 06:17:40 CST 2014


Steps to reproduce:
----------------------------
1. Plugin a earphone
2. Launch FM
3. Pair a BT headset
4. Make a call to the test device
5. Answer the incoming call

Actual results:
-------------------------
No sound is heard from the earphone, BT headset or the phone.
Summary: [Tarako][Bluetooth][Headset] No sound from the earphone, BT headset or phone after answering incoming call while the FM is on → [Tarako][Bluetooth][Headset] No sound from the earphone, BT headset or phone after answering incoming call when earphone and BT headset are both connected to the phone
(In reply to Bingqing Li from comment #0)

> Steps to reproduce:
> ----------------------------
> 
> 2. Launch FM
Step 2 can be skipped.
blocking-b2g: --- → 1.3T?
ni? Eric
Flags: needinfo?(echou)
blocking-b2g: 1.3T? → 1.3T+
:jammink is going to follow up here on the right build being used. We expect QA to use the pvt build for testing not sure where this build is coming from.
blocking-b2g: 1.3T+ → 1.3?
Flags: needinfo?(jhammink)
I'm trying this on the latest PVT Build (see below) and, as far as I can tell, it works as expected.  Here is what I am seeing:

1.) When BT, Earphone are both connected:
Audio comes through BT.  BT supercedes Earphone and built-in speaker, so no audio there.

2.) When earphone is connected (no BT):
Audio comes in through Earphone.  But earphone supercedes built-in speaker, so no audio there.

3.) When nothing is connected:
Audio comes, as expected, through built-in speaker.

The audio is horrible, but that's another issue.  :-/


Gaia      b4ed71d94c495d690ceaa43033c74d4249d5b426
Gecko     8c06de2739ed27e84b70100a6a0c33e8278eba1d
BuildID   20140402145142
Version   28.1
ro.build.version.incremental=78
ro.build.date=Wed Apr  2 15:30:37 CST 2014
Flags: needinfo?(jhammink)
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → WORKSFORME
blocking-b2g: 1.3? → ---
Thanks to John's verification. Clear my ni.
Flags: needinfo?(echou)
bluetooh depend on setting app and setting app is killed by LMK.

20140331151052/kernel/kernel.log:03-31 15:11:51.108 <4>0[   63.912208] select 430 (Settings), adj 10, size 6439, to kill
20140331151052/kernel/kernel.log:03-31 15:11:51.108 <4>0[   63.912276] send sigkill to 430 (Settings), adj 10, size 6439
20140331151052/kernel/kernel.log:03-31 15:13:41.948 <4>0[  174.750050] select 508 (Settings), adj 10, size 6515, to kill
20140331151052/kernel/kernel.log:03-31 15:13:41.948 <4>0[  174.750069] send sigkill to 508 (Settings), adj 10, size 6515
20140331152039/android/main.log:03-31 15:29:59.371  1604  1604 I log     : <4>0[ 1152.067080] select 1479 (Settings), adj 10, size 7030, to kill 
20140331152039/android/main.log:03-31 15:30:06.121  1656  1656 I log     : <4>0[ 1158.897022] select 1631 (Settings), adj 10, size 4615, to kill
Status: RESOLVED → REOPENED
blocking-b2g: --- → 1.3T?
Flags: needinfo?(fabrice)
Resolution: WORKSFORME → ---
(In reply to James Zhang from comment #6)
> bluetooh depend on setting app and setting app is killed by LMK.
> 
> 20140331151052/kernel/kernel.log:03-31 15:11:51.108 <4>0[   63.912208]
> select 430 (Settings), adj 10, size 6439, to kill
> 20140331151052/kernel/kernel.log:03-31 15:11:51.108 <4>0[   63.912276] send
> sigkill to 430 (Settings), adj 10, size 6439
> 20140331151052/kernel/kernel.log:03-31 15:13:41.948 <4>0[  174.750050]
> select 508 (Settings), adj 10, size 6515, to kill
> 20140331151052/kernel/kernel.log:03-31 15:13:41.948 <4>0[  174.750069] send
> sigkill to 508 (Settings), adj 10, size 6515
> 20140331152039/android/main.log:03-31 15:29:59.371  1604  1604 I log     :
> <4>0[ 1152.067080] select 1479 (Settings), adj 10, size 7030, to kill 
> 20140331152039/android/main.log:03-31 15:30:06.121  1656  1656 I log     :
> <4>0[ 1158.897022] select 1631 (Settings), adj 10, size 4615, to kill

James, whats the end user impact here? thanks
Flags: needinfo?(fabrice) → needinfo?(james.zhang)
Bluetooth disconnect and audio channel will be changed.
Flags: needinfo?(james.zhang)
I suggest bluetooth don't depend on setting apps, move it to system apps.
Flags: needinfo?(fabrice)
Assignee: nobody → jesse.ji
Whiteboard: [POVB]
James,

FM app uses SetForceUse to open DEVICE_OUT_FM_HEADSET, it will not be released in incoming call. In this case, audio HAL still get stuck in fm headset device. And audio driver seems not support DEVICE_OUT_FM_HEADSET in phone call mode, so there is no voice come out.


Thanks
triage: 1.3T+, [POVB]
blocking-b2g: 1.3T? → 1.3T+
(In reply to Bingqing Li from comment #1)
> (In reply to Bingqing Li from comment #0)
> 
> > Steps to reproduce:
> > ----------------------------
> > 
> > 2. Launch FM
> Step 2 can be skipped.

I tried on build 20140406164001, step 2 cannot be skipped.

That means the steps to reproduce this issue should be like this:
1. Plugin a earphone
2. Launch FM
3. Pair a BT headset
4. Make a call to the test device
5. Answer the incoming call
Meanwhile I found that when the regular earphone and BT headset are both connected, launch FM, and the sound comes through the BT headset.

Buid Info:
---------------------------------------------------
Gaia      fbf194db193f12b0db070c87bc99f6bb42e28dc0
Gecko     https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/7e9fbc8e7e2b
BuildID   20140406164001
Version   28.1
ro.build.version.incremental=eng.cltbld.20140406.203655
ro.build.date=Sun Apr  6 20:37:02 EDT 2014
The settings app is only used for the first time bluetooth pairing. The system app takes over for subsequent pairings if I recall correctly, which sounds ok to me.
Flags: needinfo?(fabrice)
How come this hasn't been updated for a week now?
(In reply to James Zhang from comment #6)
> bluetooh depend on setting app and setting app is killed by LMK.
> 
> 20140331151052/kernel/kernel.log:03-31 15:11:51.108 <4>0[   63.912208]
> select 430 (Settings), adj 10, size 6439, to kill
> 20140331151052/kernel/kernel.log:03-31 15:11:51.108 <4>0[   63.912276] send
> sigkill to 430 (Settings), adj 10, size 6439
> 20140331151052/kernel/kernel.log:03-31 15:13:41.948 <4>0[  174.750050]
> select 508 (Settings), adj 10, size 6515, to kill
> 20140331151052/kernel/kernel.log:03-31 15:13:41.948 <4>0[  174.750069] send
> sigkill to 508 (Settings), adj 10, size 6515
> 20140331152039/android/main.log:03-31 15:29:59.371  1604  1604 I log     :
> <4>0[ 1152.067080] select 1479 (Settings), adj 10, size 7030, to kill 
> 20140331152039/android/main.log:03-31 15:30:06.121  1656  1656 I log     :
> <4>0[ 1158.897022] select 1631 (Settings), adj 10, size 4615, to kill

James,

I don't think this issue is related to the Settings being killed. Just like Fabrice mentioned in comment 14, once Bluetooth headset has paired or connected, even system/settings apps gets killed does not affect the Bluetooth connection.
Summary: [Tarako][Bluetooth][Headset] No sound from the earphone, BT headset or phone after answering incoming call when earphone and BT headset are both connected to the phone → [Tarako][FM][AudioChannel] No sound from the earphone, BT headset or phone after answering incoming call when earphone and BT headset are both connected to the phone
Summary: [Tarako][FM][AudioChannel] No sound from the earphone, BT headset or phone after answering incoming call when earphone and BT headset are both connected to the phone → [Tarako][FM][AudioChannel] No sound from the earphone, BT headset or phone after answering incoming call and FM app gets killed
(In reply to James Zhang from comment #8)
> Bluetooth disconnect and audio channel will be changed.

This is not correct. I followed the STR and I didn't see Bluetooth connection dropped. The symptom is more like "after answering the call, FM app gets killed and nobody release the audio path for FM app".

James,

Please try to reproduce and see if my observaion is correct.

Wayne,

I saw you left a comment (comment 10) and that was close to what I realized. Do you think this should be a POVB or this is something Gecko should deal with?

John,

In comment 4 you said you couldn't reproduce, but I just tried by following the STR and I found it was reproduciable. Would you mind giving another try?

Thanks.
Flags: needinfo?(waychen)
Flags: needinfo?(jhammink)
Flags: needinfo?(james.zhang)
(In reply to Eric Chou [:ericchou] [:echou] from comment #17)
> (In reply to James Zhang from comment #8)
> > Bluetooth disconnect and audio channel will be changed.
> 
> This is not correct. I followed the STR and I didn't see Bluetooth
> connection dropped. The symptom is more like "after answering the call, FM
> app gets killed and nobody release the audio path for FM app".
> 
> James,
> 
> Please try to reproduce and see if my observaion is correct.
> 
> Wayne,
> 
> I saw you left a comment (comment 10) and that was close to what I realized.
> Do you think this should be a POVB or this is something Gecko should deal
> with?
> 
> John,
> 
> In comment 4 you said you couldn't reproduce, but I just tried by following
> the STR and I found it was reproduciable. Would you mind giving another try?
> 
> Thanks.

Hi Eric,
At first, I thought it could be fixed by SPRD because FM app needs to close itself when incoming call. But if FM app be killed, no one can release the driver settings. I think Bug 992478 comment #52 is a good solution, however I'm not sure it's still POVB or Gecko.
Flags: needinfo?(waychen)
Jesse, when can you land your FM patch on v1.3t?
Flags: needinfo?(james.zhang)
Depends on: 992478
Please review bug 992478 jesse's patch.
Whiteboard: [POVB] → [POVB][depend992478]
sorry for the delay - trying again on 

Gaia      3771067de006633df690a590a97b4d28c44ef8b2
Gecko     https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/51a4e4d35e30
BuildID   20140423164005
Version   28.1

I'm getting sound call audio from each of the headphones, and phone speaker.  The BT is flaky and disconnects too quickly and often to test reliably.
Flags: needinfo?(jhammink)
This issue is still pending because the patch of bug 992478 hasn't passed the review yet. This is expected to be fixed if the resources which are held by FM app could be released when FM app is killed, which is the problem that bug 992478 is going to solve.
Well, the root cause of this bug is our special hardware design(BT+FM), we need to manually change audio route(pcm/i2s) in driver after enabling or disabling FM audio, otherwise there will be an audio device conflict.
Attachment #8413608 - Flags: review?(pzhang)
Comment on attachment 8413608 [details] [diff] [review]
no_sound_from_bt_if_fm_audio_enabled.patch

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

Most of the change are gonk specific, I'm not familiar with gonk, sorry.

Let's ask @Marco for help.
Attachment #8413608 - Flags: review?(pzhang) → review?(mchen)
Comment on attachment 8413608 [details] [diff] [review]
no_sound_from_bt_if_fm_audio_enabled.patch

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

Feedback+ for V1.3T branch only. And transfer to :mwu for final review.

::: hal/Hal.cpp
@@ +1026,4 @@
>    PROXY_IF_SANDBOXED(DisableFMRadio());
>  }
>  
> +#ifdef MOZ_B2G

Please remove it.

::: hal/Hal.h
@@ +522,4 @@
>   */
>  void DisableFMRadio();
>  
> +#ifdef MOZ_B2G

Please remove this and add empty functions into gecko/hal/FallbackFMRadio.cpp.

::: hal/gonk/GonkFMRadio.cpp
@@ +27,4 @@
>  #include <sys/stat.h>
>  #include <sys/types.h>
>  
> +#define V4L2_CID_PRIVATE_FM_AUDIO (V4L2_CID_BASE + 10)

For 1.3t only, I think it is find to base from V4L2_CID_BASE.
But for master, it should be from V4L2_CID_PRIVATE_BASE and move it to an individual header.

And finally we need to have a new static variable (or enum to identify different chip) like sMsmFMMode to identify whether we should call which set of V4L2_CID_PRIVATE_TAVARUA/Others_XXX.

@@ +345,5 @@
>  
>  void
> +EnableFMAudio()
> +{
> +  setControl(V4L2_CID_PRIVATE_FM_AUDIO, 1);

int rc = setControl(V4L2_CID_PRIVATE_FM_AUDIO, 1);
if (rc < 0) {
  HAL_LOG(("Unable to enable audio of FMRadio"));
}

@@ +351,5 @@
> +
> +void
> +DisableFMAudio()
> +{
> +  setControl(V4L2_CID_PRIVATE_FM_AUDIO, 0);

The same with line 349.
Attachment #8413608 - Flags: review?(mwu)
Attachment #8413608 - Flags: review?(mchen)
Attachment #8413608 - Flags: feedback+
Status: REOPENED → ASSIGNED
Component: AudioChannel → DOM: Device Interfaces
Product: Firefox OS → Core
Whiteboard: [POVB][depend992478] → [POVB][depend992478][tarako_only]
Comment on attachment 8413608 [details] [diff] [review]
no_sound_from_bt_if_fm_audio_enabled.patch

I believe you can do this all in the tarako audio policy module. See hardware/libhardware_legacy/audio/AudioPolicyManagerBase.cpp - you should be able to integrate this call into FMRadioPlayer::init and FMRadioPlayer::shutdown.
Attachment #8413608 - Flags: review?(mwu) → review-
(In reply to Michael Wu [:mwu] from comment #26)
> Comment on attachment 8413608 [details] [diff] [review]
> no_sound_from_bt_if_fm_audio_enabled.patch
> 
> I believe you can do this all in the tarako audio policy module. See
> hardware/libhardware_legacy/audio/AudioPolicyManagerBase.cpp - you should be
> able to integrate this call into FMRadioPlayer::init and
> FMRadioPlayer::shutdown.

It's confusing if moving FM code to audio policy. Why there can be msm code in GonkFMRadio, but not tarako?
Since it's v1.3t only, so we don't want any additional logics to check whether to call this special code or not.
Attachment #8413608 - Attachment is obsolete: true
Attachment #8414191 - Flags: review?(mchen)
Attachment #8414191 - Attachment is obsolete: true
Attachment #8414191 - Flags: review?(mchen)
Flags: needinfo?(ttsai)
Flags: needinfo?(pzhang)
Flags: needinfo?(mwu)
(In reply to Michael Wu [:mwu] from comment #26)
> I believe you can do this all in the tarako audio policy module. See
> hardware/libhardware_legacy/audio/AudioPolicyManagerBase.cpp - you should be
> able to integrate this call into FMRadioPlayer::init and
> FMRadioPlayer::shutdown.

Hi Michael,

I had considered to put it into Audio HAL but finally I rejected myself.
The reason is that I think Audio HAL should take care more generic configurations. For example, audio HAL controls the settings and routing path of DSP and Codec not to control each application chips connected to Codec.

For a specific chip like FM, it's own audio control should be made by it's own HAL therefore one audio HAL can be used on platform which can equip different FM Chips. 

May I know it makes sense to you or not?
(In reply to Marco Chen [:mchen] from comment #30)
> I had considered to put it into Audio HAL but finally I rejected myself.
> The reason is that I think Audio HAL should take care more generic
> configurations. For example, audio HAL controls the settings and routing
> path of DSP and Codec not to control each application chips connected to
> Codec.
> 
> For a specific chip like FM, it's own audio control should be made by it's
> own HAL therefore one audio HAL can be used on platform which can equip
> different FM Chips. 
> 
> May I know it makes sense to you or not?

It doesn't sound like you're disagreeing with me. However, the FM HAL is just part of V4L2 + audio HAL. It sounds like you would want a separate FM HAL so the Audio HAL doesn't need to be modified for different FM chips. However, it seems to me that FM audio routing is often very closely tied with the audio configuration, so a tight coupling between these two parts are necessary anyway.

(In reply to Jesse from comment #27)
> (In reply to Michael Wu [:mwu] from comment #26)
> > Comment on attachment 8413608 [details] [diff] [review]
> > no_sound_from_bt_if_fm_audio_enabled.patch
> > 
> > I believe you can do this all in the tarako audio policy module. See
> > hardware/libhardware_legacy/audio/AudioPolicyManagerBase.cpp - you should be
> > able to integrate this call into FMRadioPlayer::init and
> > FMRadioPlayer::shutdown.
> 
> It's confusing if moving FM code to audio policy. Why there can be msm code
> in GonkFMRadio, but not tarako?

The msm code is a mistake. We want to make sure that our FM code requires as little hardware specific quirk handling as necessary, and make sure that the current V4L2 + audio HAL combination can account for all FM radio use cases. The audio policy code *already* handles FM specific issues so that location actually makes more sense to me, especially if this is a FM audio routing issue.
Flags: needinfo?(mwu)
Hi Michael,

> The audio policy code *already* handles FM specific issues so that location actually makes more sense 
> to me, especially if this is a FM audio routing issue.

Definition: Audio HAL is "Audio HW + Audio Policy" module.

-Each Chip HAL- | ----- Audio HAL -----------

FM chip  ---
           |
BT chip  -----------> Codec --> output device
           |
DTV chip ---

1. I don't think it is a FM audio "Routing" issue. Normally the routing issue is caused by runtime configuration of DSP and Codec. Which is done by Audio HAL. So if it is really a routing issue then I agree with your suggestion here. But it seems that the current patch is used to control chip level only.

2. The current patch show that it want to control the FM Chip itself with enabling/disabling audio output. It is not related to "Routing" so it should be no business with Audio HAL.

3. For me, GonkFMRadio.cpp (using V4L2) is a kind of FM HAL therefore it should take care all stuffs related to FM itself. ex: whether FM chip needs to output audio or not.

4. I don't think that Audio HAL should control every chips which can output audio into Codec.
   Or I can't image we need hook their own configurations into Audio HAL for FM, BT and DTV chips.

I raised my point here but leave the decision to Michael who wrote most of codes in GonkFMRadio.cpp and even contribute FM driver.
I think we just want to work around this bug, tarako only.
Jesse try to modify hardware git repo but hasn't better solution.
Flags: needinfo?(styang)
Our next chipset dolphin need not this patch, so it's for tarako only.
Fabrice, please help to land jesse's patch on v1.3t branch, we have verified it on my side and pass test case.
It's for tarako only. We don't need land it on master branch.
Flags: needinfo?(fabrice)
https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/e0b2c0b2d1a1
Status: ASSIGNED → RESOLVED
Closed: 5 years ago5 years ago
Flags: needinfo?(fabrice)
Resolution: --- → FIXED
Thank you for tarako only workaround.
Flags: needinfo?(styang)
Flags: needinfo?(pzhang)
Flags: needinfo?(ttsai)
You need to log in before you can comment on or make changes to this bug.