1207792 - [Gallery] If you navigate away from the Gallery application while watching a video in portrait mode when you come back the paused image will be zoomed in and the video has to resize upon initiating playback

Status: VERIFIED FIXED

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

Description

[Michael Bryant (uber)]

If you're watching a video in portrait mode, hit the home button and then select the Gallery app again the video will be paused but the screen will show a zoomed in portion of the video instead of the whole thing. Once you start playback the video will properly resize itself and continue playing.

This issue can be seen in this video: https://youtu.be/5xpdkZIMU2M

Phone build:

Device: Flame
Build ID               20150922030227
Gaia Revision          29991414eb94b6baa1ec2e63fdb4f6dfae05fb01
Gaia Date              2015-09-21 09:27:10
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/2235e56c94cf61614902fd3a4ac7b837f7154b97
Gecko Version          44.0a1
Firmware(Release)      4.4.2
Firmware(Incremental)  65
Firmware Date          Mon Dec 15 18:51:29 CST 2014
Bootloader             L1TC000118D0

Comment 1

[No-Jun Park [:njpark]]

I think this is a regression as well. qawanted for branch check.

Comment 2

[Shally Li（leave from Mozilla）]

This bug can be repro on the latest build of Flame KK 2.5 and Aires KK 2.5 by the STR in comment 0, but can't be epro on latest Flame KK 2.2.

Actual results: The video screen shows a zoomed in portion of the video instead of the whole thing in Gallery app.
See attachment: Aries_v2.5.3gp and logcat_1136.txt
Reproduce rate: 10/10


Device: Flame KK 2.2 (Unaffected) 
Build ID               20150923032502
Gaia Revision          5dd95cfb9f1d6501ce0e34414596ef3dd9c2f583
Gaia Date              2015-09-21 11:20:23
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/65ddad73ad6b
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150923.070045
Firmware Date          Wed Sep 23 07:00:57 EDT 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0

Device: Flame KK 2.5 (Affected)
Build ID               20150923150203
Gaia Revision          8472f0c736660072799aaae60e4b6001a6aaceb4
Gaia Date              2015-09-23 10:29:02
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/4a46de29431baa621d98d8f1168e43297ce1a916
Gecko Version          44.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150923.184655
Firmware Date          Wed Sep 23 18:47:17 EDT 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0

Device: Aries KK 2.5 (Affected)
Build ID               20150923233246
Gaia Revision          8472f0c736660072799aaae60e4b6001a6aaceb4
Gaia Date              2015-09-23 10:29:02
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/f1dffc8682fbba463cb4bb305f293ddcccbc20b4
Gecko Version          44.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20150923.225204
Firmware Date          Wed Sep 23 22:52:12 UTC 2015
Bootloader             s1

Comment 3

[Shally Li（leave from Mozilla）]

Attachment: logcat_1136.txt

Comment 4

[Shally Li（leave from Mozilla）]

Attachment: Aries_v2.5.3gp

Comment 5

[No-Jun Park [:njpark]]

[Blocking Requested - why for this release]:
since it's a regression that shows obvious visual anomaly, nominating as a blocker.

Comment 6

[David Flanagan [:djf]]

The video shows that this one is intermittent.

It is also closing and re-opening the video app quickly. There could be a timing issues related to when the visibility change event is sent, I suppose.

Sounds like it might be related to bug 1207788.

The video also displays the symptoms of bug 1205096. That bug was just fixed, and I wonder if the patch makes this one go away, too.

I'm investigating a couple of blockers, so not taking this one just yet.

Comment 7

[David Flanagan [:djf]]

Also: it would be interesting to know if this is a regression caused by the gaia patch from bug 1164200. (That's the same patch that caused bug 1205096)

No-Jun or Shally: if either of you has time, could you try backing out https://github.com/mozilla-b2g/gaia/commit/0cc109495bf06060e66f993255ab8a23fcfe347b and trying to reproduce the bug again?

Comment 8

[David Flanagan [:djf]]

I forgot to set needinfo above, but the commit can't be reverted cleanly, anyway.

I was hoping that this would be fixed by 1205096, but it has not been. I am guessing, though, that this is a regression from 1164200.  I suspect the onloadedmetadata handler has code that is only intended to run when a new video is first loaded, but it is running whenever we return to the gallery app with a video displayed, and that is causing the size of the video to change incorrectly. (Needinfo Andrew in case he has input on this)

I'll go ahead and take this now.

Now that bug 1207788 is fixed, this bug reproduces when watching a video in landscape mode as well as portrait mode.  I think we should block on it, if anyone is triaging. But I also think the fix will be straightforward.

Comment 9

[Andrew Osmond [:aosmond] (he/him)]

The problem is that the poster is replaced with the last captured frame of the video when the app is hidden. We need to resize the poster appropriately depending on the source. Sigh.

Comment 10

[GitHub Autolander Bot]

Attachment: [gaia] davidflanagan:bug1207792 > mozilla-b2g:master

Comment 11

[David Flanagan [:djf]]

Comment on attachment 8668715 [details] [review]
[gaia] davidflanagan:bug1207792 > mozilla-b2g:master

Andrew,

As part of fixing this bug I did a cleanup instead of going for a minimal patch.

I realized that video player doesn't actually have to care about the size of the video or of any poster image. All it needs to know is the aspect ratio and rotation.  Since we always play movies at the largest possible size, we can compute that size based on the screen size and the aspect ratio, and we can have the img and video element do the required scaling.

The current code uses a CSS transform to scale the posters and videos to fit the screen. To compute that scale we need to know the size of the source media.  But instead if we don't use a CSS transform to scale, we can just set the size of the the img and video and we're done.

It is a largish patch, but it removes more code than it adds, and I think it is a good simplification. (It also removes some unrelated unused code).  This is a big enough change that I wouldn't want to land it after FC, but I feel like it is okay at this point.

Comment 12

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 8668715 [details] [review]
[gaia] davidflanagan:bug1207792 > mozilla-b2g:master

Looks good to me. Minor nit on the playback size calculation but r+ either way.

Comment 13

[David Flanagan [:djf]]

landed on master: https://github.com/mozilla-b2g/gaia/commit/3537a4e0218f41255c6f323c492ba4d702e0e765

Comment 14

[David Flanagan [:djf]]

No-Jun: I'm flagging you so you know about this bug. I'm confident that this bug has been fixed. My patch was not a completely simple one, however, and I just wanted to ask you to keep your eye out for anything that looks broken with video playback in the gallery app and the camera app.  (Video playback in Gallery, video previews in Camera, and video confirmation previews when you do a pick activity, select Camera and then record a video.)

There are minor visual changes in what happens to a playing video when the user rotates the phone, but this seems to only affect Flame, not Aries. Videos are now scaled differently, and it seems possible that we could be hitting a different code path in gecko. Frame rate seems unaffected, so I don't think performance has regressed there. I don't know if there are any differences in power consumption with and without this patch, however.

Note that the Video app itself is completely unaffected here, and I think this patch brings Gallery and Camera slightly closer to the way the Video app handles video scaling.

Comment 15

[No-Jun Park [:njpark]]

Thanks David, I'll keep an eye on it on my weekly graphics sanity test.  This bug in particular seems to be gone in both Aries and Flame.

Comment 16

[Jessica]

Attachment: verified_Aries KK v2.5.3gp

According to the STR of Comment 0, this bug has been verified as pass on latest Flame KK v2.5 and Aries KK v2.5.

Actual results: The video screen shows the whole thing in Gallery app.
See attachment: verified_Aries KK v2.5.3gp
Reproduce rate: 0/6


Device: Flame KK v2.5 (Pass)
Build ID               20151013150203
Gaia Revision          d400cda6bf0f8b30dcf7d7d71bfa61f29a3f1588
Gaia Date              2015-10-13 06:42:17
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/2387ada864282880d3a498d643abe3d8b887ee59
Gecko Version          44.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20151013.182615
Firmware Date          Tue Oct 13 18:26:27 EDT 2015
Fireware Version       v18D v4
Bootloader             L1TC000118D0

Device: Aries KK v2.5 (Pass)
Build ID               20151013181426
Gaia Revision          d400cda6bf0f8b30dcf7d7d71bfa61f29a3f1588
Gaia Date              2015-10-13 06:42:17
Gecko Revision         https://hg.mozilla.org/integration/mozilla-inbound/rev/2387ada864282880d3a498d643abe3d8b887ee59
Gecko Version          44.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20151013.174418
Firmware Date          Tue Oct 13 17:44:27 UTC 2015
Bootloader             s1