949855 - No sound comes out when FM Radio is in the foreground and trying to play while Music app is playing in the background

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Attachment: logcat.txt

Description:



Steps to Reproduce:

1. plug in headset
2. reboot phone
3. Play a song in the music app
4. hit the home button while the music is still playing
5. launch FM Radio and tune to a known station

Expected Results:  FM Radio plays station
Actual Results: slight static sound comes out.

Additional Notes:
1. when the music app is shut down, FM Radio plays

Environmental Variables:
Gaia      6d02039072a2ae5cf9225a6f4c78ed49decfab5c
Gecko     http://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/8bae10bb0aed
BuildID   20131212004004
Version   26.0
ro.build.version.incremental=eng.archermind.20131114.105818
ro.build.date=Thu Nov 14 10:58:33 CST 2013
Version: 1.2
Device: Buri
Build Date: 12/12/2013
RIL Type (Mozilla RIL or Commericial RIL): Com Ril

Comment 1

[Jason Smith [:jsmith]]

Does this reproduce on 1.1?

Comment 2

[Marco Chen [:mchen]]

Gaia  3f2550c2fabea0f16e9c743093c4b61103a8dba8
Gecko http://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/129ad3c335a5

I tested it today with the revision above but I can't reproduce this issue.

Comment 3

[Jason Smith [:jsmith]]

(In reply to Marco Chen [:mchen] from comment #2)
> Gaia  3f2550c2fabea0f16e9c743093c4b61103a8dba8
> Gecko http://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/129ad3c335a5
> 
> I tested it today with the revision above but I can't reproduce this issue.

QA Wanted - also check if this reproduces on the latest 1.2 gaia/gecko.

Comment 4

[Roland Kunkel [:RolandK]]

(In reply to Jason Smith [:jsmith] from comment #1)
> Does this reproduce on 1.1?

With the steps provided the issue does not appear to reproduce in the latest Buri 1.1 build.

Buri v1.1
Environmental Variables:
Device: Buri v1.1 COM RIL
BuildID: 20131213041208
Gaia: 6ff3a607f873320d00cb036fa76117f6fadd010f
Gecko: bdac595a4e46
Version: 18.0
RIL Version: 01.01.00.019.281
Firmware Version: V1.2_US_20131115

> QA Wanted - also check if this reproduces on the latest 1.2 gaia/gecko.

With the steps provided the issue was reproduced in the latest Buri 1.2 build. The 'known radio station' would not play until the user toggles the play / stop button twice. Just faint static was played when the user pressed play for the first time.

Buri v1.2
Environmental Variables:
Device: Buri v1.2 COM RIL
BuildID: 20131216004002
Gaia: fcf1c2fe020c29da4755621cbffdc1a333a43be9
Gecko: 129ad3c335a5
Version: 26.0
RIL Version: 01.02.00.019.102
Firmware Version: V1.2_US_20131115

Comment 5

[Jason Smith [:jsmith]]

This sounds like another regression in the audio channels work that happened with the FM Radio in 1.2.

Comment 6

[Marco Chen [:mchen]]

This is not a AudioChannel regression.

>> The 'known radio station' would not play until the user toggles the play / stop button twice.
This behavior didn't need music player to run on the background. You can find this issue after booting up and no necessary to launch any other app.

I also check the behavior of AudioChannel and it is under expectation.

-----------------------------------

Will check into GonkFMRadio.cpp.

Comment 7

[Marco Chen [:mchen]]

This is the issue inside FMRadioService.cpp and related to detect airplan mode.
http://hg.mozilla.org/releases/mozilla-b2g26_v1_2/file/129ad3c335a5/dom/fmradio/FMRadioService.cpp#l435

The normal flow for eqnbling FMRadio is

  1. FMRadioService.cpp calls GonkFMRadio.cpp for enabling FM.
  2. GonkFMRadio.cpp writes hw.fm.analog=true into property system.
  3. FMRadioService.cpp calls EnableAudio() to change routing to DEVICE_OUT_FM.
  4. AudioPolicyManager detects hw.fm.analog=true to do something.

During the first launch of FMRadio, the RilSetting related codes will call step 3 before step 2.
That is why step 4 can't detect hw.fm.analog=true then function is not working.

----

Hi Pin,

Could you help this issue?
Thanks.

Comment 8

[Roland Kunkel [:RolandK]]

(In reply to Jason Smith [:jsmith] from comment #5)

This issue was found to reproduce in the first build for 1.2. 

6/21
Environmental Variables:
Device: Buri v1.2 MOZ RIL
BuildID: 20130621031231
Gaia: e2f19420fa6a26c4287588701efaec09a750dba1
Gecko: 7ba8c86f1a56
Version: 24.0a1
Firmware Version: 2013111

Comment 9

[Pin Zhang [:pzhang] (inactive)]

(In reply to Marco Chen [:mchen] from comment #7)
> This is the issue inside FMRadioService.cpp and related to detect airplan
> mode.
> http://hg.mozilla.org/releases/mozilla-b2g26_v1_2/file/129ad3c335a5/dom/
> fmradio/FMRadioService.cpp#l435
> 
> The normal flow for eqnbling FMRadio is
> 
>   1. FMRadioService.cpp calls GonkFMRadio.cpp for enabling FM.
>   2. GonkFMRadio.cpp writes hw.fm.analog=true into property system.
>   3. FMRadioService.cpp calls EnableAudio() to change routing to
> DEVICE_OUT_FM.
>   4. AudioPolicyManager detects hw.fm.analog=true to do something.
> 
> During the first launch of FMRadio, the RilSetting related codes will call
> step 3 before step 2.
> That is why step 4 can't detect hw.fm.analog=true then function is not
> working.
> 
> ----
> 
> Hi Pin,
> 
> Could you help this issue?
> Thanks.

OK, thanks for your investigation.

Comment 10

[Pin Zhang [:pzhang] (inactive)]

Attachment: Patch

Comment 11

[Marco Chen [:mchen]]

Comment on attachment 8349187 [details] [diff] [review]
Patch

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

Thanks for your help and some questions here.

::: dom/fmradio/FMRadioService.cpp
@@ +729,5 @@
>        }
>  
> +      // Bug 949855: enable audio after the FM radio HW is enabled, to make sure
> +      // 'hw.fm.isAnalog' could be detected as |true| during first time launch.
> +      IFMRadioService::Singleton()->EnableAudio(true);

The context here is already in the instance of FMRadioService, so I think we can call member function directly.

By the way, SetFMRadioFrequency() will be called after here then user may hear the wrong frequency first then correct one.
I suggest to call EnableAudio() after SetFMRadioFrequency() then user experience can be improved.

Comment 12

[Pin Zhang [:pzhang] (inactive)]

Attachment: Patch V2

Comment 13

[Pin Zhang [:pzhang] (inactive)]

Marco, do you know how can we write a test to cover the audio issues, like this bug?

Comment 14

[Marco Chen [:mchen]]

Comment on attachment 8349264 [details] [diff] [review]
Patch V2

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

Thanks for the quick response time here. Great.

r+ after addressing more precise comment for analog audio output.

::: dom/fmradio/FMRadioService.cpp
@@ +736,5 @@
>        SetFMRadioFrequency(mPendingFrequencyInKHz);
>  
> +      // Bug 949855: enable audio after the FM radio HW is enabled, to make sure
> +      // 'hw.fm.isAnalog' could be detected as |true| during first time launch.
> +      EnableAudio(true);

We need to add an additional comment like "This case is for audio output on analog path. (ro.moz.fm.noAnalog is not true)"

Comment 15

[Pin Zhang [:pzhang] (inactive)]

Attachment: Patch for check-in

Add one more comment.

Comment 16

[John Hammink]

This is blocking.   Let's plus it.

Comment 17

[Marcia Knous [:marcia - needinfo? me]]

Adding koi+ per triage.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/65d1f561cce3

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/65d1f561cce3

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e0b9aa6cc835
https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/6d02b83398b6