929940 - [B2G][Helix][Audio][tianhuajian]The FM volume changed to max volume level and can not change

Status: RESOLVED DUPLICATE of bug 862672

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

Description

[lecky]

User Agent: Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 6.1; Trident/4.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0; aff-kingsoft-ciba; .NET4.0C; .NET4.0E)

Steps to reproduce:

A is FFOS phone 
1. A open the device
2. Insert the headset
3. Open the FM
4. Search a clear station
5. Remove the headset
6. Insert the headset , when the fm is searching the station , remove the headset
7. Insert the headset, Search a clear station



Actual results:

In the step 7,The Volume changed to max level ,press volume  key can not change volume,The issue can dispear only close and reopen the FM 



Expected results:

In the step 7,The volume level in normal state and can be changed by pressing volume key

【Test Count*】：10
【Found Count*】：5
【Gaia commit ID*】: V1.1HD
【Gecko commit ID*】:  1.1.0 HD
【Log*】：
【Network environment】：
【Resume operation】：

Comment 1

[lecky]

Attachment: The issue log.rar

Comment 2

[lecky]

I have attached the log.
The file  issue_reproduced.txt is the issue reproduced log.
The file after_issue_reproduced_unplug_plug_headset.txt is after the issue has reprodcued ,unplug and plug the headset.
The file after_issue_reproduced_close_and_reopen_fm.txt is after the issue has reproduced, close and reopen fm,and the issue dispear.

Comment 3

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

Once you close the app, the volume will return back to normal.  I did confirm that I can reproduce this issue.

Gaia:     2a20910ea96efb538ec0c99ff25a540ed641f63f
Gecko:    http://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/7cd7f30328e1
BuildID   20131023042210
Version   18.0

Comment 4

[lecky]

The issue can be reproduced in the following Mozilla build.
gaia：f601fe6e5edb7d5016c07abfad8f069af7354f7b
gecko：60d9e982e07e8d09e9d0e52bad6433c302c17f32
gonk-misc：ca9192d7ef6037016d4723647d86b72f7a410633


I find in audio gecko layer in AudioManager.cpp, AudioManager set the Fm volume to max.
So I want to know in when the fm app will set the audio volume to max level?
10-23 05:29:41.179: ERROR/AudioManager(166):InternalSetAudioRoutesICS, setFmVolume masterVolume = 1.000000

Comment 5

[lecky]

Hi Macro,
The bug has two issue,after I modify the following code,the issue disappear.
issue:   The volume change to max level
Modify : Marked the following code in AudioManager.cpp in function InternalSetAudioRoutesICS
  #if 0 
  if (IsDeviceOn(AUDIO_DEVICE_OUT_FM)) {
    
    float masterVolume;
    AudioSystem::getMasterVolume(&masterVolume);
	LOG("thj InternalSetAudioRoutesICS, setFmVolume masterVolume = %f\n",masterVolume);
    AudioSystem::setFmVolume(masterVolume);
  }
  #endif

The issue: The volume can not be changed.
In the hal layer , when in fm mode and remove headset, we will mute the fm stream and will change the value mMuteCount[stream], but the process has error and does not increase muteCount[stream] to 0.
So when change volume after the issue has reproduced, the function checkAndSetVolume will return and does not set
volume

In the above modification, we can confirm and check the Hal layer.
Can you please help me check the modify  in AudioManager.cpp?
Thanks

Comment 6

[Marco Chen [:mchen]]

Hi Lecky,

Thanks for your help to indicate this issue and provide the investigation.
We will look at it since it can be reproduced on Moz Build. Thanks for your effort.

Comment 7

[Marco Chen [:mchen]]

(In reply to lecky from comment #5)
>   if (IsDeviceOn(AUDIO_DEVICE_OUT_FM)) {
>     
>     float masterVolume;
>     AudioSystem::getMasterVolume(&masterVolume);
> 	LOG("thj InternalSetAudioRoutesICS, setFmVolume masterVolume =
> %f\n",masterVolume);
>     AudioSystem::setFmVolume(masterVolume);
>   }

Hi,

1. I found in this test case, the masterVolume is always 1.000 because Gecko didn't try to modify master volume.

2. In step 6, after removing headset, AudioManager::SetFmRadioAudioEnabled(false) should be called but actually it is not. (issue A)

According to 1 and 2, I guess the high volume is caused by this 1.000 set to another output device (ex: speaker + FM).
Will try to solve issue A first.

Comment 8

[Wayne Chang [:wchang]]

I am setting this as a blocker given possible user impact on step 7.

Marco, can you assign someone to work on this?

QAWANTED to see if this is on master/1.2 - this should make blocker on v1.2 as well IMHO.

Thanks.

Comment 9

[William Hsu [:whsu]]

I cannot reproduce this bug on V1.2 build. :)

* Test Build:
 + V1.2
  - Gaia:     2ef9bc3c7a6de228b63e6ba3613eb0c0dd639c59
  - Gecko:    http://hg.mozilla.org/releases/mozilla-aurora/rev/4a94d2ea9d37
  - BuildID   20131028004002
  - Version   26.0a2

 + v1.3
  - Gaia:     31c260b5c853fe78d6bf44c6aeec52a651c3f025
  - Gecko:    http://hg.mozilla.org/mozilla-central/rev/59ff3a2a708a
  - BuildID   20131028040200
  - Version   27.0a1

Many thanks!

Comment 10

[Marco Chen [:mchen]]

1. Gecko - FM in the master/V1.2 did a refactory from JS to C++ already and they considered to protect the enabling state.

2. In order to solve this issue but not add too complex modification, maybe we can protect this from FM app.

Comment 11

[Wayne Chang [:wchang]]

Evelyn,

Can someone work with Marco on this?

Comment 12

[Evelyn Hung]

@Pin, can you help on this or mentor someone solving this problem? Thanks.

Comment 13

[Pin Zhang [:pzhang] (inactive)]

I think the patch for bug 862672 might help, Marco, what do you think?

Comment 14

[Marco Chen [:mchen]]

Bug 814054. This is the another one related to this issue.

And at the first glance, yes that might help this issue.

Comment 15

[Pin Zhang [:pzhang] (inactive)]

(In reply to Marco Chen [:mchen] from comment #14)
> Bug 814054. This is the another one related to this issue.
> 
> And at the first glance, yes that might help this issue.

Then can anyone try the patch for bug 862672 to check if it fix the problem?

Comment 16

[Marco Chen [:mchen]]

(In reply to Pin Zhang [:pzhang] from comment #15)
> Then can anyone try the patch for bug 862672 to check if it fix the problem?

Hi Lecky,

Could you help to check? Thanks.

Comment 17

[lecky]

Hi Macro,
I have test the patch named Patch V2 in bug 862672, the issue is still in.
For our build has no file in patch named Part II marionette tests, I did not combine this patch when I test.

(In reply to Marco Chen [:mchen] from comment #16)
> (In reply to Pin Zhang [:pzhang] from comment #15)
> Then can anyone try the
> patch for bug 862672 to check if it fix the problem?

Hi Lecky,

Could you
> help to check? Thanks.

Comment 18

[lecky]

Hi Macro,
Sorry for I made a mistake when combine the Patch V2 in bug 862672.
Now after combine the patch named Patch V2 in bug 862672  the issue dispear.
Could you please help me check whether I can combine the patch to our build?
And whether it will lead other problem?
Thanks

Comment 19

[Wayne Chang [:wchang]]

Pin,

I just checked through bug 862672 and it seems that the patch needed further testing from that bug, has this happened yet since that bug?

Are we safe to land this on b2g18 (v1.1hd) branch?

Thanks

Comment 20

[Pin Zhang [:pzhang] (inactive)]

(In reply to Wayne Chang [:wchang] from comment #19)
> Pin,
> 
> I just checked through bug 862672 and it seems that the patch needed further
> testing from that bug, has this happened yet since that bug?
> 
> Are we safe to land this on b2g18 (v1.1hd) branch?
> 
> Thanks

Hi Wayne, I think we can check the tests patch of bug 911063 as well, it's supposed to work for both the old and rewrote-in-c++ WebFM API. If the tests passed, I think the risk is very low.

Comment 21

[lecky]

(In reply to Pin Zhang [:pzhang] from comment #20)
Hi Wayne, I think
> we can check the tests patch of bug 911063 as well, it's supposed to work
> for both the old and rewrote-in-c++ WebFM API. If the tests passed, I think
> the risk is very low.

Hi Wayne, I want to know whether you have taken the test as Pin told.
For we want to combine the patch as soon as possible.
Thanks

Comment 22

[Wayne Chang [:wchang]]

Pin,

I believe if the test was landed it would have been run already?
Can we have the patches landed to v1.1hd here?

Comment 23

[Pin Zhang [:pzhang] (inactive)]

(In reply to Wayne Chang [:wchang] from comment #22)
> Pin,
> 
> I believe if the test was landed it would have been run already?
> Can we have the patches landed to v1.1hd here?

Yes, I think so, and we should always verify it by submitting to TryServer and check if it's OK on TBPL before landing it.

Comment 24

[Wayne Chang [:wchang]]

(In reply to Pin Zhang [:pzhang] from comment #23)
> (In reply to Wayne Chang [:wchang] from comment #22)
> > Pin,
> > 
> > I believe if the test was landed it would have been run already?
> > Can we have the patches landed to v1.1hd here?
> 
> Yes, I think so, and we should always verify it by submitting to TryServer
> and check if it's OK on TBPL before landing it.

Can you continue here or should I find someone to help with? :)