1108979 - [Flame][Gallery] Initial image of play view is displayed abnormally, when switching between two videos.

Status: VERIFIED FIXED

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

Description

[Sue]

Attachment: video

[1.Description]:
[Flame][V2.1][Gallery] Initial image of play view is displayed abnormally, when switching between two videos.
Occurence time: 15:42
see attachments:logcat_1542.txt, 1542.MP4

[2.Testing Steps]: 
Precondition: Record two videos
1. Launch Gallery
2. Tap a video
3. Tap another video
4. Tap previous video

[3.Expected Result]: 
Initial image of play view is displayed normally.

[4.Actual Result]: 
Initial image of play view is displayed as previous one.

[5.Reproduction build]: 
Flame 2.1 build:
Gaia-Rev        38e17b0219cbc50a4ad6f51101898f89e513a552
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-2g34_v2_1/rev/8b92c4b8f59a
Build-ID        20141205001201
Version         34.0

[6.Reproduction Frequency]: 
Always Recurrence,5/5
Free Test

Comment 1

[Sue]

Attachment: logcat

Comment 2

[Shine(Leave from Mozilla)]

Attachment: Flame2.1 logcat

Hi Mike,
This problem can be repro on latest Flame2.1.
Could you help with it, thanks!
See attachments: Flame2.1_logcat_0429.txt
Reproducing time: 04:29
Reproducing rate: 5/5

Flame 2.1 build
Gaia-Rev        73be51f998031f06db0cd660c0e388fa621c9f4c
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/05dd053f1d90
Build-ID        20150104001209
Version         34.0

Comment 3

[Mike Lien[:mlien]]

verify with v2.2 gaia/gecko, it also has this problem
Gaia-Rev        c2bf20d23851d5fda9f8f0ef0267db5f49152376
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/636498d041b5
Build-ID        20150104160203
Version         37.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150104.193130
FW-Date         Sun Jan  4 19:31:41 EST 2015
Bootloader      L1TC000118D0

Hi David, could help with this, thanks.
Refer to attached video, switch two video in Gallery app, the initial frame of clip will keep the previous video's first frame

Comment 4

[David Flanagan [:djf]]

It sounds like this is the same bug as 1102708, except in Gallery instead of Camera. Hopefully one change to MediaFrame will fix both bugs...

Comment 5

[David Flanagan [:djf]]

Attachment: link to patch on github

Russ: this fixes a bug where we get a flash of the previous video poster image before we see the correct one. The issue was that I was just hiding the poster with display:none, but not actually unloading the image. So when we go to load a new video, the <img> element is displayed and shows the old image while the new one is being loaded and decoded.

I'm not sure if you've reviewed patches to shared/js/media/video_player.js before, but it is code you'll need to get familiar with for your web component, so I think you're the right reviewer here.

Comment 6

[David Flanagan [:djf]]

Comment on attachment 8549926 [details] [review]
link to patch on github

Russ is on PTO today, so switching the review request to Punam

Comment 7

[Punam Dahiya [:pdahiya]]

Comment on attachment 8549926 [details] [review]
link to patch on github

Patch looks good and fixes subtle bug in gallery and camera preview.Thanks!

Comment 8

[David Flanagan [:djf]]

Landed on master: https://github.com/mozilla-b2g/gaia/commit/0f65b258bceddd9d479b3c027d9bd234c1e99aaf

Comment 9

[David Flanagan [:djf]]

Comment on attachment 8549926 [details] [review]
link to patch on github

This patch also fixes bug 1102708, which parters have identified as affecting releases all the way back to 2.0. I expect it will be nominated for uplift to earlier releases as well, but I'm only nom'ing it for 2.2 at this point.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): 

If this is a regression it has something to do with the way gecko displays images. The relevant gaia code has not changed.  Probably not a regression at all.

[User impact] if declined:

The user will see a brief flash of the wrong video in some cases.

[Testing completed]: 

yes, locally

[Risk to taking this patch] (and alternatives if risky):

It is a very simple patch that just moves one line of code so that the img element that displays the poster image for a video is cleared unconditionally. As a side effect of this fix, there might be a very slightly longer delay between the end of video playback in Camera and Gallery and the re-display of the poster image. I think that is a reasonable trade off for never displaying the wrong poster image.

[String changes made]:

none

Comment 10

[Sue]

Attachment: verify_v2.1.MP4

I have applied the patch on Flame v2.1, and this issue has been verified successfully on Flame v2.1
See attachment:verify_v2.1.MP4

Flame 2.1 build:
Gaia-Rev        77c57eb8a985d5cbd34a597fb1b978ba6e205af6
Gecko-Rev       
Build-ID        20150119225310
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.rose64.20150119.223518
FW-Date         Mon Jan 19 22:35:36 EST 2015
Bootloader      L1TC000118D0

Comment 11

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/b8ab790470f71c1752438414bc3c8734ce0475b2

Comment 12

[Sue]

Attachment: video.MP4

This issue has been verified successfully on Flame v2.2
See attachment: video.MP4

Flame 2.2 build:
Gaia-Rev        e4f9b5da3751798f9cc5d95f302c30722cc11fca
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/4a90da67661e
Build-ID        20150121162503
Version         37.0a2
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150121.201218
FW-Date         Wed Jan 21 20:12:28 EST 2015
Bootloader      L1TC000118D0

Comment 13

[Ryan VanderMeulen [:RyanVM]]

[Blocking Requested - why for this release]: Blocks a blocker.

Comment 14

[Sue]

Attachment: verify_v3.0.MP4

This issue has been verified successfully on Flame v3.0
See attachment:verify_v3.0.MP4
Rate:0/5

Flame 3.0 build:
Gaia-Rev        b02ec9713e6de8d96c6954d2c0dfd0442b0656ac
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/38e4719e71af
Build-ID        20150127010228
Version         38.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150127.043726
FW-Date         Tue Jan 27 04:37:39 EST 2015
Bootloader      L1TC000118D0

Comment 15

[Wesly Huang (EPM)]

[Triage] 

blocks a blocker which is 2.0+.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Please request Gaia v2.0 and v2.1 approval on this when you get a chance.

Comment 17

[David Flanagan [:djf]]

Comment on attachment 8549926 [details] [review]
link to patch on github

As requested, I'm asking to uplift this to 2.0 and 2.1. See comment 9 for the approval request details. It is a very simple patch that has been on master and 2.2 for some time now, so should be safe in earlier releases as well.

Bhavana: I don't know if you're the right one to handle these requests. Sorry if I've got the wrong person.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

v2.1: https://github.com/mozilla-b2g/gaia/commit/a7ae581a9e5cdcdfae08c92f63aa571fc024bdde
v2.0: https://github.com/mozilla-b2g/gaia/commit/21964b1e74bc1d69339d5c571f2d6bd8acd015fc

Comment 19

[Sue]

Attachment: verify_video.MP4

This issue has been verified successfully on Flame 2.0/2.1
STR:
Precondition: Record two videos.
1. Launch Gallery app.
2. Select a video to view.
3. Tap back button.
4. Select another video.
5. Back and select the previous video.
**Initial image of play view is displayed normally.
See attachment:verify_video.MP4
Rate:0/5

Flame 2.0 build:
Build ID               20150225000239
Gaia Revision          366aaa19ac474dc58b79d62a91cff41756ae9dfe
Gaia Date              2015-02-22 20:25:01
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/611444d72a92
Gecko Version          32.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150225.035740
Firmware Date          Wed Feb 25 03:57:51 EST 2015
Bootloader             L1TC000118D0

Flame 2.1 build:
Build ID               20150225001618
Gaia Revision          86af0ca427adad12c3109124f31bef2fd9614e47
Gaia Date              2015-02-24 02:22:26
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/a275f2c05ca6
Gecko Version          34.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150225.034947
Firmware Date          Wed Feb 25 03:49:58 EST 2015
Bootloader             L1TC000118D0

Comment 20

[Shine(Leave from Mozilla)]

Attachment: video

This problem is verified pass on latest build of 2.1S(256mb)
See attachments: Verify_2.1sVideo.MP4
Rate: 0/5

2.1S build (256mb):
Build ID               20150226161204
Gaia Revision          a43d64ae01ef108aa4dcc971c770fecd8416a764
Gaia Date              2015-02-26 09:24:39
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/2437280c634f
Gecko Version          34.0
Device Name            scx15_sp7715ga
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150226.193307
Firmware Date          Thu Feb 26 19:33:19 EST 2015

Comment 21

[Sue]

Attachment: video.MP4

This issue has been verified successfully on Woodduck 2.0M
STR:
Precondition: Record two videos.
1. Launch Gallery app.
2. Select a video to view.
3. Tap back button.
4. Select another video.
5. Back and select the previous video.
**Initial image of play view is displayed normally.
See attachment:video.MP4
Rate:0/5

Woodduck build:
Build ID               20150310050313
Gaia Revision          0e80bc1c501b1a7733841d724d45530c2897b757
Gaia Date              2015-03-09 06:50:46
Gecko Revision         c77dc549894cf6956ca49cfb22228d5e4d2f9a2f
Gecko Version          32.0
Device Name            jrdhz72_w_ff
Firmware(Release)      4.4.2
Firmware(Incremental)  1425935105
Firmware Date          Tue Mar 10 05:05:25 CST 2015