STR: 1. Open Video app 2. Start video playback 3. Suspend device and then resume it back using power button Expected Result: Video frame should be shown as soon as device resumed Actual result: As soon as device resumed, video playback screen remains black for few sec before actually showing the video frame Gaia and Gecko info: Gaia "f5d4ff60ffed8961f7d0380ada9d0facfdfd56b1" Gecko "db7fce920e7d782d9f601384dc95924abcdaeeb8"

[Blocking Requested - why for this release]:

The video app discards the currently playing video when it goes to the background and restores it when it comes back to the foreground. This is because a long time ago we had to be very careful that only one app was trying to use the hardware decoder at a time. I think this workaround is no longer necessary, and we could probably resolve this bug by removing it. Sotaro: can you confirm that it is now safe for multiple apps to be using video elements at the same time?

(In reply to David Flanagan [:djf] from comment #2) > Sotaro: can you confirm that it is now safe for multiple apps to be using > video elements at the same time? General video elements' hw codec sharing mechanism is not implemented yet. Because there is no such requirement in b2g product requests. Instead simple hw video codec sharing mechanism is implemented since Bug 871485. And Bug 1029552 extends it a bit more recently. The following is how hw video codec is shared between video elements. - Current hw video codec user"video element" can continue to own until release it. - video element automatically release hw video codec when document is hidden. - video element automatically release hw video codec when the video element is detached from tree.

this is not reproducible in 2.0: Gaia-Rev c6c6116ca225c2c934220ae6867e5a3256d65e00 Gecko-Rev https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/24a2aa6bf1c4 Build-ID 20141015000206 Version 32.0 Device-Name flame FW-Release 4.4.2 FW-Incremental eng.cltbld.20141015.033627 FW-Date Wed Oct 15 03:36:38 EDT 2014 Bootloader L1TC00011840

Based on my understanding of comment 3, the video app no longer needs to discard the currently playing video when it goes to the background as the code will be automatically released. I am working on implementing this behavior.

(In reply to Russ Nicoletti [:russn] from comment #5) > Based on my understanding of comment 3, the video app no longer needs to > discard the currently playing video when it goes to the background as the > code will be automatically released. yes.

(In reply to Sotaro Ikeda [:sotaro] from comment #6) > (In reply to Russ Nicoletti [:russn] from comment #5) > > Based on my understanding of comment 3, the video app no longer needs to > > discard the currently playing video when it goes to the background as the > > code will be automatically released. > > yes. hw video codec is automatically released when the app got to background.

This blocks 2.1 FC and CC so 2.1+.

I've created a patch where the video app does not discard the current video. However, when the app comes to the foreground, the first frame of the video id displayed. There was already code in the video app that sets the video position to the position when the app went to the background (the "restore" position) and this results in the first frame of the video being shown briefly and then switching to the "restore" frame -- not a great user experience. I am hiding the video player as soon as the video app gets the 'visibilitychange' event and then displaying the player when the app gets the 'loadedmetadata' event.

I've resolved the problem of the first frame being shown briefly before the "restore" frame. I've done this by listening to the 'canplay' event and only then unhiding the player. This, however, raises some questions about what gecko is doing. Btw, I have removed all of the code in the video app that releases and loads the video on visibility changes. Yet on the visibility change that brings the app to the foreground, I'm seeing the following events being sent: - loadedmetadata - loadeddata - canplay Sotaro, is this expected? Is gecko essentially unloading and reloading the video when the video app is moved to the background and then the foreground?

(In reply to Russ Nicoletti [:russn] from comment #10) > I've resolved the problem of the first frame being shown briefly before the > "restore" frame. I've done this by listening to the 'canplay' event and only > then unhiding the player. This, however, raises some questions about what > gecko is doing. > > Btw, I have removed all of the code in the video app that releases and loads > the video on visibility changes. Yet on the visibility change that brings > the app to the foreground, I'm seeing the following events being sent: > - loadedmetadata > - loadeddata > - canplay > > Sotaro, is this expected? Is gecko essentially unloading and reloading the > video when the video app is moved to the background and then the foreground? It is expected. The current implementation does not filter the above events, because some file formats could emit multiple loadedmetadata events. When application goes to background, the application becomes inactive. If video element detects it, MediaDecoder becomes dormant state and releases a part of media resources(hw video codec) if releasing is necessary. Current implementation does not release data source, audio codec and Playback state machine. And when application becomes active again, the video element resumes from dormant state. And re-start from loadedmetadata again. During resuing, seeks to previous playback time.

(In reply to No-Jun Park [:njpark] from comment #4) > this is not reproducible in 2.0: > > Gaia-Rev c6c6116ca225c2c934220ae6867e5a3256d65e00 > Gecko-Rev > https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/24a2aa6bf1c4 > Build-ID 20141015000206 > Version 32.0 > Device-Name flame > FW-Release 4.4.2 > FW-Incremental eng.cltbld.20141015.033627 > FW-Date Wed Oct 15 03:36:38 EDT 2014 > Bootloader L1TC00011840 No-Jun, I don't see any difference in the behavior between 2.2 and 2.0. The video of my testing with 2.0 is here: http://youtu.be/mG0tYfXE0r8. It would be odd for the behavior to be different because the code that handles when the video goes to the background and then comes to the foreground is the same for 2.0 and 2.2. Also, I added log statements that log the time it takes to display a video when the app comes to the foreground after being in the background. For 2.2 and 2.0, I'm seeing similar results: it takes approximately 500ms. I don't see any difference between 2.0 and 2.2. Can you test again with 2.0 and confirm whether or not you see a delay?

a(In reply to Russ Nicoletti [:russn] from comment #12) > Also, I added log statements that log the time it takes to display a video > when the app comes to the foreground after being in the background. For 2.2 > and 2.0, I'm seeing similar results: it takes approximately 500ms. I don't > see any difference between 2.0 and 2.2. Can you test again with 2.0 and > confirm whether or not you see a delay? I just double checked 2.0 and 2.1 and you're right, the black screen behavior is same, and the duration of the black screen seems to be about the same as well. Sorry about the confusion.

Must confused with the media app, where such behavior is not seen in 2.1 either.

(In reply to Sotaro Ikeda [:sotaro] from comment #11) > When application goes to background, the application becomes inactive. If > video element detects it, MediaDecoder becomes dormant state and releases a > part of media resources(hw video codec) if releasing is necessary. Current > implementation does not release data source, audio codec and Playback state > machine. > > And when application becomes active again, the video element resumes from > dormant state. And re-start from loadedmetadata again. During resuing, seeks > to previous playback time. From your comment above, you seem to be saying the video should resume at the position it was at when the app went to the background, which makes sense to me in terms of user experience. However, I'm seeing that the video always resumes at the beginning. Can you comment on that? It seems like a bug to resume the video at the beginning.

Clarification of comment 15: What I'm seeing is the first frame is displayed briefly and then the video jumps to the frame it was at when the app went into the background. Video of my testing is here: http://youtu.be/smDOSU8bVz4. I'm finding that in order to hide the brief "flash" of the first frame, I have to hide the player until the 'canplay' event. That is not ideal because the result is a brief black screen, essentially the same user experience as what is described by this bug. Sotaro, can you comment on this behavior?

When I tried the following video's playback from browser, then I put the browser tab to background and put foreground again, the video was resume from the same place. Therefore, it seems to work as expected. http://people.mozilla.org/~cpeterson/videos/H264_Baseline_Profile_Level_30_640x360p.mp4

(In reply to Russ Nicoletti [:russn] from comment #16) > Clarification of comment 15: > > What I'm seeing is the first frame is displayed briefly and then the video > jumps to the frame it was at when the app went into the background. Video of > my testing is here: http://youtu.be/smDOSU8bVz4. > > I'm finding that in order to hide the brief "flash" of the first frame, I think showing the first frame is a bug of current gecko's implementation.

(In reply to Sotaro Ikeda [:sotaro] from comment #18) > (In reply to Russ Nicoletti [:russn] from comment #16) > > Clarification of comment 15: > > > > What I'm seeing is the first frame is displayed briefly and then the video > > jumps to the frame it was at when the app went into the background. Video of > > my testing is here: http://youtu.be/smDOSU8bVz4. > > > > I'm finding that in order to hide the brief "flash" of the first frame, > > I think showing the first frame is a bug of current gecko's implementation. Bug 1084629 is created for it.

This is the patch for the video app that lets gecko manage the video codec when the app is sent to the background and then back to the foreground.

Updating the patch to remove the fix for bug 1082555 (which was included in the previous patch). This patch removes the code from the video app that manages the hw video codec and instead lets gecko handle that.

Comment on attachment 8508021 [details] Github PR: https://github.com/mozilla-b2g/gaia/pull/25342 The attached PR removes the code from the video app that manages the hw video codec and lets gecko handle that. Note that bug 1084629 has been created against gecko to fix an issue with resuming the video where it left off when coming to the foreground after going to the background.

bug 1084629 becomes dup of bug 1058429.

Comment on attachment 8508021 [details] Github PR: https://github.com/mozilla-b2g/gaia/pull/25342 A couple of things noted on github that I'd like you to fix before landing this. What did you decide to do about the flash of the first frame? Is that just deferred to the gecko bug? Is the gecko bug going to block 2.1? If not, do we need to do anything in gaia for 2.1 to hide the first frame flash?

(In reply to David Flanagan [:djf] from comment #25) > Comment on attachment 8508021 [details] > Github PR: https://github.com/mozilla-b2g/gaia/pull/25342 > > A couple of things noted on github that I'd like you to fix before landing > this. > > What did you decide to do about the flash of the first frame? Is that just > deferred to the gecko bug? Is the gecko bug going to block 2.1? If not, do > we need to do anything in gaia for 2.1 to hide the first frame flash? Yes, according to Sotaro, the flash of the first frame is a gecko bug (bug 1084629, which depends on bug 1062134 -- both have been made 'blocking-b2g: 2.1?' today.

Comment on attachment 8508021 [details] Github PR: https://github.com/mozilla-b2g/gaia/pull/25342 Review comments have been addressed; PR has been updated.

Comment on attachment 8508021 [details] Github PR: https://github.com/mozilla-b2g/gaia/pull/25342 Looks good to me!

Master: https://github.com/mozilla-b2g/gaia/commit/4d7f051cede6544f4c83580253c743c22b0cb279

Please nominate this patch for Gaia v2.1 approval when you get a chance.

Comment on attachment 8508021 [details] Github PR: https://github.com/mozilla-b2g/gaia/pull/25342 [Approval Request Comment] [Bug caused by] (feature/regressing bug #): Legacy code in the video app whereby it manages the hw video codec. [User impact] if declined: Bad user experience. Without the fix, the user sees a black screen before the video when re-opening video after it has been in the background. [Testing completed]: Manual testing and unit testing [Risk to taking this patch] (and alternatives if risky): Low. Change is to one video app js file. The changes didn't affect the semantics of the code as the unit tests passed without changes. Bug 1062134 needs to be fixed as well otherwise when the video app resumes from being in the background, the first frame of the video will appear briefly before gecko seeks to the proper frame (the one showing before the app went into the background). [String changes made]: None

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

The issue still reproduces on 2.2 Flame As soon as device resumes, video playback screen remains black for a few sec before actually showing the video frame It's fixed on 2.1 as soon the device resumes, the video frame is shown immediately Still reproduces on: Device: Flame 2.2 Master KK BuildID: 20141104040207 Gaia: 3c50520982560ccba301474d1ac43706138fc851 Gecko: 54d05732f29b Gonk: 48835395daa6a49b281db62c50805bd6ca24077e Version: 36.0a1 (2.2 Master) Firmware: V188 User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0 Fixed on: Device: Flame 2.1 KK BuildID: 20141104001202 Gaia: 8b0cf889ae0d48a9eb7ecdcb9b67590de45cc5e5 Gecko: 388b03efe92d Gonk: 48835395daa6a49b281db62c50805bd6ca24077e Version: 34.0 (2.1) Firmware: V188 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Issue no longer reproducing on Flame 2.2. Marking as verified fixed. Actual Results: Issue as described is no longer occurring. When following STR from comment 0 on higher quality videos the user sees a black second for a fraction of a second. Resuming video then repeating process can exacerbate issue, but not beyond half a second for an HD video. Consulted with sarsenyev and neither of us are able to reproduce original issue on 2.2 any longer. Device: Flame 2.2 Master (319mb)(Kitkat Base)(Shallow Flash) BuildID: 20141112040208 Gaia: 5ae28ff11b982e2bd7d1aa097cda131536952bdc Gecko: 688f821edcd4 Version: 36.0a1 (2.2 Master) Firmware: V188-1 User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0