Closed Bug 974455 Opened 6 years ago Closed 6 years ago

[Camera] Setting specific pictureSize crashes app

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: wilsonpage, Assigned: mikeh)

Details

(Keywords: crash, Whiteboard: [b2g-crash])

Attachments

(1 file)

Steps to reproduce on Hamachi:

app.camera.mozCamera.pictureSize = { width: 1920, height: 1088 };

Other picture sizes seem not to crash the camera.
Also crashes Helix. This is dangerous as pictureSizes are persisted so this can cause the camera to continually crash on app launch.
The underlying cause is that some|most|all camera drivers require a thumbnail size that matches the aspect ratio of the picture size. We enforce this in Gecko, on the assumption that every supported picture size has a usable thumbnail size -- but not in this case.

There are two solutions:
1. I can filter out this resolution as unsupported;
2. I can set the thumbnail size to 0x0 (i.e. disabling it) when no suitable match is found.

Thoughts?
Assignee: nobody → mhabicher
Flags: needinfo?(wilsonpage)
Flags: needinfo?(dmarcos)
Flags: needinfo?(dflanagan)
Hardware: x86 → ARM
Mike: I'd go with the simplest solution to prevent the crash. Probably settting the thumbnail to 0x0.

Wilson: it sounds like you can work around this crash always setting thumbnail size to 0x0 before setting a new picture size, and then picking the best thumbnail size after that.

Once we have 854795 landed, the importance of EXIF thumbnails will be dramatically reduced.  But they'll still be nice to have.

This bug (plus bug 972142) makes me suggest again that we make all of this image size/preview size/thumbnail size stuff be something that is configured at build time, and customized for each device.

History says that each device is going to have weirdness.  What if we have to run on hardware where every picture resolution works except 1920x1088? Could we disable just that one size at build time?  I think we need a way to explicitly specify the full set of picture size modes, and each one should have a thumbnail size, a preview size, and a name to be used for localization in the settings.
Flags: needinfo?(dflanagan)
mikeh: Due to the strange aspect ratio, I think it would be best to just never expose this size to Gaia. I'd rather not have to write hacks to work around abnormal picture sizes on the client if possible. If there is no Thumbnail that matches the pictureSize aspect ratio, it may be best not to expose it.

What's easiest for you?
Flags: needinfo?(wilsonpage)
Wilson, now that bug 975472 has landed, do you still see this crash?

Setting the thumbnail size to 0x0 is definitely simpler, but it feels crude. I can filter out picture sizes with unsupported thumbnail sizes when setting up the camera.

The reason for the 1920x1088 format is for taking snapshots while actively recording 1080p video. JPEGs have to be sized in 16-pixel blocks, and 1080 / 16 = 67.5, so the image size is rounded up.
Status: NEW → ASSIGNED
Flags: needinfo?(dmarcos)
Small patch to make sure we don't break anything. After some thought, I've decided it's better to just disable the thumbnail than to disable the picture size.
Attachment #8381004 - Flags: review?(dhylands)
Attachment #8381004 - Flags: review?(dhylands) → review+
https://hg.mozilla.org/mozilla-central/rev/65755cc0ee10
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Latest testing,

Hamachi:

- No longer crashes
- Unable to extract preview from metadata. Meaning the filmstrip shows no thumbnail.

Helix:

- Doesn't crash, but preview stream stops flowing (turns black).
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to Wilson Page [:wilsonpage] from comment #10)
> 
> Hamachi:
> 
> - No longer crashes
> - Unable to extract preview from metadata. Meaning the filmstrip shows no
> thumbnail.

Please file a new Gaia bug for this; the Camera app will have to generate its own thumbnail in this case.

> Helix:
> 
> - Doesn't crash, but preview stream stops flowing (turns black).

I'll look into this and will (likely) file a new bug.
Keywords: crash
Whiteboard: [b2g-crash]
Mike or Wilson: Is this still an issue? If not, lets close it.

Thanks
Hema
Flags: needinfo?(wilsonpage)
Flags: needinfo?(mhabicher)
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Flags: needinfo?(wilsonpage)
Flags: needinfo?(mhabicher)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.