Closed Bug 1133327 Opened 10 years ago Closed 10 years ago

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

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
2.2 S7 (6mar)

People

(Reporter: gerard-majax, Assigned: gerard-majax)

References

Details

(Whiteboard: [systemsfe])

Attachments

(1 file)

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?
Flags: needinfo?(dflanagan)
Summary: [Shinano][Aries] Enable use of full camera resolution → [Camera][Shinano][Aries] Enable use of full camera resolution
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.
Flags: needinfo?(dflanagan) → needinfo?(lissyx+mozillians)
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)
Flags: needinfo?(lissyx+mozillians)
(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.
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.
Flags: needinfo?(dflanagan)
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!
Flags: needinfo?(dflanagan)
(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.
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.
Flags: needinfo?(dflanagan)
djf, FYI, Aries has 2GB of RAM, so it should be able to handle a fully-decompressed 21MP image.
(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.
Flags: needinfo?(dflanagan)
Per IRC discussion, we will just push the limit.
Assignee: nobody → lissyx+mozillians
Attachment #8568470 - Flags: review?(wilsonpage)
Attachment #8568470 - Flags: review?(dflanagan)
Comment on attachment 8568470 [details] [gaia] lissyx:bug1133327 > mozilla-b2g:master I'm happy with this if djf is :)
Attachment #8568470 - Flags: review?(wilsonpage) → review+
Updated PR for one Gb failure because I did not updated the value in tests.
Comment on attachment 8568470 [details] [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.
Attachment #8568470 - Flags: review?(dflanagan) → review-
Comment on attachment 8568470 [details] [gaia] lissyx:bug1133327 > mozilla-b2g:master Repushing without Gallery app changes.
Attachment #8568470 - Flags: review-
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [systemsfe]
Target Milestone: --- → 2.2 S7 (6mar)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: