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

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Bingqing Li]

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.

Comment 1

[Bingqing Li]

(In reply to Bingqing Li from comment #0)

> Steps to reproduce:
> ----------------------------
> 
> 2. Launch FM
Step 2 can be skipped.

Comment 2

[Joe Cheng [:jcheng] (please needinfo)]

ni? Eric

Comment 3

[bhavana bajaj [:bajaj]]

: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.

Comment 4

[John Hammink]

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

Comment 5

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

Thanks to John's verification. Clear my ni.

Comment 6

[James Zhang (Spreadtrum)]

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

Comment 7

[Joe Cheng [:jcheng] (please needinfo)]

(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

Comment 8

[James Zhang (Spreadtrum)]

Bluetooth disconnect and audio channel will be changed.

Comment 9

[James Zhang (Spreadtrum)]

I suggest bluetooth don't depend on setting apps, move it to system apps.

Comment 10

[Wayne Chen [:xwaynec]]

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

Comment 11

[Joe Cheng [:jcheng] (please needinfo)]

triage: 1.3T+, [POVB]

Comment 12

[Bingqing Li]

(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

Comment 13

[Bingqing Li]

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

Comment 14

[[:fabrice] Fabrice Desré]

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.

Comment 15

[Faramarz Rashed]

How come this hasn't been updated for a week now?

Comment 16

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

(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.

Comment 17

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

(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.

Comment 18

[Wayne Chen [:xwaynec]]

(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.

Comment 19

[James Zhang (Spreadtrum)]

Jesse, when can you land your FM patch on v1.3t?

Comment 20

[James Zhang (Spreadtrum)]

Please review bug 992478 jesse's patch.

Comment 21

[John Hammink]

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.

Comment 22

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

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.

Comment 23

[Jesse]

Attachment: no_sound_from_bt_if_fm_audio_enabled.patch

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.

Comment 24

[Pin Zhang [:pzhang] (inactive)]

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.

Comment 25

[Marco Chen [: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.

Comment 26

[Michael Wu [:mwu]]

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.

Comment 27

[Jesse]

(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?

Comment 28

[Jesse]

Attachment: no_sound_from_bt_if_fm_audio_enabled.patch

Since it's v1.3t only, so we don't want any additional logics to check whether to call this special code or not.

Comment 29

[Jesse]

Attachment: no_sound_from_bt_if_fm_audio_enabled.patch

Comment 30

[Marco Chen [:mchen]]

(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?

Comment 31

[Michael Wu [:mwu]]

(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.

Comment 32

[Marco Chen [:mchen]]

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.

Comment 33

[James Zhang (Spreadtrum)]

I think we just want to work around this bug, tarako only.
Jesse try to modify hardware git repo but hasn't better solution.

Comment 34

[James Zhang (Spreadtrum)]

Our next chipset dolphin need not this patch, so it's for tarako only.

Comment 35

[James Zhang (Spreadtrum)]

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.

Comment 36

[[:fabrice] Fabrice Desré]

https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/e0b2c0b2d1a1

Comment 37

[James Zhang (Spreadtrum)]

Thank you for tarako only workaround.