1037880 - FM radio should stop sound in speaker mode when I receive an incoming call

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::FMRadio, defect)

Description

[Cynthia Tang [:cynthiatang]]

Environmental Variables
Device: Dolphin v1.4
Gaia      229864ff4ad90899f017341b9e81ed0b53498c66
Gecko     https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/1b9cee26ddf9
BuildID   20140708160206

STR:
See https://moztrap.mozilla.org/manage/case/11241/
    https://moztrap.mozilla.org/manage/case/11242/
    https://moztrap.mozilla.org/manage/case/11243/

1. Open browser app
When I receive an incoming call while the FM radio is active.

Actual Result:
I can hear sound from the FM radio for about 2 seconds.

Expected result:
I should no longer hear sound from the FM radio.

Comment 1

[William Hsu [:whsu]]

Hi, Cynthia,

Could I have your help?
It is good to paste some reference information here.
But, if you can write the steps in detail, it would be better.
Thanks!

Comment 2

[William Hsu [:whsu]]

One more question, can it be reproduced on Flame (v1.4)?
Thanks!

Comment 3

[David Flanagan [:djf]]

Maybe we could fix this the same way we fixed stopped video recording when a call comes in on Tarako. See bug 995540.

Comment 4

[Hema Koka [:hema]]

The root issue is tracked here: https://bugzilla.mozilla.org/show_bug.cgi?id=1006200

There was a hack that went in for something similar on camera: https://bugzilla.mozilla.org/show_bug.cgi?id=995540

Comment 5

[David Flanagan [:djf]]

Cynthia: is it necessary to have the browser app open to reproduce this bug?  If so, that inidicates that this is a low-memory condition like in bug 995540.

How severe is this bug? Does it really need to block 1.4?

Comment 6

[Cynthia Tang [:cynthiatang]]

(In reply to David Flanagan[OOO until July 14] [:djf] from comment #5)
> Cynthia: is it necessary to have the browser app open to reproduce this bug?
> If so, that inidicates that this is a low-memory condition like in bug
> 995540.
> 
> How severe is this bug? Does it really need to block 1.4?

Hi David,

Sorry, it is not necessary to have the browser app open to reproduce this bug.

The steps in detail:
1. Listen to FM radio in speaker mode:
   1) Plug in your wired headset 
   2) Open FM Radio app
   3) Tap speaker mode
   4) Tap play to listen to FM radio

2. Receive an incoming call

Affected:
Flam v1.3 base image v122
Flam v1.4 (BuildID:20140714160202)
Flam v2.0 (BuildID:20140714160201)
Flam v2.0 (BuildID:20140714160204)
Dolphin V1.4 (BuildID:20140708160206)

Unaffected:
Tarako v1.3T 
Gaia      665e869fbca6e42a753bda9e465228c16c9510ff
Gecko     https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/3ecb74bf5479
BuildID   20140714164001
Version   28.1
ro.build.version.incremental=eng.cltbld.20140714.200947
ro.build.date=Mon Jul 14 20:09:54 EDT 2014

Thanks!

Comment 7

[Justin D'Arcangelo [:justindarc]]

Attachment: pull-request (master)

Comment 8

[Pin Zhang [:pzhang] (inactive)]

(In reply to Justin D'Arcangelo [:justindarc] from comment #7)
> Created attachment 8457013 [details] [review]
> pull-request (master)

Hi Justin, please check my comments, thanks.

Comment 9

[Justin D'Arcangelo [:justindarc]]

Comment on attachment 8457013 [details] [review]
pull-request (master)

Addressed review comments and added some unit tests. Please re-review. Thanks!

Comment 10

[Pin Zhang [:pzhang] (inactive)]

(In reply to Justin D'Arcangelo [:justindarc] from comment #9)
> Comment on attachment 8457013 [details] [review]
> pull-request (master)
> 
> Addressed review comments and added some unit tests. Please re-review.
> Thanks!

Hi Justin, i think we need to handle the case that earphone is unplugged, please check my comment, thanks.

In the patch, we are trying to disable the FM radio if the attention screen is shown and re-enable it when the attention screen disappeared, but the cases would be kinda complicated if we considered antenna plugged/unplugged status, say:
  - User opened FM radio app
  - User turned the speaker on
  - A call is coming, and user picked it up
  - User unplugged the headset
  - User hang up the phone
  - User plugged in the headset back

according to the codes, nothing would happen, because the FM radio is turned off and we remembered the disabled state in the |antennaAvailable| event handler[1].

So is it possible that we just handle the |speakerForced| status instead of turning the FM radio off and on?


[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/fm/js/fm.js#L799

Comment 11

[Justin D'Arcangelo [:justindarc]]

Comment on attachment 8457013 [details] [review]
pull-request (master)

Pin: I've updated the patch with your comments from the pull request.

Comment 12

[Justin D'Arcangelo [:justindarc]]

(In reply to Pin Zhang [:pzhang] from comment #10)
> So is it possible that we just handle the |speakerForced| status instead of
> turning the FM radio off and on?

I don't think we can handle it this way. According to Comment 0, the user expects to hear the FM radio again when the call is complete (assuming they've left their headphones connected). I think your suggestion in the pull request (which I've now updated) is sufficient. We simply check if the antenna is still available when returning from a call before re-enabling the radio again.

Comment 13

[Pin Zhang [:pzhang] (inactive)]

Comment on attachment 8457013 [details] [review]
pull-request (master)

(In reply to Justin D'Arcangelo [:justindarc] from comment #12)
> (In reply to Pin Zhang [:pzhang] from comment #10)
> > So is it possible that we just handle the |speakerForced| status instead of
> > turning the FM radio off and on?
> 
> I don't think we can handle it this way. According to Comment 0, the user
> expects to hear the FM radio again when the call is complete (assuming
> they've left their headphones connected). I think your suggestion in the
> pull request (which I've now updated) is sufficient. We simply check if the
> antenna is still available when returning from a call before re-enabling the
> radio again.

r=me, thanks!

Comment 14

[Justin D'Arcangelo [:justindarc]]

Landed on master:

https://github.com/mozilla-b2g/gaia/commit/78cbbaeb041e5dd25a32c521b3d4ff08735ac139

Comment 15

[Justin D'Arcangelo [:justindarc]]

Attachment: pull-request (v2.0)

Pull request for v2.0 branch, carrying R+ forward

Comment 16

[Justin D'Arcangelo [:justindarc]]

Attachment: pull-request (v1.4)

Pull request for v1.4 branch, carrying R+ forward

Comment 17

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/7c98279e5577b499b0e0165f93cc3bc25d2b3d86
v1.4: https://github.com/mozilla-b2g/gaia/commit/6e4eaa5befce9c1812e07bcc78b2138645bdbe7a

Comment 18

[William Hsu [:whsu]]

Verified this patch on the latest dolphin build (v1.4)

* Build information:
 - Gaia      eb3b185325901d4c04e2d43eb58d90835213bea9
 - Gecko     https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/1f7f2fb4ea12
 - BuildID   20140729160209
 - Version   30.0

Comment 19

[jingmei.zhang]

hi Justin:

 the patch might cause another issue,please see bug1069887,can you help to check??

 thank you very much!!

Comment 20

[Justin D'Arcangelo [:justindarc]]

(In reply to jingmei.zhang from comment #19)
> hi Justin:
> 
>  the patch might cause another issue,please see bug1069887,can you help to
> check??
> 
>  thank you very much!!

I've taken Bug 1069887.

Comment 21

[Chaochao Huang]

Hi David,

The patch on the branch 1.4  results in another issue, https://bugzilla.mozilla.org/show_bug.cgi?id=1069887. After removing the patch,  this bug Bug995540 still doesnot reproduce. Is there any change of communication mechanism to speed response? Is it workable that removing the patch to fix Bug1069887 in Bug995540 unreproduce case? 

Thanks.

Comment 22

[Chaochao Huang]

Plz see comment 21. Thanks.

Comment 23

[Chaochao Huang]

So sorry, ignore comment 21.

Hi David, 

The patch on the branch 1.4  results in another issue, https://bugzilla.mozilla.org/show_bug.cgi?id=1069887. After removing the patch,  this bug 1037880 still doesnot reproduce. Is there any change of communication mechanism to speed response? Is it workable that removing the patch to fix Bug1069887 in Bug1037880 unreproduce case? 

Thanks.

Comment 24

[David Flanagan [:djf]]

Chaochao,

I think you're saying that if we revert this patch it will fix bug 1069887 and will not cause this original bug to come back. If so, that is great and it is fine with me if you do that in your own 1.4 tree. I wouldn't want to revert this patch in 2.0 or later without very careful testing, however.

I don't what, if anything, has changed to make this patch unnecessary. Maybe there was a fix to the audio competing policy code so that only one sound plays at a time.  

Justin may know more about this than I do. And Dominic might be able to say more about the audio competing policy code and whether there are any fixes to it in the 1.4 tree.

Comment 25

[Dominic Kuo [:dkuo]]

As I know, there is no significant changes on the audio competing policy in 1.4 branch, all the resolved issues are almost fixed by gaia hacks, if those cannot be easily addressed by gecko or gonk, but I remember there are some gecko/gonk fixes I didn't quite follow that are trying to address the corner cases, and gaia didn't aware of it and they are in some specific branch, like 1.3t...

And I have took a quick look on the hacky patch of this bug, because we listened to the mozSettings to disable FM, it should be fast enough to prevent the FM leak(bug 1069887) while the callscreen shows up, though I don't why bug 1069887 is reproducible. Also it's strange that, if we revert this bug in 1.4, comment 23 said this bug is not reproducible, so that means the FM radio will just be interrupted when an incoming call comes, without this hack? if so, I believe gecko has done something to pause/disable the FM radio when the ril signals comes in, but I don't know it.

Chaochao, would you please confirm this? because I am unclear about comment 23 and needs your inputs, thanks.

Comment 26

[Chaochao Huang]

Dominic, 

I have tested many times. I confirm that reverting this patch in v1.4 will fix bug 1069887 and will not cause this original bug.

Comment 27

[Dominic Kuo [:dkuo]]

(In reply to Chaochao Huang from comment #26)
> Dominic, 
> 
> I have tested many times. I confirm that reverting this patch in v1.4 will
> fix bug 1069887 and will not cause this original bug.

Then this bug's status-b2g-v1.4 was wrong flagged, it should be unaffected and does not need the patch, but it's more weird that this bug was initially filed for 1.4. Cynthia, would you please help to clarify the status-b2g-v1.4 flag before landing the v1.4 patch? thanks!

Comment 28

[William Hsu [:whsu]]

Hi, Chaochao,

Could you please provide the build information and device name you tested?

Device: ? ( Flame? / Dolphin Dev? / Dolphin Prod? )
Gaia: ?
Gecko: ?
BuildID: ?

It is hard to trace this problem since we had uplifted many patch on v1.4 within three months.
Also, it's unreasonable to back out a patch, just because you cannot reproduce this bug.
Were you using the right method, build, and device?
As Dominic mentioned, some Gonk or Gecko might impact your test result.
So, please align these information, and then trying to reproduce this bug again.
The patch is fine with me since it seems to enhance the ERROR handling of audio channel.
What do you think? Any thought?
Thanks for your help. :)

Comment 29

[Chaochao Huang]

(In reply to William Hsu [:whsu] from comment #28)

Device    Dolphin Dev
Gaia      9ff9ee9e56ba04aff6900f872535cd8f3aee7904
Gecko     5af8d51709124cf773580a3016c26382b1219c77
BuildID   20141016191504
Version   30.0

The patch can enhance the ERROR handling of audio channel, but results in bug 1069887. So far, there isnot good plan for bug 1069887. I think it is good to trace why reverting this patch will not cause this original bug.

Comment 30

[William Hsu [:whsu]]

(In reply to Chaochao Huang from comment #29)
> (In reply to William Hsu [:whsu] from comment #28)
> 
> Device    Dolphin Dev
> Gaia      9ff9ee9e56ba04aff6900f872535cd8f3aee7904
> Gecko     5af8d51709124cf773580a3016c26382b1219c77
> BuildID   20141016191504
> Version   30.0
> 
> The patch can enhance the ERROR handling of audio channel, but results in
> bug 1069887. So far, there isnot good plan for bug 1069887. I think it is
> good to trace why reverting this patch will not cause this original bug.

Perhaps, we can try to find regression window on Flame since Flame can reproduce it.

Comment 31

[Cynthia Tang [:cynthiatang]]

(In reply to Dominic Kuo [:dkuo] from comment #27)
> (In reply to Chaochao Huang from comment #26)
> > Dominic, 
> > 
> > I have tested many times. I confirm that reverting this patch in v1.4 will
> > fix bug 1069887 and will not cause this original bug.
> 
> Then this bug's status-b2g-v1.4 was wrong flagged, it should be unaffected
> and does not need the patch, but it's more weird that this bug was initially
> filed for 1.4. Cynthia, would you please help to clarify the status-b2g-v1.4
> flag before landing the v1.4 patch? thanks!

Hi Dominic,
This issue can be reproduced on flame. Thank you.

==== Build Information - Flame v1.4 (Shallow Flash Build) ===
Gaia-Rev        b7d36622c7df92c976c37520ccab25199c7ada91
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/de7ecfb00955
Build-ID        20140714160202
Version         30.0
Device-Name     flame
FW-Release      4.3
FW-Incremental  109
FW-Date         Mon Jun 16 16:51:29 CST 2014
Bootloader      L1TC00011220