910584 - [B2G][Helix][Audio][tianhuajian]The call mode can not be changed to Headset after insert the headset

Status: RESOLVED FIXED

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

Description

[tianhuajian]

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:

phone A is FFOS device , phone B is other phone
1. phone B call phone A
2. phone A connect the call
3. phone A chang the call mode to handfree 
4. insert the headset 


Actual results:

The phone A still in handfree mode


Expected results:

The phone A will changed to headset mode

Comment 1

[Evelyn Hung]

(In reply to tianhuajian from comment #0)
> 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:
> 
> phone A is FFOS device , phone B is other phone
> 1. phone B call phone A
> 2. phone A connect the call
> 3. phone A chang the call mode to handfree 
> 4. insert the headset 
> 
> 
> Actual results:
> 
> The phone A still in handfree mode
> 
I heard the ring tone from both phone and headset if I insert my headset when the phone is ringing.
Marco, is it our design?

> 
> Expected results:
> 
> The phone A will changed to headset mode

Comment 2

[Marco Chen [:mchen]]

(In reply to Evelyn Hung [:evelyn] from comment #1)
> I heard the ring tone from both phone and headset if I insert my headset
> when the phone is ringing.
> Marco, is it our design?

Yes, this is under design.
This can avoid user ignore this call because he/her forget this phone on the table is plugged headset.

Comment 3

[Marco Chen [:mchen]]

(In reply to tianhuajian from comment #0)
Hi,

My first thought is that this is under design too.
When BT Handsfree profile is connected and it is on the phone call now, AudioManager will set ForceUse to BT_SCO in USE_COMMUNICATION.
https://mxr.mozilla.org/mozilla-central/source/dom/system/gonk/AudioManager.cpp#244

So during the phone call routing path will be SCO even headset is detected.

Comment 4

[Marco Chen [:mchen]]

Hi UX team,

Could you give feedback about the behavior on bug description?
Thanks,

Comment 5

[Stephany Wilkes]

Flagging Francis to comment on the headset/speaker issue here as well.

Comment 6

[Francis Djabri [:djabber]]

(In reply to Evelyn Hung [:evelyn] from comment #1)
> (In reply to tianhuajian from comment #0)
> > 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:
> > 
> > phone A is FFOS device , phone B is other phone
> > 1. phone B call phone A
> > 2. phone A connect the call
> > 3. phone A chang the call mode to handfree 
> > 4. insert the headset 
> > 
> > 
> > Actual results:
> > 
> > The phone A still in handfree mode
> > 
> I heard the ring tone from both phone and headset if I insert my headset
> when the phone is ringing.
> Marco, is it our design?
> 
> > 
> > Expected results:
> > 
> > The phone A will changed to headset mode

As mentioned in bug #910038, I would expect that connecting a headset should turn off the speaker mode. 

Having said that, while a headset is connected and speaker mode is off, I think it is good practice for an incoming call to route to both the headset and speaker to cover the situations that Marco described above in comment #2.

Comment 7

[Francis Djabri [:djabber]]

> Having said that, while a headset is connected and speaker mode is off, I
> think it is good practice for an incoming call to route to both the headset
> and speaker to cover the situations that Marco described above in comment #2.

To be more specific here, an incoming call *ringtone* should route to both the speaker and a headset when a headset is connected.

Comment 8

[lecky]

Hi Macro,
-----My first thought is that this is under design too.
If it is a design , then how the voice route to the headset?
I think if the user insert the headset to the device , he wanted to hear the voice in headset.
Thanks

Comment 9

[Marco Chen [:mchen]]

(In reply to lecky from comment #8)
> Hi Macro,
> -----My first thought is that this is under design too.
> If it is a design , then how the voice route to the headset?
> I think if the user insert the headset to the device , he wanted to hear the
> voice in headset.
> Thanks

Easy. there are two icons in the dialer app. One is for enabling speaker and the other for BT SCO.
When headset is coming and BT_SCO is enabled, user can press the BT_SCO icon to disable BT_SCO.
Finally user can hear the voice came from headset.

Comment 10

[Marco Chen [:mchen]]

Since this behavior can be customized by each vendor and each UX export, I would suggest that it can be modified in dialer app.

Ex: dialer app can listen the headset in by AudioChannelManager then decide whether it want to disable BT_SCO or not.

Comment 11

[lecky]

(In reply to Marco Chen [:mchen] from comment #10)
> Since this behavior can be customized by each vendor and each UX export, I
> would suggest that it can be modified in dialer app.

Ex: dialer app can
> listen the headset in by AudioChannelManager then decide whether it want to
> disable BT_SCO or not.
This issue mentioned is wired headset.
And my question is : The phone has connect the call and in speaker mode ,why I insert the wired headset, the audio output can not change to headset?
Ex: Do you mean we modify the dialer app by ourselves to solve this issue?

Comment 12

[Marco Chen [:mchen]]

(In reply to lecky from comment #11)
> (In reply to Marco Chen [:mchen] from comment #10)
> This issue mentioned is wired headset.
> And my question is : The phone has connect the call and in speaker mode ,why
> I insert the wired headset, the audio output can not change to headset?
> Ex: Do you mean we modify the dialer app by ourselves to solve this issue?

Yes. I suggest to modify dialer app for solving this issue.

Comment 13

[lecky]

(In reply to Marco Chen [:mchen] from comment #12)
> (In reply to lecky from comment #11)
> (In reply to Marco Chen [:mchen] from
> comment #10)
> This issue mentioned is wired headset.
> And my question is :
> The phone has connect the call and in speaker mode ,why
> I insert the wired
> headset, the audio output can not change to headset?
> Ex: Do you mean we
> modify the dialer app by ourselves to solve this issue?

Yes. I suggest to
> modify dialer app for solving this issue.
Hi Macro,
I reclear my request, my request is in step4 the speaker icon should be dark and the audio output should change to headset.
Do you mean this change should we modify the dialer app?
1. phone B call phone A
2. phone A connect the call
3. phone A chang the call mode to handfree 
4. insert the headset 



But I think this issue is audio channel bug.
Our request is the phone is in speaking ,when I insert the wired headset , the audio output should be wired headset, it

Comment 14

[lecky]

Hi Macro,
Since this behavior can be reproduced by each vendor and each UX export, I want to know whether you have any fix plan of this issue?
Thanks.

(In reply to Marco Chen [:mchen] (Summit 10/03 ~10/08) from comment #10)
> Since this behavior can be customized by each vendor and each UX export, I
> would suggest that it can be modified in dialer app.

Comment 15

[Marco Chen [:mchen]]

(In reply to lecky from comment #14)
> Hi Macro,
> Since this behavior can be reproduced by each vendor and each UX export, I
> want to know whether you have any fix plan of this issue?
> Thanks.
> 

Yes, I still recommend to fix it on dialer app.

The dialer app can use the Web API called MozAudioChannelManager to monitor the headset state.
If it find headset is plugged in and speaker is forced to enabled then try to disable the speakerForce from telephony Web API can achieve what you want.

Hi Etienne,

Based on comment 6 from Francis, it recommend that
  1. in voic_call state.
  2. User toggle the speaker icon (gaia called telephony web api to enable speakerForce)
  3. user insert the headset.
  4. The voice call should be routed to headset.

The problem we need gaia help is that
  1. from step 2, the web api there force the audio routing to speaker no matter any other audio output device is exist or not.
  2. So we need Gaia's help to monitor the headset state by MozAudioChannelManager web api then decide to disable speakerForce from telephony web api.

Thanks.

Comment 16

[Etienne Segonzac (:etienne)]

Attachment: Pointer to gaia PR

Here's a patch, works well for me!

Comment 17

[Anthony Ricaud (:rik)]

Comment on attachment 814864 [details]
Pointer to gaia PR

Yup, this does the work.

I'm a bit sad that we call CallScreen.turnSpeakerOff() rather than CallsHandler.turnSpeakerOff() (which would update the CallScreen) but that's not something to fix in this bug.

Comment 18

[lecky]

Hi Etienne,
I can not find the  calls_handler.js and calls_handler_test.js in our build.
Can you help me check it?
Our build version is as following:
【Gaia commit ID*】: V1.1HD
【Gecko commit ID*】:  1.1.0 HD

(In reply to Etienne Segonzac (:etienne) from comment #16)
> Created attachment 814864 [details]
Pointer to gaia PR

Here's a patch,
> works well for me!

Comment 19

[Etienne Segonzac (:etienne)]

(In reply to lecky from comment #18)
> Hi Etienne,
> I can not find the  calls_handler.js and calls_handler_test.js in our build.
> Can you help me check it?

It didn't land on master yet (waiting for a green travis build).
But if it becomes a 1.1 blocker we will probably need a specific patch for 1.1, it won't uplift easily.

Comment 20

[Wayne Chang [:wchang]]

Thanks Etienne.

As this behaviour is consistent across v1.1 devices/releases, i'd recommend this change to be introduced on v1.2 onwards. Bumping to Koi?

(also noted comment 19 on possible large amount of work to land this on v1.1)

Comment 21

[Etienne Segonzac (:etienne)]

(In reply to Wayne Chang [:wchang] from comment #20)
> Thanks Etienne.
> 
> As this behaviour is consistent across v1.1 devices/releases, i'd recommend
> this change to be introduced on v1.2 onwards. Bumping to Koi?
> 
> (also noted comment 19 on possible large amount of work to land this on v1.1)

Cool, uplifting this patch to 1.2 should not be an issue.

Comment 22

[Etienne Segonzac (:etienne)]

https://github.com/mozilla-b2g/gaia/commit/1deb60f128b819212161de90205a6b5e1e97935d

Comment 23

[lecky]

hi Beatriz,
For this issue, Wayne Chang had said will be resolved in V1.2.
So please check whether you accept this result?
Thanks

Comment 24

[Beatriz Rodríguez [:brg]]

(In reply to lecky from comment #23)
> hi Beatriz,
> For this issue, Wayne Chang had said will be resolved in V1.2.
> So please check whether you accept this result?
> Thanks
You can take v1.2. We do not land this functionality in v1.1. Agree with comment 20.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

John, can you please assist with the uplift? :)

Comment 26

[Jason Smith [:jsmith]]

Conflict here - triage originally decided this was a not a blocker for release, but was certainly something we should consider taking on an approval.

Comment 27

[Beatriz Rodríguez [:brg]]

(In reply to Jason Smith [:jsmith] from comment #26)
> Conflict here - triage originally decided this was a not a blocker for
> release, but was certainly something we should consider taking on an
> approval.
I do not unerstand why koi+ was removed here. According to comment 21 the patch should fit in v1.2. Can we reset this to koi+?

Comment 28

[Jason Smith [:jsmith]]

(In reply to Beatriz Rodríguez [:brg] from comment #27)
> (In reply to Jason Smith [:jsmith] from comment #26)
> > Conflict here - triage originally decided this was a not a blocker for
> > release, but was certainly something we should consider taking on an
> > approval.
> I do not unerstand why koi+ was removed here. According to comment 21 the
> patch should fit in v1.2. Can we reset this to koi+?

Well, right. That means you should ask for approval to get this uplifted to 1.2. There was bug conflict that happened in triage with setting the blocking flag. It was intended as a not a blocker for release, but we could definitely ask for approval and take this onto 1.2.

Comment 29

[Beatriz Rodríguez [:brg]]

We need this fix in v1.2. Nominating accordingly to koi+

Comment 30

[Jason Smith [:jsmith]]

(In reply to Beatriz Rodríguez [:brg] from comment #29)
> We need this fix in v1.2. Nominating accordingly to koi+

I think we made it very clear that this isn't a blocker for release. Please ask for approval.

Comment 31

[Jason Smith [:jsmith]]

Etienne - Can you nominate this for uplift?

Comment 32

[Etienne Segonzac (:etienne)]

(In reply to Jason Smith [:jsmith] from comment #31)
> Etienne - Can you nominate this for uplift?

Can I? :)
Not sure how to do this...

The patch is safe but I don't have a strong opinion (and I believe in the trains ;)).

Comment 33

[Jason Smith [:jsmith]]

(In reply to Etienne Segonzac (:etienne) from comment #32)
> (In reply to Jason Smith [:jsmith] from comment #31)
> > Etienne - Can you nominate this for uplift?
> 
> Can I? :)
> Not sure how to do this...
> 
> The patch is safe but I don't have a strong opinion (and I believe in the
> trains ;)).

You can do this by setting the flag "approval-gaia-v1.2" under details. Then, fill out the associated form.

Comment 34

[Etienne Segonzac (:etienne)]

(In reply to Jason Smith [:jsmith] from comment #33)
> You can do this by setting the flag "approval-gaia-v1.2" under details.
> Then, fill out the associated form.

Thanks.
Like I said, don't have a strong opinion on this one, I'll let Beatriz ask for approval if needed.

Comment 35

[Beatriz Rodríguez [:brg]]

Comment on attachment 814864 [details]
Pointer to gaia PR

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined: user cannot use the headset during a call
[Testing completed]:
[Risk to taking this patch] (and alternatives if risky): read comment 32
[String changes made]:

Comment 36

[Jason Smith [:jsmith]]

One other note with the approval request - there's no string changes with this patch. The patch is simple with unit tests included. It looks safe to take into 1.2.

Comment 38

[Preeti Raghunath(:Preeti)]

Comment on attachment 814864 [details]
Pointer to gaia PR

We've bee looking for this patch for a while now. I've approved it for 1.2

Comment 39

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted 1deb60f128b819212161de90205a6b5e1e97935d to:
v1.2: ff498298899bd60ecafc286f0dd780aa3331399d

Comment 40

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted 1deb60f128b819212161de90205a6b5e1e97935d to:
v1.3 already had this commit