If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

RESOLVED FIXED in Firefox OS v2.0

Status

Firefox OS
Gaia::Camera
P1
blocker
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: hema, Assigned: dmarcos)

Tracking

({footprint, perf})

unspecified
2.0 S6 (18july)
ARM
Gonk (Firefox OS)
footprint, perf
Dependency tree / graph

Firefox Tracking Flags

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

Details

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

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
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.
(Reporter)

Updated

3 years ago
blocking-b2g: --- → 2.0+
Depends on: 1027592
No longer depends on: 1024692
(Assignee)

Updated

3 years ago
Assignee: nobody → dmarcos

Comment 1

3 years ago
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)
(Reporter)

Updated

3 years ago
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

Updated

3 years ago
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)
(Assignee)

Comment 4

3 years ago
Created attachment 8457668 [details] [review]
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+
(Assignee)

Comment 6

3 years ago
Landed on master:

https://github.com/mozilla-b2g/gaia/commit/58c00c37e65b21cf0c80c457beee7b69b46f7ce5
Flags: needinfo?(ryanvm)
(Assignee)

Updated

3 years ago
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Reporter)

Updated

3 years ago
status-b2g-v2.0: --- → affected
(Reporter)

Updated

3 years ago
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
status-b2g-v2.0: affected → fixed
status-b2g-v2.1: --- → fixed
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)

Comment 8

3 years ago
(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)

Updated

3 years ago
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.