Closed Bug 1157121 Opened 10 years ago Closed 9 years ago

[B2G] The speaker status is wrong, when we resume the FM.

Categories

(Firefox OS Graveyard :: AudioChannel, defect)

defect
Not set
normal

Tracking

(firefox39 wontfix, firefox40 wontfix, firefox41 fixed, b2g-v2.1 verified, b2g-v2.1S verified, b2g-v2.2 verified, b2g-master verified)

RESOLVED FIXED
2.2 S13 (29may)
Tracking Status
firefox39 --- wontfix
firefox40 --- wontfix
firefox41 --- fixed
b2g-v2.1 --- verified
b2g-v2.1S --- verified
b2g-v2.2 --- verified
b2g-master --- verified

People

(Reporter: alwu, Assigned: alwu)

References

Details

(Keywords: regression)

Attachments

(2 files, 4 obsolete files)

Only happen on v2.1. STR. 1. Switch routing to the speaker - speaker-switch button lights is on 2. Stop FM 3. Resume FM, the routing is still out from the speaker - speaker-switch button lights is off (ERROR)
Assignee: nobody → alwu
Attached patch [v2.1] Update speaker status (obsolete) (deleted) — Splinter Review
(In reply to Alastor Wu [:alwu] from comment #1) > Created attachment 8597078 [details] [diff] [review] > [v2.1] Update speaker status > > Wait for try-server result. > https://treeherder.mozilla.org/#/jobs?repo=try&revision=5f480a0ae209 Hi Alastor, We need to modify the codes in speaker manager API at the same time. With your patch in comment1,open fm, play fm ,open speaker,go back to homescreen,fm will play in headset other than speaker ;(
Flags: needinfo?(alwu)
Attached patch [v2.1] Update speaker status (obsolete) (deleted) — Splinter Review
Here is the revised patch. It seems that the v2.1 try-server has some questions. I will ask the review after finishing the test.
Attachment #8597078 - Attachment is obsolete: true
Flags: needinfo?(alwu)
Attached patch [v2.1] Update speaker status (obsolete) (deleted) — Splinter Review
Add the revised version.
Attachment #8597162 - Attachment is obsolete: true
Hi, Ryan, On comment5, there are lots crashed on the try-server even if I only commit the test to the try command. Do you know why? I don't think this changeset would caused such serious error. Thanks a lots!
Flags: needinfo?(ryanvm)
Try pushes without any changes are redundant. Please save resources and just look at the b2g34 tree on Treeherder. https://treeherder.mozilla.org/#/jobs?repo=mozilla-b2g34_v2_1 The results on your Try push look fine considering that Try attempts to run everything that mozilla-central supports, which is substantially more than b2g34 does. All you need to verify is that the suites that run on b2g34 at the link above are still passing with your patch. Bonus points for using better Try syntax to avoid running extra builds/tests in the first place. https://wiki.mozilla.org/Sheriffing/How:To:Recommended_Try_Practices
Flags: needinfo?(ryanvm)
In v2.1, the condition of the issue is in the description. In v2.0/v2.2/m-c, the speaker status would be reset to the headphone. That is all because of we forgot to check the speaker status. In bug1089526, I only add the checking in AudioChannelService, but forgot to add it in the AudioChannelServiceChild.
Summary: [v2.1] The speaker-switch button of FM is on the wrong status, when we resume the FM. → [B2G] The speaker status is wrong, when we resume the FM.
Attached patch Update speaker status (obsolete) (deleted) — Splinter Review
Attachment #8597168 - Attachment is obsolete: true
Try-server result. It seems fine, except the last one mochitest. However, I also saw the crash on other people's test, I think this is not resulted by my patch. https://treeherder.mozilla.org/#/jobs?repo=try&revision=008b181f599e
Comment on attachment 8599216 [details] [diff] [review] Update speaker status Hi, Baku, Could you help me review this patch :)? The details of this issue is in the comment8. Very appreciate!
Attachment #8599216 - Flags: review?(amarchesini)
Comment on attachment 8599216 [details] [diff] [review] Update speaker status Review of attachment 8599216 [details] [diff] [review]: ----------------------------------------------------------------- can you help me to bring this code to the new implementation of AudioChannelService? ::: dom/audiochannel/AudioChannelService.cpp @@ +378,5 @@ > + * (1) apps in the foreground. > + * (2) apps in the backgrund and inactive. > + * Notice : check the state when the visible status is stable, because there > + * has lantency in passing the visibility events. > + **/ same here. ::: dom/audiochannel/AudioChannelServiceChild.cpp @@ +99,5 @@ > + * (1) apps in the foreground. > + * (2) apps in the backgrund and inactive. > + * Notice : modify only when the visible status is stable, because there > + * has lantency in passing the visibility events. > + **/ indentation of this last line.
Attachment #8599216 - Flags: review?(amarchesini) → review+
Attached patch Update speaker status. r=baku. (deleted) — Splinter Review
Sure, I would help to rebase it after landing the new architecture :)
Attachment #8599216 - Attachment is obsolete: true
Attachment #8611127 - Flags: review+
Comment on attachment 8611127 [details] [diff] [review] Update speaker status. r=baku. This bug is the regression error of bug1089526, and that bug was landed on v2.1/v2.2. Therefore, I need to uplift the changeset to these versions. [Approval Request Comment] Bug caused by (feature/regressing bug #): Forget to check the speaker status User impact if declined: The FM status would be not correct Testing completed: Yes Risk to taking this patch (and alternatives if risky): None String or UUID changes made by this patch: None
Attachment #8611127 - Flags: approval-mozilla-b2g37?
Attachment #8611127 - Flags: approval-mozilla-b2g34?
The try-server result is on the comment10.
Keywords: checkin-needed
Comment on attachment 8611127 [details] [diff] [review] Update speaker status. r=baku. Requesting QA verify on 2.1/2.2/master after patch landed
Attachment #8611127 - Flags: approval-mozilla-b2g37?
Attachment #8611127 - Flags: approval-mozilla-b2g37+
Attachment #8611127 - Flags: approval-mozilla-b2g34?
Attachment #8611127 - Flags: approval-mozilla-b2g34+
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S13 (29may)
Attached video Verify.mp4 (deleted) —
This bug has been verified as pass on latest build of Flame v2.1/v2.2/master, Nexus5 v2.2/master and 2.1s_256mb/2.1s_512mb by the STR in Comment 0. Actually Result: speaker-switch button lights is on. Reproduce rate: 0/10 See attachment: Verify.mp4 Device:Flame v2.1 build(pass) Build ID 20150531001203 Gaia Revision 2304a1f6327c2ccf35d6995ee16f2231ed1f22a3 Gaia Date 2015-05-26 13:30:41 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/e52807dee101 Gecko Version 34.0 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150531.035411 Firmware Date Sun May 31 03:54:22 EDT 2015 Bootloader L1TC000118D0 Device:Flame v2.2 build(pass) Build ID 20150531162502 Gaia Revision b4582cc394e0919623263997c0cdb0b4751a1403 Gaia Date 2015-05-31 11:06:34 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/78d8b0a4303d Gecko Version 37.0 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150531.195816 Firmware Date Sun May 31 19:58:28 EDT 2015 Bootloader L1TC000118D0 Device:Flame master build(pass) Build ID 20150531160203 Gaia Revision e6dc0f4c583407a4a52a66ce7cb11f058302a762 Gaia Date 2015-05-29 17:20:26 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/f8d21278244b Gecko Version 41.0a1 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150531.192324 Firmware Date Sun May 31 19:23:35 EDT 2015 Bootloader L1TC000118D0 Device:Nexus 5 v2.2 build(pass) Build ID 20150531002502 Gaia Revision 0a46394dbee0ed2eb71a136cee38ddd8549dd597 Gaia Date 2015-05-30 14:50:16 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/ed2f6aeb1d81 Gecko Version 37.0 Device Name hammerhead Firmware(Release) 5.1 Firmware(Incremental) eng.cltbld.20150531.043812 Firmware Date Sun May 31 04:38:27 EDT 2015 Bootloader HHZ12f Device:Nexus 5 master build(pass) Build ID 20150531160203 Gaia Revision e6dc0f4c583407a4a52a66ce7cb11f058302a762 Gaia Date 2015-05-29 17:20:26 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/f8d21278244b Gecko Version 41.0a1 Device Name hammerhead Firmware(Release) 5.1 Firmware(Incremental) eng.cltbld.20150531.192050 Firmware Date Sun May 31 19:21:07 EDT 2015 Bootloader HHZ12f Device:2.1s_256mb build(pass) Build ID 20150531001204 Gaia Revision 0493b4dbb59f9617c1c1a45f422303c2aff32a9a Gaia Date 2015-05-27 19:33:12 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/1aae9fc735b5 Gecko Version 34.0 Device Name scx15_sp7715ga Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150531.035115 Firmware Date Sun May 31 03:51:25 EDT 2015 Device:2.1s_512mb build(pass) Gaia-Rev 2def4255c59065e3ac9265bb165adc1550398477 Gecko-Rev https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/1aae9fc735b5 Build-ID 20150531001204 Version 34.0 Device-Name scx15_sp7715ea FW-Release 4.4.2 FW-Incremental 122 FW-Date Thu Feb 5 12:42:58 CST 2015
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: