1014903 - [B2G][Open_C] When the 3rd person in a group call ends the call, the remaining devices transmit no sound

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Josh Schmitt [Joshs]]

Attachment: log.txt

Description:
After the 3rd person ends the call while in a group call, no sound can be heard from the original phone call between device 1 and device 2.

Repro Steps:
1) Update a Open_C to BuildID: 20140522000200
2) With the DUT call device 2
3) With DUT call device 3 
4) Merge the calls together to make a Group call
5) Have device 3 end the call

Actual:
Device 1 and Device 2 cannot hear sounds from each other after the 3rd person leaves.

Expected:
Device 1 and Device 2 can hear sound after the 3rd person leaves.

1.4 Environmental Variables:
Device: Open_C 1.4
BuildID: 20140522000200
Gaia: 233dd43b3b976e66a619dbc1b4044ed1e3ca3e34
Gecko: c3c0c57daef8
Version: 30.0
Firmware Version: P821A10V1.0.0B06_LOG_DL

Notes:
Repro frequency: 100%
See attached: logcat

Comment 1

[Josh Schmitt [Joshs]]

Tested on 1.4 Buri and 1.3 Buri, issue does NOT repro.

1.4 Environmental Variables:
Device: Buri 1.4 MOZ
BuildID: 20140522000200
Gaia: 233dd43b3b976e66a619dbc1b4044ed1e3ca3e34
Gecko: c3c0c57daef8
Version: 30.0
Firmware Version: v1.2-device.cfg

Comment 2

[Josh Schmitt [Joshs]]

Adding qawanted to test on Open_C Base.

Comment 3

[Lionel Mauritson]

This does NOT reproduce when using the Open_C 1.3 base build as the DUT.

Firmware Version: P821A10V1.0.0B06_LOG_DL

Comment 4

[Jason Smith [:jsmith]]

This sounds quite bad. Definitely something that would block certification.

Comment 5

[Lionel Mauritson]

The issue occurs on the earliest build I have access to.

Device: Open_C
Build ID: 20140417000006
Gaia: 7591e9dc782ac2e97d63a96f9deb71c7b3588328
Gecko: e71ed4135461
Version: 31.0a1  
Firmware Version: P821A10V1.0.0B06_LOG_DL

Other devices: iPhone 4S and Open_C 1.4

Comment 6

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Not sure if this is on dialer app.
Also according to comment1, it doesn't happen on Buri either 1.3 or 1.4.

Comment 7

[Anthony Ricaud (:rik)]

If it does not reproduce on Buri then it's probably a RIL or Vendcom issue.

Szu-Yu: Can you take a look?

Comment 8

[Szu-Yu Chen [:aknow] (inactive)]

Sure, I will check it. It could be reproduced on Flame.

Comment 9

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: Handle conference call state change properly

Bug reason:
In telephonyService, we get callStateChange first than conferenceCallStateChange. Then we turn off the audio because the conference is disconnected. However at that time, we still have an active normal call.

The patch aim to handle the audio state change properly.

Comment 10

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: Handle audio state change properly

The previous patch doesn't solve the problem correctly.

Comment 11

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 8436788 [details] [diff] [review]
Handle audio state change properly

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

Looks great, thank you.

::: dom/telephony/gonk/TelephonyService.js
@@ +220,2 @@
>  
> +    if (incoming && this._numActiveCall === 0) {

nit: s/this._numActiveCall === 0/!this._numActiveCall/

@@ +227,5 @@
> +      this._updateCallAudioState(AUDIO_STATE_NO_CALL);
> +    }
> +  },
> +
> +  _updateCallAudioState: function(audioState) {

s/audioState/aAudioState/

Comment 12

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: [final] Part 1: Handle audio state change properly. r=hsinyi

Comment 13

[Anthony Ricaud (:rik)]

I'm concerned that this patch has no new tests :(

Comment 14

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Anthony Ricaud (:rik) from comment #13)
> I'm concerned that this patch has no new tests :(

Thanks for the comment, rik.

Actually, the emulator cannot detect this issue because it is triggered due to different modem behaviour. However, we can still enhance the test [1] to make sure it works fine on emulator.

[1] http://dxr.mozilla.org/mozilla-central/source/dom/telephony/test/marionette/test_audiomanager_phonestate.js?from=test_audiomanager_phonestate.js&case=true#

Comment 15

[Szu-Yu Chen [:aknow] (inactive)]

https://tbpl.mozilla.org/?tree=Try&rev=7a0c4e963e26

Comment 16

[Szu-Yu Chen [:aknow] (inactive)]

Anthony,
Thank you for the suggestion. I understood your concern. However, even we could enhance that test, we still cannot simulate the same behavior unless we make some change in the emulator...
I prefer to land the patch first. Then we could think about how to handle this kind of test requirement.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/ed6bd9e51cc7

Comment 18

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: Part 2: Get active call by query

Comment 19

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: (1.4) Handle audio state change properly

Hsinyi,

Please help me review the v1.4 version of the patch. We should be careful of some differences between central and v1.4.
- 1. placeholder callIndex
- 2. isActive is not yet removed

Comment 20

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 8438996 [details] [diff] [review]
Part 2: Get active call by query

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

::: dom/telephony/Telephony.cpp
@@ +210,1 @@
>        aCallState == nsITelephonyService::CALL_STATE_CONNECTED;

Also CALL_STATE_DISCONNECTING?

Comment 21

[Ryan VanderMeulen [:RyanVM]]

This was merged to m-c.
https://hg.mozilla.org/mozilla-central/rev/ed6bd9e51cc7

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/6e0f4138959e

Though I guess this really isn't fixed yet due to Part 2 not landing anywhere?

Comment 23

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: [final] Part 2: Get active call by query. r=hsinyi

Comment 24

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 8439055 [details] [diff] [review]
(1.4) Handle audio state change properly

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

Looks good! I am still waiting for part 2 for v1.4 :)

Comment 25

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: (1.4) Part 2: Get active call by query

Comment 26

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 8439702 [details] [diff] [review]
(1.4) Part 2: Get active call by query

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

Looks good!

Comment 27

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: (1.4) [final] Part 1: Handle audio state change properly. r=hsinyi

Comment 28

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: (1.4) [final] Part 2: Get active call by query. r=hsinyi

Comment 29

[Szu-Yu Chen [:aknow] (inactive)]

check-in-needed

for central: part 2
https://tbpl.mozilla.org/?tree=Try&rev=e3b1a90dc495

for 1.4: part 1, 2
https://tbpl.mozilla.org/?tree=Try&rev=ec64b543fc59

Comment 30

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/f6669e8d7891

Comment 31

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/f6669e8d7891

Comment 32

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/f10c7d6face5

https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/66f478d88f6a
https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/3e030f473eae

Comment 33

[Coler]

This issue has been verified successfully on Flame2.0&2.1.

Reproducing rate: 0/5
See attachment: Verify_Flame_Callsound.mp4

Flame 2.0 build version:
Gaia-Rev        8d1e868864c8a8f1e037685f0656d1da70d08c06
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/c756bd8bf3c3
Build-ID        20141127000203
Version         32.0

Flame2.1 build version:
Gaia-Rev        5372b675e018b6aac97d95ff5db8d4bd16addb9b
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/f34377ae402b
Build-ID        20141127001201
Version         34.0

Comment 34

[Coler]

Attachment: Verify_Flame_Callsound.MP4