1217989 - [Flame][Video} Gallery app reports video resolutions based on poster image, not the video file

Status: RESOLVED FIXED

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

Description

[Michael Bryant (uber)]

Videos recorded on the Aries device (at 1280x720) are not able to be played when transferred to a Flame device. This bug is similar to Bug 1177250 but since the resolution output of Aries videos is different it was requested that I open a separate bug for this case.

STR

1) Record video on Aries device
2) Copy to PC/Mac
3) Load onto Flame device either via SD card or on device
4) Video does not show up in the Video application

Flame Device Information:
Build ID               20151023030241
Gaia Revision          410e91ddabc7ba82a9b43b3711a3fdf2cb8de309
Gaia Date              2015-10-23 05:57:04
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/0625c68c0abcfe4d10880d15d8fe7d06df3369c9
Gecko Version          44.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  65
Firmware Date          Mon Dec 15 18:51:29 CST 2014
Bootloader             L1TC000118D0

Aries Device Information:
Build ID               20151023104059
Gaia Revision          410e91ddabc7ba82a9b43b3711a3fdf2cb8de309
Gaia Date              2015-10-23 05:57:04
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/0625c68c0abcfe4d10880d15d8fe7d06df3369c9
Gecko Version          44.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20150619.224059
Firmware Date          Fri Jun 19 22:41:08 UTC 2015
Bootloader             s1

Comment 1

[No-Jun Park [:njpark]]

[Blocking Requested - why for this release]:
functional regression, same reason for Bug 1177250.

Comment 2

[Hema Koka [:hema]]

Adding punam for investigation.

Comment 3

[Michael Bryant (uber)]

Edit for the first line:

"Videos recorded on the Aries device (at 1280x720) are not able to be played when transferred to a Flame device."

Videos are still being recorded at 2160x3840 but the gallery is reporting them to be 1280x720.

Comment 4

[No-Jun Park [:njpark]]

[Blocking Requested - why for this release]:

So it turns out that Gallery app was incorrectly reporting the resolution, because the resolution was obtained from the poster image, and not the video file itself.  Video app however, correctly reports the resolution.  Changing the bug title, and re-noming it to 2.5 to reevaluation.

Comment 5

[Punam Dahiya [:pdahiya]]

For getting resolution of video file we need to load video element offscreen and get its width and height, that is an extra time taking step in metadataparser.  We are displaying video resolution when we view video files in Video app.

Discussed with djf and for 2.5 unless we have a compelling use case, we should show resolution only for image files in gallery app and create a new bug to update info screen with resolution info for video files.

I will attach patch with the fix. Thanks!

Comment 6

[GitHub Autolander Bot]

Attachment: [gaia] punamdahiya:Bug1217989 > mozilla-b2g:master

Comment 7

[Punam Dahiya [:pdahiya]]

Comment on attachment 8680746 [details] [review]
[gaia] punamdahiya:Bug1217989 > mozilla-b2g:master

Hi David

Attaching patch that shows resolution field in info view only for image files. Please review. Thanks!

Comment 8

[Punam Dahiya [:pdahiya]]

Created Bug 1219884 to fix resolution info for video files and display it in info view. Thanks!

Comment 9

[David Flanagan [:djf]]

Comment on attachment 8680746 [details] [review]
[gaia] punamdahiya:Bug1217989 > mozilla-b2g:master

Thanks for doing this, Punam. r+ if you address the issues on github. (I'm happy to review again if you'd like, but it is not required.)

This was harder than I expected. I'd forgotten that there was just one static info dialog and not something we were creating dynamically each time. It would have been easier if you could just omit the HTML for the elements we didn't want to display...

My primary concern is with the CSS being too generic and requiring those !important declarations. Making the classes very specific should help.

Comment 10

[Punam Dahiya [:pdahiya]]

Thanks David for review. I have updated patch with your feedback and it looks good. Carrying over the r+ and will land the patch once tests passes on treeherder. Thanks!

Comment 11

[Punam Dahiya [:pdahiya]]

Patch landed on master

https://github.com/mozilla-b2g/gaia/commit/788f240f4ec8c2a6a79c87edc711e6e6d990bc12