975955 - [Sora][Streaming]the audio gaps after lock screen

Status: VERIFIED FIXED

(Core :: Audio/Video, defect, P1)

Description

[sync-1]

Mozilla build ID: 20140208004002
 
 DEFECT DESCRIPTION:
    play a stream on browser, the audio gaps after lock screen. regression.
  REPRODUCING PROCEDURES:
  1.Enable wifi and connected wifi;
  2.Launch browser and go to:http://imps.tcl-ta.com/liuqiong/media/e5_video.html;
  3.Select video media;
  4.Open any one streaming to play;
  5.Press power key to lock screen during playing; the audio gaps sometimes.->KO
  EXPECTED BEHAVIOUR:
    the stream is still playing smooth after press powser key. 
  ASSOCIATE SPECIFICATION:
 
  TEST PLAN REFERENCE:
 
  TOOLS AND PLATFORMS USED:
 
  USER IMPACT:
 
  REPRODUCING RATE:
 
  For FT PR, Please list reference mobile's behavior:

Comment 1

[Jason Smith [:jsmith]]

This sounds horrible on device. See Vance's video for why - http://www.youtube.com/watch?v=OVFj0FgY030&feature=youtu.be.

Comment 2

[Jason Smith [:jsmith]]

This appears to only reproduce on Jellybean devices, not ICS devices. That means getting a regression window isn't going to be possible - we don't have few builds available for JB testing.

Comment 3

[bhavana bajaj [:bajaj]]

No CJ/:mwu, to weigh in here and help narrow down the problem..This is JB specific so not sure if its a gonk issue? 

Per triage discussion with QA this was easily reproducible on any youttube video and seemed like a common use case.

Comment 4

[Gregor Wagner [:gwagner]]

Is the lockscreen messing with the cpu lock?

Comment 5

[Sotaro Ikeda [:sotaro]]

I confirmed the problem happens on nexus-4 and nexus-5. When usb is plugged to the device the problem does not happen. From it, it seems that some cpu cores are inactive and/or cpu clock down when display is off and usb is not plugged. Then hardware does not have enough cpu power to handle streaming.

Comment 6

[Blake Wu [:bwu][:blakewu]]

May I know the correct UX spec? 
IMO, this playback should be stopped after pressing power key.

Comment 7

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

May refer to this spec: https://wiki.mozilla.org/WebAPI/AudioChannels

Comment 8

[Jason Smith [:jsmith]]

(In reply to Blake Wu [:bwu] from comment #6)
> May I know the correct UX spec? 
> IMO, this playback should be stopped after pressing power key.

If I recall, there has been a lot of debate on this one. The current behavior allows the audio to play in the background while the phone is sleeping.

Note - the blocking factor on this bug is the factor that there's audio gaps present, not the fact that the audio is playing in the background while the phone is sleeping.

Comment 9

[Blake Wu [:bwu][:blakewu]]

(In reply to Jason Smith [:jsmith] from comment #8)
> (In reply to Blake Wu [:bwu] from comment #6)
> > May I know the correct UX spec? 
> > IMO, this playback should be stopped after pressing power key.
> 
> If I recall, there has been a lot of debate on this one. The current
> behavior allows the audio to play in the background while the phone is
> sleeping.
> 
> Note - the blocking factor on this bug is the factor that there's audio gaps
> present, not the fact that the audio is playing in the background while the
> phone is sleeping.
Agree. 
Just want to know the correct UX behavior first, not to fix a problem from an unreasonable use case. I just found there is already a bug 971414 created for this UX issue discussion.

Comment 10

[Danny Liang [:dliang]]

Update test result on flame.
1. CPU frequency almost stayed at 300MHz and CPU usage is lower than 60% when screen off playback w/ USB cable. It showed we don't need too much CPU when screen off playback.
2. Check dmesg log when issue happened, we found system is trying to suspend. It should be caused by no one hold proper wakelock.
3. Use command "adb shell "echo test > /sys/power/wake_lock"" to hold a wakelock, the audio is smooth when screen off playback w/o usb cable

Moreover, connect USB cable, it caused system cannot enter suspend due to wakelock held, so screen off playback is smooth.

Comment 11

[Danny Liang [:dliang]]

Attachment: cpu_info.tar.gz

Attachment is the test result and log.

Comment 12

[Star Cheng [:scheng]]

(In reply to Danny Liang [:dliang] from comment #10)
> Update test result on flame.
> 1. CPU frequency almost stayed at 300MHz and CPU usage is lower than 60%
> when screen off playback w/ USB cable. It showed we don't need too much CPU
> when screen off playback.
> 2. Check dmesg log when issue happened, we found system is trying to
> suspend. It should be caused by no one hold proper wakelock.
> 3. Use command "adb shell "echo test > /sys/power/wake_lock"" to hold a
> wakelock, the audio is smooth when screen off playback w/o usb cable
> 

In hamachi & flame device, the /sys/power/wake_lock is empty, in the reproduce step of this bug . And the audio stream is played smoothly in hamachi, not smoothly in flame instead.
 
In hamachi & flame device, the /sys/power/wake_lock is "gecko" in playing audio file in music app.

> Moreover, connect USB cable, it caused system cannot enter suspend due to
> wakelock held, so screen off playback is smooth.

Comment 13

[Danny Liang [:dliang]]

(In reply to Star Cheng [:scheng] from comment #12)
> (In reply to Danny Liang [:dliang] from comment #10)
> > Update test result on flame.
> > 1. CPU frequency almost stayed at 300MHz and CPU usage is lower than 60%
> > when screen off playback w/ USB cable. It showed we don't need too much CPU
> > when screen off playback.
> > 2. Check dmesg log when issue happened, we found system is trying to
> > suspend. It should be caused by no one hold proper wakelock.
> > 3. Use command "adb shell "echo test > /sys/power/wake_lock"" to hold a
> > wakelock, the audio is smooth when screen off playback w/o usb cable
> > 
> 
> In hamachi & flame device, the /sys/power/wake_lock is empty, in the
> reproduce step of this bug . And the audio stream is played smoothly in
> hamachi, not smoothly in flame instead.
>  
> In hamachi & flame device, the /sys/power/wake_lock is "gecko" in playing
> audio file in music app.
> 
> > Moreover, connect USB cable, it caused system cannot enter suspend due to
> > wakelock held, so screen off playback is smooth.

In hamachi, system didn't try to suspend when screen off playback, it might be some kernel wake lock has been held. If no this lucky kernel wake lock, hamachi will also meet this issue. To avoid the issue on different platform, suggest app to hold its own wake lock.

Comment 14

[Star Cheng [:scheng]]

Attachment: bug-975955-fix.patch

While playing video streaming, the /sys/power/wake_lock is empty. I try to hold a "cpu" wake_lock in the HtmlVideoElement.

Comment 15

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 8385942 [details] [diff] [review]
bug-975955-fix.patch

Andrea, can you find a reviewer for this?

Comment 16

[Star Cheng [:scheng]]

Attachment: bug-975955-fix_v2.patch

1.keep the "cpu" wake lock in HTMLVideoElement::WakeLockCreate()
2.release the "cpu" wake lock in HTMLVideoElement::WakeLockRelease()

Comment 17

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8388985 [details] [diff] [review]
bug-975955-fix_v2.patch

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

Thanks.

Please make another patch to rename WakeLockUpdate to UpdateScreenWakeLock.

Comment 18

[Star Cheng [:scheng]]

Attachment: bug-975955-patch-reviewed

1.try server result : https://tbpl.mozilla.org/?tree=Try&rev=42959763fac5
2.add reviewer

Comment 19

[Star Cheng [:scheng]]

Attachment: bug-975955-patch-reviewed

remove ":" for reviewer name

Comment 20

[Ryan VanderMeulen [:RyanVM]]

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

Comment 21

[Ryan VanderMeulen [:RyanVM]]

Backed out for the same test failures that your Try run showed. Please verify that the run is actually passing *before* requesting checkin and wasting both people and machine time.
https://hg.mozilla.org/integration/b2g-inbound/rev/f4f2901282ea

https://tbpl.mozilla.org/php/getParsedLog.php?id=36331400&tree=B2g-Inbound

Comment 22

[Bruce Sun [:brsun]]

FYI, content/html/content/test/test_video_wakelock.html needs to be updated for checking both "screen" and "cpu" wake-lock topics.

Comment 23

[Star Cheng [:scheng]]

Attachment: bug-975955-testcase.patch

To modify test case for video playback in order to get/release "cpu" wakelock.

Comment 24

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8395552 [details] [diff] [review]
bug-975955-testcase.patch

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

Make sure you do another try run that actually passes.

::: content/html/content/test/test_video_wakelock.html
@@ +47,5 @@
>                   state == "locked-background";
>  
> +    if (topic == "cpu") {
> +      is(locked, lockState_cpu, "Video element locked the cpu - paused");
> +       count_cpu++;				 

remove trailing spaces, and fix indent.

@@ +89,5 @@
>                   state == "locked-background";
>  
> +    if (topic == "cpu") {
> +      is(locked, lockState_cpu, "Video element locked the cpu - paused");
> +      count_cpu++;				 

remove trailing spaces

Comment 25

[Star Cheng [:scheng]]

Attachment: bug-975955-testcase-patch-reviewed

1.try server result : https://tbpl.mozilla.org/?tree=Try&rev=b5795d0554e4
2.add reviewer - :roc

Comment 26

[Star Cheng [:scheng]]

there are 2 patches for this bug.

1.bug-975955-patch-reviewed 
2.bug-975955-testcase-patch-reviewed

Comment 27

[Ryan VanderMeulen [:RyanVM]]

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

Comment 28

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

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

Comment 29

[Ryan VanderMeulen [:RyanVM]]

Please request approval-mozilla-b2g28 on this patch if it's ready for uplift to v1.3.

Comment 30

[Star Cheng [:scheng]]

Comment on attachment 8392745 [details] [diff] [review]
bug-975955-patch-reviewed

NOTE: This flag is now for security issues only. 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 #): not a regression
User impact if declined: the sound of video stream is not playing smoothly after pressing power key
Testing completed: m-c and manual test in nexus4 & flame device
Risk to taking this patch (and alternatives if risky): low. while playing video, to get the cpu + screen lock; and release both while stopping/pausing the video. 
String or UUID changes made by this patch: none

Comment 31

[Star Cheng [:scheng]]

Comment on attachment 8396112 [details] [diff] [review]
bug-975955-testcase-patch-reviewed

NOTE: This flag is now for security issues only. 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 #): not a regression
User impact if declined: test case failure
Testing completed: verify ok in try server (see comment 25 - 1.try server result)
Risk to taking this patch (and alternatives if risky): low. Because basing on original test case to add another test item for "cpu" wakelock.
String or UUID changes made by this patch: none

Comment 32

[bhavana bajaj [:bajaj]]

Comment on attachment 8392745 [details] [diff] [review]
bug-975955-patch-reviewed

Requesting QA ro verify the audio gap issue, once this lands.

Comment 33

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8b29be28b761
https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/350c1dd1b5be

Comment 34

[Ada [:adalucinet]]

At a first sight, this problem seems do not apply for Desktop Firefox.
Star Cheng, could you please give me feedback on this?

Comment 35

[Star Cheng [:scheng]]

(In reply to Alexandra Lucinet, QA Mentor [:adalucinet] from comment #34)
> At a first sight, this problem seems do not apply for Desktop Firefox.
> Star Cheng, could you please give me feedback on this?


As I know, there is no "wake lock" mechanism on PC. Could you describe what is the user story that you expect?
 
Thanks

Comment 36

[Ada [:adalucinet]]

I was asking just to make sure that this issue cannot be reproduced and verified on Desktop Firefox - based on the verifyme keyword.

Comment 37

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Star Cheng [:scheng] from comment #35)
> (In reply to Alexandra Lucinet, QA Mentor [:adalucinet] from comment #34)
> > At a first sight, this problem seems do not apply for Desktop Firefox.
> > Star Cheng, could you please give me feedback on this?
> 
> 
> As I know, there is no "wake lock" mechanism on PC. Could you describe what
> is the user story that you expect?

I'm not sure what you mean here, but we have WakeLockListeners to disable the screen saver in desktop Firefox on some platforms (they listen on the "screen" topic). Or do you mean a wake lock listening on the "cpu" topic? I don't think we have WakeLockListener for the "cpu" topic on desktop platforms yet.

Comment 38

[Star Cheng [:scheng]]

(In reply to Chris Pearce (:cpearce) from comment #37)
> (In reply to Star Cheng [:scheng] from comment #35)
> > (In reply to Alexandra Lucinet, QA Mentor [:adalucinet] from comment #34)
> > > At a first sight, this problem seems do not apply for Desktop Firefox.
> > > Star Cheng, could you please give me feedback on this?
> > 
> > 
> > As I know, there is no "wake lock" mechanism on PC. Could you describe what
> > is the user story that you expect?
> 
> I'm not sure what you mean here, but we have WakeLockListeners to disable
> the screen saver in desktop Firefox on some platforms (they listen on the
> "screen" topic). Or do you mean a wake lock listening on the "cpu" topic? I
> don't think we have WakeLockListener for the "cpu" topic on desktop
> platforms yet.


Yes, I mean "cpu" topic. To set the "cpu" wake lock to avoid entering sleep mode despite user press power key.

Hi, Chris

Is the WakeLockListeners which you mentions only for desktop firefox on Window OS?

[1] http://dxr.mozilla.org/mozilla-central/source/widget/windows/nsAppShell.cpp#32

Comment 39

[Chris Pearce [:cpearce (Not reading bugmail)]]

scheng: Yes. Windows only so far. Edwin is working on Linux right now in bug 811261. MacOSX in bug 772347, but no one is actively working on it.

Comment 40

[bmorgan]

1.3 Buri: Video continues to stream without problem when screen is locked and unlocked with power button
1.4 Buri: Video continues to stream without problem when screen is locked and unlocked with power button
2.0 Flame: Video continues to stream without problem when screen is locked and unlocked with power button

1.3 Environmental Variables:
Device: Buri 1.3 MOZ
BuildID: 20140528024003
Gaia: 0ce948e378cab7ed3db20231281dd7ca2eb99779
Gecko: ce0dd450bffb
Version: 28.0
Firmware Version: v1.2-device.cfg

1.4 Environmental Variables:
Device: Buri 1.4 MOZ
BuildID: 20140528000201
Gaia: cd595be0a8e975559e8938830df5face89bec3e8
Gecko: af180db8a4bf
Version: 30.0
Firmware Version: v1.2-device.cfg

2.0 Environmental Variables:
Device: Flame 2.0 MOZ
BuildID: 20140528040205
Gaia: bc6f07c149770c6e6dfbea941ac65138dc364a15
Gecko: e017c15325ae
Version: 32.0a1
Firmware Version: 10g-2

Comment 41

[pete siphantong [:Petes]]

Verified it on latest Tarako build

1.3T Environmental Variables:
Device: Tarako 1.3T
BuildID: 20140530014002
Gaia: e68858693b71d917c9c5ee7e215f7ceea04635f7
Gecko: 1945abae19ff
Version: 28.1
Firmware Version: SP6821a-Gonk-4.0-4-29

1.3 tarako: Video continues to stream without problem when screen is locked and unlocked with power button