1133327 - [Camera][Shinano][Aries] Enable use of full camera resolution

Status: RESOLVED FIXED

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

Description

[:gerard-majax]

Devices based on Shinano have a 21Mpx Camera, but the current Gaia code limits it. By pushing CONFIG_MAX_IMAGE_PIXEL_SIZE from 5Mpx to 32Mpx, we can make use of the full capabilities.

I have not been able to find any pref or settings that we could set to override this at build time. David, Wilson told me that this config value was here to help coping with memory issues on Gallery side. Do you have more infos?

Comment 1

[David Flanagan [:djf]]

There used to be build-time flags that would allow CONFIG_MAX_IMAGE_PIXEL_SIZE to be set at build time. They were used for 1.3T, for example. When the camera was refactored in 1.4, I think that kind of granular build time configurability was lost.

Before v1.4, decoding large images in Gallery would cause OOM, and we needed to be very careful not to open images that were too large for the device's memory. That is why we've got this variable. It is probably obsolete now because gecko can downsample images while decoding them. So increasing this config variable is probably safe in the sense that it will probably not cause the Gallery app to crash. The gallery is now able to open and display images up to at least 64mp in size, though it does still have a config variable that limits the maximum size it will display them at.  If you modify the config variable for camera, but leave gallery unchanged, the user will only be able to zoom in on the 20mp photos to somewhere around 5mp.

Also, I've heard from mikeh that this camera driver always produces absurdly small EXIF preview images. If so, then if the camera is taking 21mp photos, and the EXIF preview is too small to use, then Gallery will have to read the entire file (5mb, maybe?) in order to produce a thumbnail or show a screen-size preview of the photo.  Even if gecko is downsampling while decoding so that it does not use too much memory, it still has to process millions of bytes instead of tens of kilobytes. So because of the lack of good EXIF previews, I would expect to see a noticeable performance decrease in Gallery scanning and image display if you enable 20mp images.

Comment 2

[:gerard-majax]

Looking at the EXIF thumbnails on the same device running Android, I see that they are also very tiny, according to exiftool:
> Thumbnail Image                 : (Binary data 6764 bytes, use -b option to extract)

Comment 3

[Wilson Page [:wilsonpage]]

(In reply to David Flanagan [:djf] from comment #1)
> There used to be build-time flags that would allow
> CONFIG_MAX_IMAGE_PIXEL_SIZE to be set at build time. They were used for
> 1.3T, for example. When the camera was refactored in 1.4, I think that kind
> of granular build time configurability was lost.

The only reason we kept that global variable around was to ensure backwards-compatibility for this build-time configuration. If this is no longer required we should either remove this variable, or move it into the app's official config.js file.

Comment 4

[:gerard-majax]

For now, from what I could test taking quite a lot of 21Mpx pictures and then firing up a frehs Gallery app, it's actually quite fast and good.

Comment 5

[David Flanagan [:djf]]

Thanks. It is good to know that gallery's scanning performance is still fast. The other thing to check is the time it takes to display images when swiping as quickly as you can through 5 or more large photos. If you can ever see the image filling the screen as it decodes, that is sub-optimal, but it is also something that we've had to deal with as we start using large screen devices, so I guess I just won't worry about it!

Comment 6

[:gerard-majax]

(In reply to David Flanagan [:djf] from comment #5)
> Thanks. It is good to know that gallery's scanning performance is still
> fast. The other thing to check is the time it takes to display images when
> swiping as quickly as you can through 5 or more large photos. If you can
> ever see the image filling the screen as it decodes, that is sub-optimal,
> but it is also something that we've had to deal with as we start using large
> screen devices, so I guess I just won't worry about it!

I'm not sure exactly whether I see what you describe. While swiping in the gallery, there is indeed a small delay before the image is displayed ; it's just black before.

Comment 7

[:gerard-majax]

David, do you think you could get your hands on one of those devices and have a look ? From playing with it since a bit more than one week, I would say that we could switch 21Mpx without any fear, the device is still coping very well.

Comment 8

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

djf, FYI, Aries has 2GB of RAM, so it should be able to handle a fully-decompressed 21MP image.

Comment 9

[David Flanagan [:djf]]

(In reply to Alexandre LISSY :gerard-majax from comment #7)
> David, do you think you could get your hands on one of those devices and
> have a look ? From playing with it since a bit more than one week, I would
> say that we could switch 21Mpx without any fear, the device is still coping
> very well.

I'm unlikely to be able to get my hands on a device, and wouldn't know how to build gecko for it if I did. If the performance seems good, it is fine with me to go ahead use the full size images.

Comment 10

[:gerard-majax]

Per IRC discussion, we will just push the limit.

Comment 11

[GitHub Autolander Bot]

Attachment: [gaia] lissyx:bug1133327 > mozilla-b2g:master

Comment 12

[Wilson Page [:wilsonpage]]

Comment on attachment 8568470 [details] [review]
[gaia] lissyx:bug1133327 > mozilla-b2g:master

I'm happy with this if djf is :)

Comment 13

[:gerard-majax]

Updated PR for one Gb failure because I did not updated the value in tests.

Comment 14

[:gerard-majax]

green: https://treeherder.mozilla.org/#/jobs?repo=gaia-try&revision=8646308ea008

Comment 15

[David Flanagan [:djf]]

Comment on attachment 8568470 [details] [review]
[gaia] lissyx:bug1133327 > mozilla-b2g:master

Please just change the config setting for camera. If you also change it in Gallery, then gallery will start allowing 24mp PNG images, which will require 96mb to decode and will cause OOMs.   A 24mp solid color PNG image might only have a file size of a few kb, but would require nearly 100mb to display, so it is an easy DOS attack against someone to text them that image, for example.

Gallery already ignores those config variables for jpegs because it can downsample them while decoding. But the same is not true for other image types.

Comment 16

[:gerard-majax]

Comment on attachment 8568470 [details] [review]
[gaia] lissyx:bug1133327 > mozilla-b2g:master

Repushing without Gallery app changes.

Comment 17

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/9b2e65db5e291b5aecb31c0b252ff13683134cb5