Closed Bug 931722 Opened 6 years ago Closed 6 years ago

[Fugu]Can't mute in the call

Categories

(Firefox OS Graveyard :: RIL, defect)

defect
Not set

Tracking

(blocking-b2g:fugu+, firefox26 wontfix, firefox27 wontfix, firefox28 fixed, b2g-v1.2 affected)

RESOLVED FIXED
1.3 Sprint 6 - 12/6
blocking-b2g fugu+
Tracking Status
firefox26 --- wontfix
firefox27 --- wontfix
firefox28 --- fixed
b2g-v1.2 --- affected

People

(Reporter: sam.hua, Assigned: sku)

References

Details

(Whiteboard: [Fugu] [v1.2f-uplift-needed])

Attachments

(6 files, 24 obsolete files)

5.00 KB, patch
Details | Diff | Splinter Review
4.90 KB, patch
Details | Diff | Splinter Review
1.41 KB, patch
Details | Diff | Splinter Review
4.81 KB, patch
Details | Diff | Splinter Review
4.93 KB, patch
Details | Diff | Splinter Review
1.59 KB, patch
Details | Diff | Splinter Review
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.22 (KHTML, like Gecko) Ubuntu Chromium/25.0.1364.160 Chrome/25.0.1364.160 Safari/537.22

Steps to reproduce:

1. setup a call.
2. press mute icon


Actual results:

The mic phone couldn't be muted


Expected results:

The mic phone should be muted
In TelephonyProvider.js
set microphoneMuted() will call "gAudioManager.microphoneMuted = aMuted",but our tara device fails to mute the mic.

In android, there are two modules to handle mic muting,audio and RIL. Tara device works well if apps choose the RIL.

Could TelephonyProvider in Gecko also support the muting operation in the RIL module? Sending "setMicMute" message to ril_worker and ril_worker sends the "REQUEST_SET_MUTE" to rild.
Summary: [tara]Can't mute in the call → [Fugu]Can't mute in the call
Ming, pls add the patch.
In TelephonyProvider.js ,we route the request of "mute the mic" to RIL, not to AudioManger.

Referd to ANDROID , it send the request to RIL, not to AudioManager ,actually the reason is that the value of parameter of "send_mic_mute_to_AudioManager"  is false.
Add Ken
Can't reproduce it in Buri with v1.2.
Flags: needinfo?(sam.hua)
(In reply to styang from comment #5)
> Can't reproduce it in Buri with v1.2.

Maybe it's chipset hardcode.
Because our 7710 android bsp version is also ok.
(In reply to James Zhang from comment #6)
> (In reply to styang from comment #5)
> > Can't reproduce it in Buri with v1.2.
> 
> Maybe it's chipset hardcode.
> Because our 7710 android bsp version is also ok.

This feature is chipset dependent.
Some chipset need mute request from audioManager, some need from RIL.


The information should be provided by chipset provider.

thanks!!
sku
(In reply to shawn ku [:sku] from comment #7)
> (In reply to James Zhang from comment #6)
> > (In reply to styang from comment #5)
> > > Can't reproduce it in Buri with v1.2.
> > 
> > Maybe it's chipset hardcode.
> > Because our 7710 android bsp version is also ok.
> 
> This feature is chipset dependent.
> Some chipset need mute request from audioManager, some need from RIL.
> 
> 
> The information should be provided by chipset provider.
> 
> thanks!!
> sku

We need mute from RIL.
I think gecko should support more chipset, qcom and sprd, the same as android. 
You can test this case on hamachi and fugu device.
The patch does not look okay to us.

1. no matter RIL or AudioManager path, it should be only one be triggered. not both of them
2. After offline discussion with RIL team, AudioManager.cpp might be the proper place to distinguish/trigger the mute/un-mute with correct path.


Hi Randy:
 Could you please give us your comment/suggestion?

Thanks!!
sku
Flags: needinfo?(rlin)
This patch would broke the original design, right?
I also agree sku's option because we may remove out the microphoneMuted API from telephony interface.
Flags: needinfo?(sam.hua)
Flags: needinfo?(rlin)
Flags: needinfo?(sam.hua)
if we want distinguish/trigger the correct path in AudioManager.cpp, we will have to find out how to send the mute command to ril path.
That's the reason i distinguish it in the TelephonyProvider.js and add "setMicMute" method in ril_worker.js,the routes are ready.
Flags: needinfo?(sam.hua)
Can we use property just like moz.mute.ril to handle sprd chipset case.
Flags: needinfo?(sku)
I am checking if there is way to go through mute/unmute via AudioManager.cpp.
you can simply change 

mRouteMuteToRIL = Preferences::GetBool("device.phone.mute_via_RIL", false);
to
mRouteMuteToRIL = Preferences::GetBool("device.phone.mute_via_RIL", true);

to make mute request through RIL, not Audio HAL.
Flags: needinfo?(sku)
Comment on attachment 827897 [details] [diff] [review]
trial patch for setMute part. (still need getMute)

This patch for setMute is in-complete. obsolete first.
Attachment #827897 - Attachment is obsolete: true
[Trial] Let AudioManager.cpp to handle mute request (via RIL or via HAL).
Still need getMute part.
Attachment #823276 - Attachment is obsolete: true
Attachment #827913 - Attachment is obsolete: true
Assignee: nobody → sku
Attachment #831250 - Flags: review?(mchen)
Attachment #831251 - Flags: review?(htsai)
Comment on attachment 831250 [details] [diff] [review]
Bug 931722 - Part1: seperate mute route at AudioManager. [Fugu]Can't mute in the call.

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

Hi Shawn,

Thanks for taking care this issue.

1. Is the muted state of RIL per each voice_call or not? (the ril here means components outside the gecko for telephony feature)
  ex:
    a. A first voice call is picked up then set muted to true. And hang up the call.
    b. A second voice call is picked up again.
    In this case, is the muted state in RIL "true" or "false"?
   I am afraid that muted state in RIL for second call is false but it is true in AudioManager.

2. I think we need to complete the case for multi-sim too.

::: dom/system/gonk/AudioManager.cpp
@@ +460,5 @@
>  
>  NS_IMETHODIMP
>  AudioManager::SetMicrophoneMuted(bool aMicrophoneMuted)
>  {
> +  if (mRouteMuteToRIL) {

In order to reduce the levels of if-else, we can do the "return error" before do following stuffs.
And the "return error" check can use macro to beautify the codes.

ex: 
nsCOMPtr<nsIRadioInterfaceLayer> ril = do_GetService("@mozilla.org/ril;1");
NS_ENSURE_TRUE(ril, NS_ERROR_FAILURE);

...
NS_ENSURE_TRUE(radioInterface, NS_ERROR_FAILURE);

@@ +465,5 @@
> +    nsCOMPtr<nsIRadioInterfaceLayer> ril = do_GetService("@mozilla.org/ril;1");
> +    if (ril) {
> +      nsCOMPtr<nsIRadioInterface> radioInterface;
> +      // TODO: acquire correct RadioInterface instance in Multi-SIM
> +      // configuration

Since Multi-SIM is starting to test on Fugu device, we need to make sure Multi-SIM is workable too.

@@ +470,5 @@
> +      ril->GetRadioInterface(0 /* clientId */, getter_AddRefs(radioInterface));
> +      if (radioInterface) {
> +        AutoSafeJSContext cx;
> +        JS::Rooted<JSObject*> obj(cx, JS_NewObject(cx, nullptr, nullptr,
> +                                                   nullptr));

JS::Rooted<JSObject*> obj(cx,
                          JS_NewObject(cx, nullptr, nullptr, nullptr));
Attachment #831250 - Flags: review?(mchen)
Hi Marco:
 Thanks for your time and comment.
Please check my inline reply, correct me if anything is wrong.

(In reply to Marco Chen [:mchen] from comment #19)
> Comment on attachment 831250 [details] [diff] [review]
> Bug 931722 - Part1: seperate mute route at AudioManager. [Fugu]Can't mute in
> the call.
> 
> Review of attachment 831250 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Hi Shawn,
> 
> Thanks for taking care this issue.
> 
> 1. Is the muted state of RIL per each voice_call or not? (the ril here means
> components outside the gecko for telephony feature)
>   ex:
>     a. A first voice call is picked up then set muted to true. And hang up
> the call.
>     b. A second voice call is picked up again.
>     In this case, is the muted state in RIL "true" or "false"?
>    I am afraid that muted state in RIL for second call is false but it is
> true in AudioManager.

gaia will reset mute to false when first mt/mo. therefore, the status will be all synced from gaia<->AudioManger<->RIL<->modem.

for the case you mentioned, there should be no such un-sync probelm.
am I right?

// calls_handler.js at gaia
    // First incoming or outgoing call, reset mute and speaker.
    if (handledCalls.length == 0) {
      CallScreen.unmute();
      CallScreen.turnSpeakerOff();
    }

> 
> 2. I think we need to complete the case for multi-sim too.
> 

 Roger that.

> ::: dom/system/gonk/AudioManager.cpp
> @@ +460,5 @@
> >  
> >  NS_IMETHODIMP
> >  AudioManager::SetMicrophoneMuted(bool aMicrophoneMuted)
> >  {
> > +  if (mRouteMuteToRIL) {
> 
> In order to reduce the levels of if-else, we can do the "return error"
> before do following stuffs.
> And the "return error" check can use macro to beautify the codes.
> 
> ex: 
> nsCOMPtr<nsIRadioInterfaceLayer> ril = do_GetService("@mozilla.org/ril;1");
> NS_ENSURE_TRUE(ril, NS_ERROR_FAILURE);
> 
> ...
> NS_ENSURE_TRUE(radioInterface, NS_ERROR_FAILURE);
> 

Thanks for lecture me coding style/convention.
Will update this part in next patch.

> @@ +465,5 @@
> > +    nsCOMPtr<nsIRadioInterfaceLayer> ril = do_GetService("@mozilla.org/ril;1");
> > +    if (ril) {
> > +      nsCOMPtr<nsIRadioInterface> radioInterface;
> > +      // TODO: acquire correct RadioInterface instance in Multi-SIM
> > +      // configuration
> 
> Since Multi-SIM is starting to test on Fugu device, we need to make sure
> Multi-SIM is workable too.
>

Sure, will check how to adopt this.
 
> @@ +470,5 @@
> > +      ril->GetRadioInterface(0 /* clientId */, getter_AddRefs(radioInterface));
> > +      if (radioInterface) {
> > +        AutoSafeJSContext cx;
> > +        JS::Rooted<JSObject*> obj(cx, JS_NewObject(cx, nullptr, nullptr,
> > +                                                   nullptr));
> 
> JS::Rooted<JSObject*> obj(cx,
>                           JS_NewObject(cx, nullptr, nullptr, nullptr));

revice in next patch.


Thanks again!!
sku
Blocks: 883051
Comment on attachment 831251 [details] [diff] [review]
Bug 931722 - Part2: change signature of setMute at ril_worker. [Fugu]Can't mute in the call.

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

Looks good, thanks!
Attachment #831251 - Flags: review?(htsai) → review+
Attachment #831250 - Attachment is obsolete: true
Attachment #831251 - Attachment is obsolete: true
Attachment #832838 - Attachment is obsolete: true
Attachment #832839 - Attachment is obsolete: true
Attachment #832840 - Flags: review?(htsai)
Attachment #832841 - Flags: review?(mchen)
Attachment #832842 - Flags: review?(htsai)
Hi Etienne:
 We plan to "reset mute state to false" when user try to dial/answer calls at TelephonyProvider.
Could you please have a look to see if any impact regarding Gaia? Or anything need to be synced between Gaia and Gecko?

Thanks!!
sku
Flags: needinfo?(etienne)
I think we're good.
We're already doing it in gaia I think :)
https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/calls_handler.js#L174-178
Flags: needinfo?(etienne)
Comment on attachment 832841 [details] [diff] [review]
Bug 931722 - Part2: seperate mute route at AudioManager. [Fugu]Can't mute in the call. v2.

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

In this patch, we did a lot of things related to RIL module.
Maybe keep these stuffs into RIL module itself and expose an interface for AudioManager to trigger mute/unmute is more clear between RIL and AudioManager.

::: dom/system/gonk/AudioManager.cpp
@@ +454,4 @@
>    }
> +
> +  NS_ENSURE_FALSE(AudioSystem::isMicrophoneMuted(aMicrophoneMuted),
> +                  NS_ERROR_FAILURE);

This is my personal opinion.
I feel use macro here is too over-engineering already.
Attachment #832841 - Flags: review?(mchen)
Comment on attachment 832840 [details] [diff] [review]
Bug 931722 - Part1: RIL related code change. [Fugu]Can't mute in the call. v2.

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

Really sorry for the delay. :(

Per offline discussion with Shawn and Marco, we agree that having a new attribute exposing to nsIRadioInterfaceLayer is better for clear modulization. Please revise the patch for that. Thank you!
Attachment #832840 - Flags: review?(htsai)
Attachment #832840 - Attachment is obsolete: true
Attachment #832841 - Attachment is obsolete: true
Attachment #832842 - Attachment is obsolete: true
Attachment #832842 - Flags: review?(htsai)
Attachment #8338239 - Flags: review?(htsai)
Attachment #8338240 - Flags: review?(mchen)
Attachment #8338242 - Flags: review?(htsai)
Comment on attachment 8338239 [details] [diff] [review]
Bug 931722 - Part1: RIL related code change. [Fugu]Can't mute in the call. v3.

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

::: dom/telephony/gonk/TelephonyProvider.js
@@ +390,5 @@
>        aNumber = gPhoneNumberUtils.normalize(aNumber);
>      }
>      if (this._validateNumber(aNumber)) {
> +      // set mute to false before dialing request.
> +      this.microphoneMuted = false;

Per comment 28, I don't see why we need this still.

@@ +412,5 @@
>    },
>  
>    answerCall: function(aClientId, aCallIndex) {
> +    // set mute to false before answering a call.
> +    this.microphoneMuted = false;

ditto.
Hi Hsin-Yi:
 Thanks for your comment.

I would like to clarify one thing here.

By reading the code that Etienne provided, we can see gaia only reset mute state to false then "first" MT/MO call.
My consideration here (refer to AOSP behaviour) is we might need to reset mute to false when every dial/answer request.
For example, 1 Active + 1 Waiting with muted state, user try to answer the wating call, we unmute the call automatically for user's convenient. if we don't do so, device still keep in muted state after answering waiting call.

Please correct me if anything is wrong in your viewpoint. or give me some extra suggestion.

Thanks!!
sku





(https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/calls_handler.js#L174-178)
    // First incoming or outgoing call, reset mute and speaker.
    if (handledCalls.length == 0) {
      CallScreen.unmute();
      CallScreen.turnSpeakerOff();
    }
(In reply to shawn ku [:sku] from comment #35)
> Hi Hsin-Yi:
>  Thanks for your comment.
> 
> I would like to clarify one thing here.
> 
> By reading the code that Etienne provided, we can see gaia only reset mute
> state to false then "first" MT/MO call.
> My consideration here (refer to AOSP behaviour) is we might need to reset
> mute to false when every dial/answer request.
> For example, 1 Active + 1 Waiting with muted state, user try to answer the
> wating call, we unmute the call automatically for user's convenient. if we
> don't do so, device still keep in muted state after answering waiting call.
> 
Thanks for the explanation, Shawn :)

I am fine with this user behaviour. However I think this is out of scope of this bug. I'd suggest filing another bug to have thorough discussion on mute behaviour. For example, if user mutes before a call is putting on hold, then should we unmute it automatically when the call is resumed? Another concern is, even we want to mute/unmute automatically for user's convenience, where should we handle this, gaia or gecko? Looks gaia is taking part of it, isn't it better to let gaia manage the whole logic instead of spreading the same thing into different components? 

> Please correct me if anything is wrong in your viewpoint. or give me some
> extra suggestion.
> 
> Thanks!!
> sku
> 
> 
> 
> 
> 
> (https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/
> js/calls_handler.js#L174-178)
>     // First incoming or outgoing call, reset mute and speaker.
>     if (handledCalls.length == 0) {
>       CallScreen.unmute();
>       CallScreen.turnSpeakerOff();
>     }
Attachment #8338239 - Attachment is obsolete: true
Attachment #8338240 - Attachment is obsolete: true
Attachment #8338242 - Attachment is obsolete: true
Attachment #8338239 - Flags: review?(htsai)
Attachment #8338240 - Flags: review?(mchen)
Attachment #8338242 - Flags: review?(htsai)
Comment on attachment 8338312 [details] [diff] [review]
Bug 931722 - Part1: RIL related code change. [Fugu]Can't mute in the call. v4.

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

remove reset mute at TelephonyProvider.js
Attachment #8338312 - Flags: review?(htsai)
Comment on attachment 8338313 [details] [diff] [review]
Bug 931722 - Part2: seperate mute route at AudioManager. [Fugu]Can't mute in the call. v4.

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

change reading preference to system property to determine which mute path should go.
Attachment #8338313 - Flags: review?(mchen)
Comment on attachment 8338336 [details] [diff] [review]
Bug 931722 - Part3: Add test cases for call mute test. [Fugu]Can't mute in the call. v4.

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

Add test cases to make sure set/get mute via telephony work as expect.
Attachment #8338336 - Flags: review?(htsai)
Comment on attachment 8338312 [details] [diff] [review]
Bug 931722 - Part1: RIL related code change. [Fugu]Can't mute in the call. v4.

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

Thanks! r=me with the comments addressed.

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +599,5 @@
> +
> +  setMicrophoneMuted: function setMicrophoneMuted(muted) {
> +    for (let clientId = 0; clientId < this.numRadioInterfaces; clientId++) {
> +      let radioInterface = this.radioInterfaces[clientId];
> +      radioInterface.workerMessenger.send("setMute", { mute: muted });

nit: s/mute/muted/

::: dom/system/gonk/ril_worker.js
@@ +1454,3 @@
>      Buf.newParcel(REQUEST_SET_MUTE);
>      Buf.writeInt32(1);
> +    Buf.writeInt32(options.mute ? 1 : 0);

s/mute/muted/
Attachment #8338312 - Flags: review?(htsai) → review+
Comment on attachment 8338336 [details] [diff] [review]
Bug 931722 - Part3: Add test cases for call mute test. [Fugu]Can't mute in the call. v4.

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

General comment:

I don't see the connection b/w mute attribute and call states.
If test_call_mute.js is to test if telephony.muted works as expected, why not simply do the following:
    telephony.muted = false;
    is(telephony.muted, false);
    telephony.muted = true;
    is(telephony.muted, true);

::: dom/telephony/test/marionette/test_call_mute.js
@@ +3,5 @@
> +
> +MARIONETTE_TIMEOUT = 60000;
> +MARIONETTE_HEAD_JS = 'head.js';
> +
> +let telephony = window.navigator.mozTelephony;

We've done this in 'head.js.' Remove it.

@@ +9,5 @@
> +let outgoingCall;
> +let number = "1234567890";
> +
> +startTest(function() {
> +  test_outgoing_call();

Please move this function to the end of the file. We follow the convention in telephony marionette tests.

@@ +50,5 @@
> +      incomingCall.hangUp();
> +    };
> +
> +    incomingCall.ondisconnected = function ondisconnected(event) {
> +      cleanUp();

Make sure there's no calls in emulator ('gsm list') before cleanUp().

Please add this:
emulator.run("gsm list", function(result) {
      log("Call list is now: " + result);
      is(result[0], "OK");
      cleanUp();
    });
Attachment #8338336 - Flags: review?(htsai)
Comment on attachment 8338388 [details] [diff] [review]
Bug 931722 - Part3: Add test cases for call mute test. [Fugu]Can't mute in the call. v5.

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

Thanks HsinYi,
It is my bad to keep call state with mute.
My original purpose was to make sure we got un-muted state with every dial/answer request. however, this will be move to new bug for discussion first.

I should only keep below part just to make sure APIs works as expected.

+  telephony.muted = true;
+  is(telephony.muted, true);
+  telephony.muted = false;
+  is(telephony.muted, false);

Thanks again!!
Attachment #8338388 - Flags: review?(htsai)
Comment on attachment 8338388 [details] [diff] [review]
Bug 931722 - Part3: Add test cases for call mute test. [Fugu]Can't mute in the call. v5.

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

Thank you!
Attachment #8338388 - Flags: review?(htsai) → review+
(In reply to shawn ku [:sku] from comment #48)
> Comment on attachment 8338388 [details] [diff] [review]
> Bug 931722 - Part3: Add test cases for call mute test. [Fugu]Can't mute in
> the call. v5.
> 
> Review of attachment 8338388 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks HsinYi,
> It is my bad to keep call state with mute.

No worries :)

> My original purpose was to make sure we got un-muted state with every
> dial/answer request. however, this will be move to new bug for discussion
> first.
> 
> I should only keep below part just to make sure APIs works as expected.
> 
> +  telephony.muted = true;
> +  is(telephony.muted, true);
> +  telephony.muted = false;
> +  is(telephony.muted, false);
> 
> Thanks again!!
Comment on attachment 8338313 [details] [diff] [review]
Bug 931722 - Part2: seperate mute route at AudioManager. [Fugu]Can't mute in the call. v4.

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

::: dom/system/gonk/AudioManager.cpp
@@ +423,5 @@
>      NS_WARNING("Failed to add mozsettings-changed observer!");
>    }
> +
> +  char value[PROPERTY_VALUE_MAX];
> +  property_get("device.send-mic-mute-to-ril", value, "0");

1. Refer to the old property for FxOS, the naming will be "ro.moz.XXX".
ex: ro.moz.audio.mute.from_ril

2. And I suggest to set value as "true" or "false" and that will be more readable.

@@ +472,5 @@
> +    NS_ENSURE_TRUE(ril, NS_ERROR_FAILURE);
> +    ril->SetMicrophoneMuted(aMicrophoneMuted);
> +    mIsMicMuted = aMicrophoneMuted;
> +    return NS_OK;
> +  }

I am sorry to bring this question late.

If the mute is came from audio recording app and video conference app in the future (not in voice_call state.)
Is sending mute to ril really workable?
Or we need to enable mute from RIL and AudioSystem.
Attachment #8338313 - Flags: review?(mchen)
When can I add "device.send-mic-mute-to-ril=1" property to fugu device?
(In reply to Marco Chen [:mchen] from comment #51)
> Comment on attachment 8338313 [details] [diff] [review]
> Bug 931722 - Part2: seperate mute route at AudioManager. [Fugu]Can't mute in
> the call. v4.
> 
> Review of attachment 8338313 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/system/gonk/AudioManager.cpp
> @@ +423,5 @@
> >      NS_WARNING("Failed to add mozsettings-changed observer!");
> >    }
> > +
> > +  char value[PROPERTY_VALUE_MAX];
> > +  property_get("device.send-mic-mute-to-ril", value, "0");
> 
> 1. Refer to the old property for FxOS, the naming will be "ro.moz.XXX".
> ex: ro.moz.audio.mute.from_ril
> 
> 2. And I suggest to set value as "true" or "false" and that will be more
> readable.
> 
> @@ +472,5 @@
> > +    NS_ENSURE_TRUE(ril, NS_ERROR_FAILURE);
> > +    ril->SetMicrophoneMuted(aMicrophoneMuted);
> > +    mIsMicMuted = aMicrophoneMuted;
> > +    return NS_OK;
> > +  }
> 
> I am sorry to bring this question late.
> 
> If the mute is came from audio recording app and video conference app in the
> future (not in voice_call state.)
> Is sending mute to ril really workable?
> Or we need to enable mute from RIL and AudioSystem.

Marco, you're right, ro.moz.audio.mute.from_ril=true or false is better name.
Hi Marco:
 I will update the naming for property today, thanks for addressing it.

Hi James:
 Could you please help check/answer question from Marco about comment 51?
Flags: needinfo?(james.zhang)
(In reply to Marco Chen [:mchen] from comment #51)
> Comment on attachment 8338313 [details] [diff] [review]
> Bug 931722 - Part2: seperate mute route at AudioManager. [Fugu]Can't mute in
> the call. v4.
> 
> Review of attachment 8338313 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/system/gonk/AudioManager.cpp
> @@ +423,5 @@
> >      NS_WARNING("Failed to add mozsettings-changed observer!");
> >    }
> > +
> > +  char value[PROPERTY_VALUE_MAX];
> > +  property_get("device.send-mic-mute-to-ril", value, "0");
> 
> 1. Refer to the old property for FxOS, the naming will be "ro.moz.XXX".
> ex: ro.moz.audio.mute.from_ril
> 
> 2. And I suggest to set value as "true" or "false" and that will be more
> readable.
> 
> @@ +472,5 @@
> > +    NS_ENSURE_TRUE(ril, NS_ERROR_FAILURE);
> > +    ril->SetMicrophoneMuted(aMicrophoneMuted);
> > +    mIsMicMuted = aMicrophoneMuted;
> > +    return NS_OK;
> > +  }
> 

It's ok.

> I am sorry to bring this question late.
> 
> If the mute is came from audio recording app and video conference app in the
> future (not in voice_call state.)
> Is sending mute to ril really workable?
> Or we need to enable mute from RIL and AudioSystem.

Sam will check and answer your question.
Flags: needinfo?(james.zhang) → needinfo?(sam.hua)
Attachment #8339029 - Flags: review?(mchen)
Comment on attachment 8339029 [details] [diff] [review]
Bug 931722 - Part2: extra mute request to RIL at AudioManager. [Fugu]Can't mute in the call. v5.

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

It looks good for me now. r=me.

And please wait for confirmation from partner then asking for landing.
Thanks.
Attachment #8339029 - Flags: review?(mchen) → review+
Attached patch 931722.test.4.patch (obsolete) — Splinter Review
Attachment #8338388 - Attachment is obsolete: true
(In reply to James Zhang from comment #55)
> (In reply to Marco Chen [:mchen] from comment #51)
> > If the mute is came from audio recording app and video conference app in the
> > future (not in voice_call state.)
> > Is sending mute to ril really workable?
> > Or we need to enable mute from RIL and AudioSystem.
> 
> Sam will check and answer your question.

1. audio recording app don't have mute function now. and, it is controlled by app that send mute to Audio or RIL.

2. Video call should use RIL
Flags: needinfo?(sam.hua)
blocking-b2g: --- → fugu?
Keywords: checkin-needed
gonna check in to v1.2, any concern?
blocking-b2g: fugu? → koi+
Flags: needinfo?(fabrice)
Hi Sam:
 Please setup below property to make mute work on fugu.
(We are still waiting for code phase-in, should be ready by next week)

ro.moz.mute.call.to_ril=true

Thanks!!
sku
(In reply to shawn ku [:sku] from comment #67)
> Hi Sam:
>  Please setup below property to make mute work on fugu.
> (We are still waiting for code phase-in, should be ready by next week)
> 
> ro.moz.mute.call.to_ril=true
> 
> Thanks!!
> sku

OK
Flags: needinfo?(sam.hua)
(In reply to shawn ku [:sku] from comment #67)
> Hi Sam:
>  Please setup below property to make mute work on fugu.
> (We are still waiting for code phase-in, should be ready by next week)
> 
> ro.moz.mute.call.to_ril=true
> 
> Thanks!!
> sku

Done.
Status: UNCONFIRMED → NEW
Ever confirmed: true
This is going to need branch-specific patches for the b2g26 uplift.
Flags: needinfo?(sku)
(In reply to Steven Yang [:styang] from comment #66)
> gonna check in to v1.2, any concern?

I'm only sherrifing 1.3. You need to ask approval for 1.2 on the patch itself.
Flags: needinfo?(fabrice)
This one should go to 1.2f.
blocking-b2g: koi+ → fugu?
It should go to 1.3 also. Then it will be of benefit to new device.

--
Keven
Hi Ryan:
 As James/Keven said, this patch should go to m-c(including 1.3)/1.2f.

Please let me know if anything I missed.
Thanks!!
sku
Flags: needinfo?(sku) → needinfo?(ryanvm)
It's still going to need branch-specific patches for 1.2f of b2g26. It's already in 1.3 (m-c).
Flags: needinfo?(ryanvm) → needinfo?(sku)
Thanks Ryan, I will prepare a patch and upload to bugzilla tomorrow (due to I am OOO today).
Attached patch 931722.1.2f.test.patch (obsolete) — Splinter Review
Try result -
https://tbpl.mozilla.org/?tree=Try&rev=e6a9a0812f52

Hi Ryan:
 I merge the patch based on 1.2f repo.
Please help me check if anything I missed.

Thanks!!
shawn
Flags: needinfo?(ryanvm)
This doesn't have fugu+ blocking status yet. Once it does and the status-b2g-v1.2f flag is created, I'll uplift it. I'll point out that koi+ status (which it had at one point) would have gotten it on to v1.2 (which then merges to v1.2f automatically).
Flags: needinfo?(ryanvm)
Hi! Steven,

Please set fugu+. Thanks.

--
Keven
Flags: needinfo?(styang)
blocking-b2g: fugu? → fugu+
Flags: needinfo?(styang)
Hi shawn,
anything should i do for this bug?
Flags: needinfo?(sam.hua)
Hi Sam:
 If three "[Patch for 1.2f]..." are all merged into 1.2f, there is nothing to do with this bug.
Thanks!!
sku
Whiteboard: [Fugu] [v1.2f-uplift-needed]
You need to log in before you can comment on or make changes to this bug.