974264 - [MADAI][Camera] abnormal preview size

Status: RESOLVED WORKSFORME

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

Description

[hyuna.cho]

Attachment: Log_camera_crash.txt

* Precondition : Settings - Developer - check 'Home gesture enabled'

When configure the preview size, the viewport size(window.innerWidth/window.innerHeight) set as 384*640 or 360*640.
* Q1. Why the viewport size set as 384*640(360*640) instead of actual device size?

In CameraUtils.selectOptimalPreviewSize function in camera-utils.js, select the optimal preview size using mozCamera.capabilities.previewSizes.
At that time, abnormal abnormal preview size selected in some devices.
* Q2. The width/height of mozCamera.capabilities.previewSize set like 1280*720. It's reverse value with viewportSize. Is it normal?

[In Nexus4 - capabilities.previewSize]
1280	720
800	480
768	432
720	480
640	480
576	432
480	320
384	288
352	288 -> selected preview size
320	240
240	160
176	144

[In other device - capabilities.previewSize]
1920	1080
1776	1080
1440	1080
1280	960
1280	720
960	720
880	720 -> selected preview size
848	480
800	480
768	432
720	480
640	480
576	432
480	320
384	288
352	288
320	240
240	160
176	144

* Q3. The viewport size set as 360*640 but the preview size set as 880*720. At that time, camera crash when launch camera app.

I attached the log file. 
You can find the logs using 'previewSizes' keyword.

Comment 1

[Justin D'Arcangelo [:justindarc]]

It looks as though the viewport is being reported as 360x640 instead of 640x360 (width and height are flipped). The `CameraUtils.selectOptimalPreviewSize` function attempts to select the preview size that has the least amount of overflow area outside of the viewport. To do this, it must first scale all of the preview sizes to "aspect-fill" in order to cover the entire viewport. It then selects the preview size with the least amount of overflow area that requires the least amount of scale adjustment.

Based on this line in the log...

01-02 00:46:48.190 E/GeckoConsole( 8074): Content JS LOG at app://camera_ref.gaiamobile.org/js/main.js:3667 in Camera.prototype.configurePicturePreviewSize: viewportSize.width: 360,  viewportSize.height: 640

...I think the source of the problem being reported here has to do with the fact that the viewport dimensions seem to be reversed. All of the available preview sizes from the camera hardware have a larger width than height.

Comment 2

[Justin D'Arcangelo [:justindarc]]

To clarify Q2, YES the viewport dimensions seem be be backwards (this is not normal). Because the viewport dimensions are being reported backwards (360x640), its aspect ratio is appearing abnormally low (0.5625). Because of this low aspect ratio, there are only 3 preview sizes that are eligible for selection because they have the lowest aspect ratio of all available preview sizes:

 880 x  720 = 1.222 aspect ratio
 352 x  288 = 1.222 aspect ratio
 176 x  144 = 1.222 aspect ratio

Now, if the viewport size was being reported correctly (640x360, 1.778 aspect ratio), the following preview sizes would be eligible:

1920 x 1080 = 1.778 aspect ratio
1280 x  720 = 1.778 aspect ratio
 768 x  432 = 1.778 aspect ratio

Since these preview sizes have the exact same aspect ratio as 640x360, there would be ZERO overflow outside of the viewport. If the viewport size was being reported correctly, the selected preview should have been 768x432 since it would require the least amount of scale adjustment to aspect-fill the viewport.

Comment 3

[hyuna.cho]

(In reply to Justin D'Arcangelo from comment #1)
> It looks as though the viewport is being reported as 360x640 instead of
> 640x360 (width and height are flipped). The
> `CameraUtils.selectOptimalPreviewSize` function attempts to select the
> preview size that has the least amount of overflow area outside of the
> viewport. To do this, it must first scale all of the preview sizes to
> "aspect-fill" in order to cover the entire viewport. It then selects the
> preview size with the least amount of overflow area that requires the least
> amount of scale adjustment.
> 
> Based on this line in the log...
> 
> 01-02 00:46:48.190 E/GeckoConsole( 8074): Content JS LOG at
> app://camera_ref.gaiamobile.org/js/main.js:3667 in
> Camera.prototype.configurePicturePreviewSize: viewportSize.width: 360, 
> viewportSize.height: 640
> 
> ...I think the source of the problem being reported here has to do with the
> fact that the viewport dimensions seem to be reversed. All of the available
> preview sizes from the camera hardware have a larger width than height.

In Nexus, the viewport size set as 384*640 using window.innerWidth/window.innerHeight also.
So I wonder where/how these low dimension are set? The actual display dimensions are 1280*720(16:9) or 1280*960(4:3)
Because the viewport set to the low dimension, the viewfinder resolution is very poor.

I changed the viewport size set to 352*288(it's the selected preview size on nexus) by forcefully in other device. In case of this, the devices doesn't crash. That device has the viewport size with 360*640.
Is this crash related with the preview size setting?
Plz check it.

Comment 4

[Justin D'Arcangelo [:justindarc]]

window.innerWidth/innerHeight are reported in CSS pixels, NOT in device pixels. To get the actual size of the viewport in device pixels, you need to multiply by window.devicePixelRatio (which we should already be doing when selecting the preview size). Can you verify you get the correct resolution on the other device if you multiply by the devicePixelRatio?

Comment 5

[hyuna.cho]

In other device not Nexus4,
I multiply by the window.devicePixelRatio in CameraUtils.selectOptimalPreviewSize function,

    var vh = viewportSize.width,  // I changed width/height by force
        vw = viewportSize.height,
    // vw: 640, vh: 320 in log

    vw = vw * window.devicePixelRatio;  // Full HD LCD
    vh = vh * window.devicePixelRatio;

    // vw: 1920, vh: 1080 in log

selected preview size is 1920*1080 properly and not crashed.


If I don't change width/height,

    var vw = viewportSize.width,  // I changed width/height by force
        vh = viewportSize.height,
    // vw: 360, vh: 640 in log

    vw = vw * window.devicePixelRatio;
    vh = vh * window.devicePixelRatio;

    // vw: 1080, vh: 1920 in log

selected preview size is 880*720 and camera app crashed.

If you want to get other information, leave comments

Comment 6

[Wilson Page [:wilsonpage]]

This is Justin's area, so I'm leaving discussions to him.

Comment 7

[Justin D'Arcangelo [:justindarc]]

Attachment: Patch to correct the viewport size

It appears as though the viewport width/height got reversed during the new CameraControl API refactor. It was also not using device pixels for selecting the optimal preview size which was causing it to select some really bad preview sizes.

Comment 8

[Ahmed Nefzaoui [:Nefzaoui]]

(In reply to Justin D'Arcangelo from comment #7)

> It appears as though the viewport width/height got reversed during the new
> CameraControl API refactor.

So would Bug 973597 relate to that ?

Comment 9

[Diego Marcos [:dmarcos]]

No. I don't think Bug 973597 relates. This bug only affects the way we configure the size of the preview video stream returned by the camera hardware.

Comment 10

[hyuna.cho]

(In reply to Justin D'Arcangelo from comment #7)
> Created attachment 8378444 [details] [review]
> Patch to correct the viewport size
> 
> It appears as though the viewport width/height got reversed during the new
> CameraControl API refactor. It was also not using device pixels for
> selecting the optimal preview size which was causing it to select some
> really bad preview sizes.

Could you check in nexus4?

This patches can resolve resolution issue that mismatch with actual device pixel.
But the incorrect preview issue is remaining in nexus4. Because the viewport size set as 384*640 not 360*640.
I know nexus4 can set up to 1280*720, but the selected preview size is 800*480.
(Refer to the first comment)

Plz check nexus also.

Comment 11

[Justin D'Arcangelo [:justindarc]]

We are investigating right now to see why the viewport size seems to be being reported incorrectly on Nexus 4.

Comment 12

[Justin D'Arcangelo [:justindarc]]

A quick Google search reveals that Nexus 4 has a resolution of 1280 x 768 pixels with a device pixel ratio of 2.0 which yields 640 x 384. The reason the selected preview size is 800 x 480 is because it fits the aspect ratio of the screen perfectly (1280 x 720 does not). So, with the selected preview size of 800 x 480, a scale of 1.6 is applied to the preview which ends up being 1280 x 768 (exact resolution of the Nexus 4 display). So, currently, the priority is matching the closest aspect ratio so that there is a minimal amount of overflow/cut-off outside of the viewport.

Wilson: Could we make a setting to allow the priority of the "optimal" preview size to be specified at build-time? I'm thinking there would be at least two options:

1.) Aspect-Ratio, Scale
2.) Scale, Aspect-Ratio

If we had the ability to "flip" the priority of preview size selection, option 2 would find the preview size requiring the least amount of scale first and then the closest aspect-ratio would be used as a tie-breaker. In that case, the 1280 x 720 preview size would be selected because it would only require a scale of 1.067 to fill the viewport and that would be the smallest scale adjustment of all the preview sizes.

We could always just "flip" the priority by default (scale first, aspect-ratio second), but I'm not sure if this would then have any negative consequences for other devices? That's why I'm thinking it might be beneficial to let this be a build-time configuration option.

Comment 13

[Justin D'Arcangelo [:justindarc]]

Comment on attachment 8378444 [details] [review]
Patch to correct the viewport size

Updated the PR to have a maximum scale threshold for the preview size. So, in the case of the Nexus4, this should result in it selecting the 1280x720 preview size.

Comment 14

[Diego Marcos [:dmarcos]]

Hyuna. Can you confirm if the new patch resolves your problem?

Comment 15

[hyuna.cho]

(In reply to Diego Marcos from comment #14)
> Hyuna. Can you confirm if the new patch resolves your problem?

It's reproduced.
In CameraUtils.selectOptimalPreviewSize function, 'optimalPreviewSize.scale' value is 'undefined'.
Is it normal?

Comment 16

[Diego Marcos [:dmarcos]]

Hyuna. We still problems with the selection logic. I commented in the pull request. We will fix them as soon as we can. I apologize for the inconvenience.

Comment 17

[Justin D'Arcangelo [:justindarc]]

Comment on attachment 8378444 [details] [review]
Patch to correct the viewport size

Diego: I fixed the `undefined` issue in the PR you reviewed. I forgot that the two different selection functions were returning *actual* previewSizes (the ones returned from the Camera API) instead of the augmented objects with scale/overflow attributes used for selection. I changed them to return the full objects so selectOptimalPreviewSize() can correctly determine which one to use.

I also updated the unit test to expect the correct result. I guess that should have been a dead giveaway that the unit tests were still passing!! Apologies, my bad :-/

Comment 18

[Justin D'Arcangelo [:justindarc]]

Just received my Nexus 4 this morning! Will be looking at this closer later today.

Comment 19

[Justin D'Arcangelo [:justindarc]]

Wilson: I was able to reproduce this on my Nexus 4 with the old `previewSize` selection that was based on the viewport size. However, this seems to not be an issue anymore with the new `previewSize` selection that is based on the selected `pictureSize` from settings. Can you verify this since this is your implementation? If that's the case, I can probably just close out my PR.

Comment 20

[Wilson Page [:wilsonpage]]

Confirmed with Hyuna that latest previewSizes selection logic is 'as desired'.

We are picking the largest `previewSize` that matches (or closest match) to the aspect ratio of the chosen `pictureSize`.

Comment 21

[Justin D'Arcangelo [:justindarc]]

Sounds good, marking this as resolved. Thanks for verifying, Wilson!