844920 - [meta] Camera - expose runtime-detected platform-specific picture settings

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, enhancement)

Description

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

Bug 804359 tracks video-recording profiles; this one tracks picture settings.

(Note that in some cases, the values returned by the underlying AOSP camera API may not be usable due to resource constraints.)

Comment 1

[Inder]

We should do it for 1.3?

Comment 2

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

I think 1.3 is wrapping up, so whatever is involved in this issue isn't going to make it into 1.3.

Comment 3

[Jason Smith [:jsmith]]

Meta bugs won't block, so clearing nom.

Comment 4

[Wilson Page [:wilsonpage]]

It has been raised several times during the foxfooding program that users would like to be able to change picture/video resolution.

With the Z3C support 4k video this may now be sensible as users may want to sometimes record 4k, but this is not really a sensible default.

Does anyone object to exposing picture/video resolution in the Camera settings menu by default?

Comment 5

[Wilson Page [:wilsonpage]]

Some work may need to be done to the settings logic if we want the default resultion to be different to the maximum available.

I also noticed we have '1080p' in our `exclude` list. IIRC this was something to do with issues on Nexus4. How can we address profiles working on some devices and not others? Should this be a Gecko thing as I believe we already have device specific configuration on that side?

Comment 6

[Diego Marcos [:dmarcos]]

Exposing all resolutions is going to confusing. How about showing just two options: normal / high quality?

Comment 7

[:gerard-majax]

(In reply to Diego Marcos [:dmarcos] from comment #6)
> Exposing all resolutions is going to confusing. How about showing just two
> options: normal / high quality?

Why would it be confusing? The number of resolutions supported on the Z3c is not that big, considering we already filter 16/9 or 4/3 (we could expose both ...). It's just six resolutions. Is it already too much?

Comment 8

[Johan Lorenzo [:jlorenzo]]

From a QA perspective, recording bug videos with an Aries on 4K is problematic. Most of my 10-second-video are now 100-150MB. I would love to have the choice of resolution.

Comment 9

[Johan Lorenzo [:jlorenzo]]

Added foxfood, based on comment 5.

Comment 10

[No-Jun Park [:njpark]]

From what I remember, there was also an issue with Buri device with 1080p and 720p, so I had to pref them out in order to test camera.  But it would've been better if the user could select the resolution (and grey out ones that are unavailable to the device).  We can still expose all resolutions but assign a default resolution per device, then it probably won't make too much difference for the user?

I think ux input might be beneficial here: ni?ing tif fyi.

Comment 11

[:gerard-majax]

I think the proposal in comment 10 would be a good solution.

Comment 12

[Andrew Osmond [:aosmond] (he/him)]

(In reply to Wilson Page [:wilsonpage] (PTO until 15th) from comment #5)
> Some work may need to be done to the settings logic if we want the default
> resultion to be different to the maximum available.
> 
> I also noticed we have '1080p' in our `exclude` list. IIRC this was
> something to do with issues on Nexus4. How can we address profiles working
> on some devices and not others? Should this be a Gecko thing as I believe we
> already have device specific configuration on that side?

1080p recording in general should be fine. If it doesn't work on certain devices, then I agree, this should be filtered at the Gecko/Gonk layer, not at the application layer; that way third party apps don't try to use broken profiles. We don't have anything implemented for this at the moment, but I can easily add this if there is a need via system properties.

As for the camera settings, Gecko exposes a "default" profile which just maps to the highest supported profile. There is no reason why this has to be the highest though -- some cameras exposed a preferred recording resolution (e.g. on the Z3C, this is 1080p) which may be preferable. Why not just always use "default" profile as your default value, assuming I change Gecko to give something sane?

Comment 13

[Tiffanie Shakespeare [:tif]]

Adding Amy - she might be able to get to this before me.

Comment 14

[Amy Lee [:amylee] UX]

If the option is available to provide multiple resolution options on Z3, I think we should expose it in the options menu. It should default to the optimal resolution for the device and not the highest setting IMO. Tiff to confirm.

Comment 15

[Tiffanie Shakespeare [:tif]]

Totally agree. The UI for this has already been done last year for a partner so there shouldn't be anything you would need from our side...?

Comment 16

[Tiffanie Shakespeare [:tif]]

I should add (reading over the comments) that it would be awesome to list both the resolution and have a more user-friendly term like "high", "medium" as justindarc suggested in comment 6. This, I believe (it's so long ago), is a departure from the UI that we had done.

Comment 17

[:gerard-majax]

FYI, if we remove 16/9 exclusion we can get the following list for pictures on Z3/Z3 Compact:
 - 21Mpx (4/3)
 - 15Mpx (16/9)
 - 8Mpx (4/3 and 16/9)
 - 3Mpx (4/3)
 - 2Mpx (16/9)
 - 1Mpx (16/9)
 - 640x480 (4/3)
 - 480x320 (3/2)
 - 320x240 (4/3)

Comment 18

[:gerard-majax]

Attachment: Expose all Shinano Camera pictures modes

Comment 20

[David Flanagan [:djf]]

(In reply to Wilson Page [:wilsonpage] from comment #4)

> Does anyone object to exposing picture/video resolution in the Camera
> settings menu by default?

No objection from me. It is a UX issues as far as I'm concerned, and they've already given the go-ahead.

Alexandre: do you want to take the bug and ask Wilson to review your patch?

Comment 21

[Wilson Page [:wilsonpage]]

Comment on attachment 8639005 [details] [diff] [review]
Expose all Shinano Camera pictures modes

Review of attachment 8639005 [details] [diff] [review]:
-----------------------------------------------------------------

- If we choose to expose aspects other than 4:3 I'd like to see us move to aspect-fit style viewfinder so that the aspect is clear to the user at the time of taking the picture/video, and is not a shock afterwards.

- 'scene' setting seems to have been exposed in this patch which is unrelated to this particular bug.

Comment 22

[Wilson Page [:wilsonpage]]

So for this patch I suggest we expose the picture and video settings menus, but only show 4:3 aspects. We can expose other non-standard aspects as part of a separate bug.

Comment 23

[:gerard-majax]

(In reply to David Flanagan [:djf] from comment #20)
> (In reply to Wilson Page [:wilsonpage] from comment #4)
> 
> > Does anyone object to exposing picture/video resolution in the Camera
> > settings menu by default?
> 
> No objection from me. It is a UX issues as far as I'm concerned, and they've
> already given the go-ahead.
> 
> Alexandre: do you want to take the bug and ask Wilson to review your patch?

No, I gave my point of view and my local patches on this, but I don't want to get into the UX battle.

Comment 24

[:gerard-majax]

Attachment: Expose all Shinano Camera pictures modes

Moving scene enabling in the other patch

Comment 25

[Wilson Page [:wilsonpage]]

Comment on attachment 8639872 [details] [diff] [review]
Expose all Shinano Camera pictures modes

Review of attachment 8639872 [details] [diff] [review]:
-----------------------------------------------------------------

Required for this patch to land:

1. Dedupe picture/video profiles of the same resolution (eg. '480p' &'default').
2. Hide/grey-out picture/video resolution from the Settings menu when in an activity that has defined a target resolution/file-size.
3. Fix the selection confirmation notification text, currenty: 'Camera/Video Resolution undefined'.
4. Show only 4:3 aspect ratios until we have aspect-fit viewfinder that shows the bounds of the viewfinder frame.

Comment 26

[No-Jun Park [:njpark]]

This may sound obvious, but tif pointed out that if we are adding a user selection for video resolution, we should make sure that all videos recorded with any selectable resolution is playable on the same device.  I.e., we shouldn't enable 1080p on Flame 319MB configuration.  ni?ing tif in case I missed something.

Comment 27

[Tiffanie Shakespeare [:tif]]

Yes!! Thank you for finding this bug and commenting :)

I can't emphasize this point enough. Having the device choke in playback on a video taken with the camera is going to be a horrible (and extremely frustrating) experience for people.

If the Flame, for example, is not going to be able to play the 1080p, the option should not be in the settings menu.

Thanks guys!

Comment 28

[Wilson Page [:wilsonpage]]

(In reply to Tiffanie Shakespeare from comment #27)
> Yes!! Thank you for finding this bug and commenting :)
> 
> I can't emphasize this point enough. Having the device choke in playback on
> a video taken with the camera is going to be a horrible (and extremely
> frustrating) experience for people.
> 
> If the Flame, for example, is not going to be able to play the 1080p, the
> option should not be in the settings menu.
> 
> Thanks guys!

We don't yet have the infrastructure in place on the clientside to exclude profiles for specific device types. We can either exclude 1080p for all devices, or include it for all. We can either:

A. Invest some time creating per device video profile configuration
B. Exclude specific profiles on Gecko side so Gaia never sees them

Comment 29

[Szmozsánszky István [:flaki]]

(In reply to Wilson Page [:wilsonpage] from comment #28)
> (In reply to Tiffanie Shakespeare from comment #27)
> > If the Flame, for example, is not going to be able to play the 1080p, the
> > option should not be in the settings menu.
> > 
> > Thanks guys!
> 
> We don't yet have the infrastructure in place on the clientside to exclude
> profiles for specific device types. We can either exclude 1080p for all
> devices, or include it for all. We can either:
> 
> A. Invest some time creating per device video profile configuration
> B. Exclude specific profiles on Gecko side so Gaia never sees them

I think it would be a nice compromise to only expose 720p for the time being (that is, B.) on all devices (that including hiding 1080p on the Aries devices), as it's still provides a nice quality/size ratio, even on the Aries, and work on providing facilities for A.) on the longer term.

As a C.) Sony's original Android firmware provides "camera apps", that are kind of what addons on Firefox OS - I could imagine an addon existing for unlocking 1080p (maybe even 4K) recording as a separately installable addon for those power users who would like to experiment with that functionality.

(FWIW 4K video recording option is only accessible via the pre-installed addon/camera app in the original Android firmware too)

Comment 30

[:gerard-majax]

(In reply to Szmozsánszky István [:flaki] from comment #29)
> (In reply to Wilson Page [:wilsonpage] from comment #28)
> > (In reply to Tiffanie Shakespeare from comment #27)
> > > If the Flame, for example, is not going to be able to play the 1080p, the
> > > option should not be in the settings menu.
> > > 
> > > Thanks guys!
> > 
> > We don't yet have the infrastructure in place on the clientside to exclude
> > profiles for specific device types. We can either exclude 1080p for all
> > devices, or include it for all. We can either:
> > 
> > A. Invest some time creating per device video profile configuration
> > B. Exclude specific profiles on Gecko side so Gaia never sees them
> 
> I think it would be a nice compromise to only expose 720p for the time being
> (that is, B.) on all devices (that including hiding 1080p on the Aries
> devices), as it's still provides a nice quality/size ratio, even on the
> Aries, and work on providing facilities for A.) on the longer term.

I disagree, because I see how this will go: we will cut the capabilities and people will get busy on other stuff and this will not be prioritized. Not to mention the value of the argument of limiting a device to what other (much) lower end devices can do. This is not like this that we will create a great user experience especially for dogfooders and community.

Comment 31

[Wilson Page [:wilsonpage]]

(In reply to No-Jun Park [:njpark] from comment #26)
> This may sound obvious, but tif pointed out that if we are adding a user
> selection for video resolution, we should make sure that all videos recorded
> with any selectable resolution is playable on the same device.  I.e., we
> shouldn't enable 1080p on Flame 319MB configuration.  ni?ing tif in case I
> missed something.

We only enable the profiles that Gecko exposes to us via mozCamera.capabilities.recorderProfiles. This is the Flame:

 { 480p: CameraRecorderProfile, qvga: CameraRecorderProfile, vga: CameraRecorderProfile, cif: CameraRecorderProfile, qcif: CameraRecorderProfile, default: CameraRecorderProfile, low: CameraRecorderProfile }

As you can see, 1080p is not an available option. 480p is the max.

Comment 32

[No-Jun Park [:njpark]]

(In reply to Wilson Page [:wilsonpage] from comment #31)
> 
>  { 480p: CameraRecorderProfile, qvga: CameraRecorderProfile, vga:
> CameraRecorderProfile, cif: CameraRecorderProfile, qcif:
> CameraRecorderProfile, default: CameraRecorderProfile, low:
> CameraRecorderProfile }
> 
> As you can see, 1080p is not an available option. 480p is the max.

Yes, I was curious what would happen in the future when we have the UI for the user to select the resolution.  I was sort of hoping for device-specific mechanism to detect applicable resolution, but as you said in comment 28, I guess that's not the case. 

We were worried about hypothetical situation where we enable the user to select 1080p for aries, the same code would also allow to select 1080p in flame. So from comment 30, I guess we don't need to worry about this scenario?  (or any other future ffos phone, since its mozCamera.capabilities.recorderProfiles will only allow the resolution that can be played on itself?)

Comment 33

[Wilson Page [:wilsonpage]]

(In reply to No-Jun Park [:njpark] from comment #32)
> (In reply to Wilson Page [:wilsonpage] from comment #31)
> > 
> >  { 480p: CameraRecorderProfile, qvga: CameraRecorderProfile, vga:
> > CameraRecorderProfile, cif: CameraRecorderProfile, qcif:
> > CameraRecorderProfile, default: CameraRecorderProfile, low:
> > CameraRecorderProfile }
> > 
> > As you can see, 1080p is not an available option. 480p is the max.
> 
> Yes, I was curious what would happen in the future when we have the UI for
> the user to select the resolution.  I was sort of hoping for device-specific
> mechanism to detect applicable resolution, but as you said in comment 28, I
> guess that's not the case. 
> 
> We were worried about hypothetical situation where we enable the user to
> select 1080p for aries, the same code would also allow to select 1080p in
> flame. So from comment 30, I guess we don't need to worry about this
> scenario?  (or any other future ffos phone, since its
> mozCamera.capabilities.recorderProfiles will only allow the resolution that
> can be played on itself?)

Correct. The list displayed to the user is derived from the currently loaded Camera. It is not hard coded.

Comment 34

[:gerard-majax]

Attachment: Expose all Shinano Camera pictures modes

Comment 35

[GitHub Autolander Bot]

Attachment: [gaia] wilsonpage:844920 > mozilla-b2g:master

Comment 36

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 8684178 [details] [review]
[gaia] wilsonpage:844920 > mozilla-b2g:master

LGTM.

Comment 37

[Wilson Page [:wilsonpage]]

Comment on attachment 8684178 [details] [review]
[gaia] wilsonpage:844920 > mozilla-b2g:master

https://github.com/mozilla-b2g/gaia/commit/01014867086a6fdfe9910ff981d5dfc1eea16a96

Comment 39

[:gerard-majax]

Attachment: Add 16:9 picture ratio r=...