974455 - [Camera] Setting specific pictureSize crashes app

Status: RESOLVED FIXED

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

Description

[Wilson Page [:wilsonpage]]

Steps to reproduce on Hamachi:

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

Other picture sizes seem not to crash the camera.

Comment 1

[Wilson Page [:wilsonpage]]

logcat: https://pastebin.mozilla.org/4338701

Comment 2

[Wilson Page [:wilsonpage]]

Also crashes Helix. This is dangerous as pictureSizes are persisted so this can cause the camera to continually crash on app launch.

Comment 3

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

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?

Comment 4

[David Flanagan [:djf]]

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.

Comment 5

[Wilson Page [:wilsonpage]]

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?

Comment 6

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

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.

Comment 7

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

Attachment: If we can't find a matching thumbnail size, disable the thumbnail, v1

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.

Comment 8

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

https://hg.mozilla.org/integration/b2g-inbound/rev/65755cc0ee10

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/65755cc0ee10

Comment 10

[Wilson Page [:wilsonpage]]

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).

Comment 11

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

(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.

Comment 12

[Hema Koka [:hema]]

Mike or Wilson: Is this still an issue? If not, lets close it.

Thanks
Hema