1037383 - [QRD 256MB] Delay of 4-6 sec observed while playing recorded video for the first time

Status: RESOLVED FIXED

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

Description

[Pooja]

It happens only with 480p recording profile,with other lower recording profile like cif, low, qif playback is fast.

Issue was not present on v1.3  and v1.4
Target: 8x10 QRD target with 256MB  memory.

STR:
1. Open camera app
2. Switch to recording. start recording say for 10-15sec min then stop recording
3. Tap on generated filmstrip
4. Play recorded video

Actual behavior:
Grey screen stays on the display for 4-5 sec and then playback starts.


Expected behavior:
Videoplayback should start without any noticeable delay.

Comment 1

[Jason Smith [:jsmith]]

QA Wanted - Can this be reproduced on Flame?

Comment 2

[Pi Wei Cheng [:piwei] (inactive)]

(In reply to Jason Smith [:jsmith] from comment #1)
> QA Wanted - Can this be reproduced on Flame?

On Flame with 273mb mem, recording a video immediately resulted in:
FF OS rebooted itself x 2
Camera app exited itself x3
(total of 5 attempts)

Therefore we can't fully follow the STR.

Tested on:
Device: Flame 2.0
Build ID: 20140711112114
Gaia: c24d115bb51e40d5cb28c49324102b893966944b
Gecko: 1016a9d4fa70
Version: 32.0a2 (2.0)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 3

[Mike Habicher [:mikeh] (high bugzilla latency)]

(In reply to Pi Wei Cheng [:piwei] from comment #2)
> (In reply to Jason Smith [:jsmith] from comment #1)
> > QA Wanted - Can this be reproduced on Flame?
> 
> On Flame with 273mb mem, recording a video immediately resulted in:
> FF OS rebooted itself x 2
> Camera app exited itself x3
> (total of 5 attempts)

273MB mem is nowhere near enough to record video on Flame. See: https://bugzilla.mozilla.org/show_bug.cgi?id=1027592#c15

Comment 4

[bhargavg1]

(In reply to Mike Habicher [:mikeh] from comment #3)
> 273MB mem is nowhere near enough to record video on Flame. See:
> https://bugzilla.mozilla.org/show_bug.cgi?id=1027592#c15

What is the default resolution that's getting selected on flame?

Comment 5

[bhargavg1]

(In reply to bhargavg1 from comment #4)
> (In reply to Mike Habicher [:mikeh] from comment #3)
> > 273MB mem is nowhere near enough to record video on Flame. See:
> > https://bugzilla.mozilla.org/show_bug.cgi?id=1027592#c15
> 
> What is the default resolution that's getting selected on flame?

did we try forcing that to 480p.

1 quick way can be to edit /system/etc/media_profiles.xml and remove all references to 720p (height=720) for both camera1/camera2

Basically where ever Encoderprofile quality is high or 720p

Comment 6

[bhavana bajaj [:bajaj]]

Ni :mkieh, to see if we can pursue options listed in comment #5 , can we reduce the recording to 480px for 256 MB devices running 2.0. May be something similar to what we are doing here(https://bugzilla.mozilla.org/show_bug.cgi?id=1036637). CAF is defaulted their testing 480px on these low memory devices.

Comment 7

[Mike Habicher [:mikeh] (high bugzilla latency)]

Based on bug 1027592, it doesn't sound like 480p mode would alleviate this issue.

Comment 8

[Mike Habicher [:mikeh] (high bugzilla latency)]

Pooja, a couple of questions: when this happens, what is the output from the kernel log (captured ~10 seconds after you press the record button)?

# adb shell cat dmesg | grep sigkill

If you're seeing processes getting killed, please try increasing the amount of memory on the QRD device until nothing gets killed anymore. At that threshold, does this 4-6s issue reproduce? (One way of doing this is by making sure that everything works with 512MB, and doing a bisection to locate the thresholds where the kernel log starts to show processes getting killed.)

Comment 9

[Hema Koka [:hema]]

Mike will work with Kyle (Performance/Mem team) to further analyze this issue.

Comment 10

[Pooja]

Attachment: Daely of 1-2 sec in starting recording video with 286MB

Comment 11

[Pooja]

(In reply to Mike Habicher [:mikeh] from comment #8)
> Pooja, a couple of questions: when this happens, what is the output from the
> kernel log (captured ~10 seconds after you press the record button)?
> 
> # adb shell cat dmesg | grep sigkill
> 
> If you're seeing processes getting killed, please try increasing the amount
> of memory on the QRD device until nothing gets killed anymore. At that
> threshold, does this 4-6s issue reproduce? (One way of doing this is by
> making sure that everything works with 512MB, and doing a bisection to
> locate the thresholds where the kernel log starts to show processes getting
> killed.)

Recording duration 30sec

At 271Mb - received sigkill to Homescreen -Delay was 4-6 sec 

At 286 MB - No sigkill - delay reduced to 1-2 sec (Attached video with 286MB).

Comment 12

[Mike Habicher [:mikeh] (high bugzilla latency)]

(In reply to Pooja from comment #11)

> At 286 MB - No sigkill - delay reduced to 1-2 sec (Attached video with
> 286MB).

Are you referring to the grey screen before the video player appears? I see the same thing on Flame w/ 512MB of RAM, but there is nothing missing from the video.

David, is the grey screen part of the UI?

Comment 13

[Pooja]

Yes Mike. Its about how long that grey screen stays on UI.
With 271MB it was staying quite longer i.e 4-6 sec
But with increased memory it stays only fr 1-2 sec.

Also On other platform we don't see such grey or any screen before video starts. Need confirmation if or not its a part of UI.

Comment 14

[Mike Habicher [:mikeh] (high bugzilla latency)]

(In reply to Pooja from comment #13)

> Yes Mike. Its about how long that grey screen stays on UI.
> With 271MB it was staying quite longer i.e 4-6 sec
> But with increased memory it stays only fr 1-2 sec.

That makes sense: the tighter memory constraint would certainly make bringing up new UI components take longer. djf, is there anything we can do to speed up the switch-to-preview time.

Comment 15

[David Flanagan [:djf]]

The gray screen is not an intentional part of the API. I think that it is just the background that becomes visible while we hide the poster screen and display the actual video element. I imagine zmem is swapping or thrashing and this process just takes a really long time.

If it would help to resolve this bug, it might be possible to change the code (in shared/js/media/media_frame.js, probably) that does that swapping so that it first displays the video element and then removes the poster image...  But then instead of having a blank gray screen, we'd have a static poster image and the user would feel like nothing had happened when they tapped to play the video.

Comment 16

[Mike Habicher [:mikeh] (high bugzilla latency)]

(In reply to David Flanagan [:djf] from comment #15)

> If it would help to resolve this bug, it might be possible to change the
> code (in shared/js/media/media_frame.js, probably) that does that swapping
> so that it first displays the video element and then removes the poster
> image...  But then instead of having a blank gray screen, we'd have a static
> poster image and the user would feel like nothing had happened when they
> tapped to play the video.

It pains me to suggest this: but perhaps this is a job for a spinner?

Comment 17

[Justin D'Arcangelo [:justindarc]]

Stealing this from Mike to implement the dreaded spinner :-/

Comment 18

[Hema Koka [:hema]]

Justin is working on adding a visual indicator (spinner) in place of the gray screen. Lets keep this bug open and re-evaluate when the homescreen optimizations land. 

The other worst case option is to disable 480p recording. 

Thanks
Hema

Comment 19

[David Flanagan [:djf]]

Rather than adding ad-hoc spinners whenever this happens, it would be nice if we had a system-level spinner, like the MacOS beachball.  I wonder if there is some way that the gonk code that handles zmem swapping could draw some kind of beachball thing to the screen. Even if we can fix this particular bug, we are always going to have slowdown issues when memory is close to full and swapping happens.

Comment 20

[Justin D'Arcangelo [:justindarc]]

Attachment: pull-request (v2.0)

Diego: Please review. Note, I renamed our `camera:busy`/`camera:ready` events at the app level to simply be `busy`/`ready` because we now use the loading screen for more things than just the camera. Also, the `busy` event already takes an optional parameter to indicate the reason for the busy-ness (we have thresholds for showing the spinner in config.js based on this `type`).

David: Added you for feedback since I made a minor change to video_player.js. We already had an `onplaying` callback that was called after the video begins to play. So, I simply added an `onplay` callback that gets called as soon as video playback is requested for the first time (when the poster is still visible). I decided to take this route because it has minimal impact for breakage for other apps that depend on video_player.js. Also, we already had most of the mechanisms in-place within the Camera app to show our spinner, so I figured it would be less invasive to have video_player.js show its own spinner.

Comment 21

[David Flanagan [:djf]]

Comment on attachment 8458964 [details] [review]
pull-request (v2.0)

I'm giving feedback- because the new play event from video player is emitted every time the video starts playing, but is only different than the playing event for the first play, when the poster is hidden and the video shown.  If you're going to define a new event, I'd like it to be meaningful. So a 'loading' event emitted just on that first play would be fine.

But see my other comments on github as well: I think there might be other problems with this... Playing a video might re-enable the viewfinder while we're still in preview gallery, for example.

Comment 22

[David Flanagan [:djf]]

What a pain in the butt this bug is... 

Have you tried solving it without a spinner by just swapping the lines hidePoster() and showPlayer() so that we show the player first?  Maybe doing that and animating the play button until playback actually begins would be good enough.  Instead of hiding it the button, pulse it by dimming and brightening until you get the actual playing event from the video element and then hide it.

Comment 23

[David Flanagan [:djf]]

Comment on attachment 8458964 [details] [review]
pull-request (v2.0)

Thanks for addressing my concerns, Justin. Changing f- to f+.

Comment 24

[Diego Marcos [:dmarcos]]

Comment on attachment 8458964 [details] [review]
pull-request (v2.0)

Made a few suggestions on GH to rename things

Comment 25

[Justin D'Arcangelo [:justindarc]]

Landed on v2.0:

https://github.com/mozilla-b2g/gaia/commit/b9d19011123487009c80d1200937652d58c434a0

Comment 26

[Justin D'Arcangelo [:justindarc]]

Attachment: pull-request (master)

Pull request for master -- Carrying R+ forward

Comment 27

[Justin D'Arcangelo [:justindarc]]

Landed on master:

https://github.com/mozilla-b2g/gaia/commit/265bcc8bd54f9da013ff67e1f5826ccfb84df1de

Comment 28

[Yeojin Chung [:YeojinC]]

Found Test Case: https://moztrap.mozilla.org/manage/case/13753/

The step 1 through 5 covers the issue.

Comment 29

[KTucker [:KTucker][Inactive 3/4/2016]]

Test case has been updated to ensure there is not a significant delay when playing the video:

https://moztrap.mozilla.org/manage/case/13753/