Closed Bug 1037638 Opened 6 years ago Closed 6 years ago

Restrict to 3MP images (CONFIG_MAX_IMAGE_PIXEL_SIZE) only for low-memory devices

Categories

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

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:2.0+, b2g-v2.0 fixed, b2g-v2.1 fixed)

RESOLVED FIXED
2.0 S6 (18july)
blocking-b2g 2.0+
Tracking Status
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: hkoka, Assigned: dmarcos)

References

Details

(Keywords: memory-footprint, perf, Whiteboard: [c=memory p= s= u=2.0] [caf priority: p1][CR 697038][MemShrink:P2])

Attachments

(1 file)

46 bytes, text/x-github-pull-request
djf
: review+
Details | Review
Followup fix for addressing: https://bugzilla.mozilla.org/show_bug.cgi?id=1024692#c68

So the 3MP restriction is contained only for low-mem devices. 

+++ This bug was initially created as a clone of Bug #1024692 +++

STR:

Go to Camera->take a photo-> tap on shortcut to open the photo
try Zoom out/Zoom in and observe
Expected result:
device should be Zoom in and Zoom out
Actual result:
camera app is killed

we are seeing following thing in dmesg:

<6>[  151.338733] send sigkill to 1847 ((Preallocated a), adj 667, size 1215
<6>[  151.636678] send sigkill to 1615 (Homescreen), adj 534, size 1090
<6>[  151.782002] send sigkill to 1697 (Camera), adj 134, size 1445
<4>[  151.782204] Camera used greatest stack depth: 4444 bytes left


This indicated that we are running out of memory and LMK is killing apps in proper order as expected .


Device used : MSM8610 256MB.
Issue is present in both FFOS v1.3 and FFOS v1.4 running on same 256MB target. If we increase memory to 512MB then issue does not reproduce for same build.

This is observed with fix from 
[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1002593#c46
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=1014955#c39


Please let me know if you want to see DMD report or not.
blocking-b2g: --- → 2.0+
Depends on: 1027592
No longer depends on: 1024692
Assignee: nobody → dmarcos
For the MSM8610 device, what is the screen resolution you're using with it? The specs suggest that it's a FWVGA (840x480) device, but I wanted to confirm this. It should also be possible to set the resolution to something smaller. My understanding is that this device is imitating a 256 MB device that has an HVGA screen (480x320).

If this is being tested on something with 256 MB and a FWVGA resolution, then it's not surprising to me that it doesn't work. The extra screen size will add to the memory usage and could push us over the limit and cause problems.

As far as I'm aware, there are no planned consumer-oriented devices with both 256 MB RAM and a FWVGA screen. If there is such a device, let me know. If not, then I think it would be best to test this issue (and other memory-related issues) with a device that's closer to reality; namely, something with 256 MB RAM and an HVGA resolution.
Flags: needinfo?(ikumar)
Summary: camera app is killed during zoom in/zoom out (out of memory) on 256 MB target → Restrict to 3MP images (CONFIG_MAX_IMAGE_PIXEL_SIZE) only for low-memory devices
Whiteboard: [c=memory p= s= u=1.4] [caf priority: p1][CR 673864][MemShrink:P2] → [c=memory p= s= u=2.0] [caf priority: p1][CR 673864][MemShrink:P2]
See the patch attached to bug 1037097 (it needs your review). I think that once that patch lands for gallery, all you'll need to do is add the necessary permission to the camera manifest file, and we will automatically get the 3mp image decode restriction in the camera.  Then you can put the photo size back up to 5mp.
Note that if you take the approach described above, then CONFIG_MAX_IMAGE_PIXEL_SIZE may no longer be necessary at all.
Flags: needinfo?(dmarcos)
Attached file Pull Request
Attachment #8457668 - Flags: review?(dflanagan)
Flags: needinfo?(dmarcos)
Comment on attachment 8457668 [details] [review]
Pull Request

There is one nit on github, but the code looks good to me. Let's make sure between the two of us to test this carefully before landing, however.
Attachment #8457668 - Flags: review?(dflanagan) → review+
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [c=memory p= s= u=2.0] [caf priority: p1][CR 673864][MemShrink:P2] → [c=memory p= s= u=2.0] [caf priority: p1][CR 673864][MemShrink:P2] [landing eta 7/18]
v2.0: https://github.com/mozilla-b2g/gaia/commit/97ad2f62203ed322c2c69bd16321686d87dc513d
Flags: needinfo?(ryanvm)
Whiteboard: [c=memory p= s= u=2.0] [caf priority: p1][CR 673864][MemShrink:P2] [landing eta 7/18] → [c=memory p= s= u=2.0] [caf priority: p1][CR 673864][MemShrink:P2]
Target Milestone: --- → 2.0 S6 (18july)
(In reply to Jim Porter (:squib) from comment #1)
> For the MSM8610 device, what is the screen resolution you're using with it?
WVGA (800x480)
Flags: needinfo?(ikumar)
Whiteboard: [c=memory p= s= u=2.0] [caf priority: p1][CR 673864][MemShrink:P2] → [c=memory p= s= u=2.0] [caf priority: p1][CR 697038][MemShrink:P2]
You need to log in before you can comment on or make changes to this bug.