Closed Bug 949855 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: DOM: Device Interfaces, defect)

26 Branch
ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla29
blocking-b2g koi+
Tracking Status
firefox27 --- wontfix
firefox28 --- fixed
firefox29 --- fixed
b2g-v1.2 --- fixed
b2g-v1.3 --- fixed

People

(Reporter: nhirata, Assigned: pzhang)

Details

(Keywords: regression, Whiteboard: [fxos-bug-bash-1.2])

Attachments

(2 files, 2 obsolete files)

Attached file 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
Does this reproduce on 1.1?
Component: Gaia::FMRadio → AudioChannel
Keywords: qawanted
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 Contact: rkunkel
(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.
(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
Keywords: qawanted
This sounds like another regression in the audio channels work that happened with the FM Radio in 1.2.
blocking-b2g: --- → koi?
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.
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.
Flags: needinfo?(pzhang)
Component: AudioChannel → DOM: Device Interfaces
Product: Firefox OS → Core
Version: unspecified → 26 Branch
(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
(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.
Assignee: nobody → pzhang
Flags: needinfo?(pzhang)
Attached patch Patch (obsolete) — Splinter Review
Attachment #8349187 - Flags: review?(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.
Attachment #8349187 - Flags: review?(mchen)
Attached patch Patch V2 (obsolete) — Splinter Review
Attachment #8349264 - Flags: review?(mchen)
Marco, do you know how can we write a test to cover the audio issues, like this bug?
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)"
Attachment #8349264 - Flags: review?(mchen) → review+
Add one more comment.
Attachment #8349187 - Attachment is obsolete: true
Attachment #8349264 - Attachment is obsolete: true
Attachment #8349293 - Flags: review+
Keywords: regression
This is blocking.   Let's plus it.
Flags: needinfo?(mozillamarcia.knous)
Adding koi+ per triage.
blocking-b2g: koi? → koi+
Flags: needinfo?(mozillamarcia.knous)
https://hg.mozilla.org/mozilla-central/rev/65d1f561cce3
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in before you can comment on or make changes to this bug.