Android VideoCaptureCamera2: Use closest supported resolution instead of requested resolution

Android VideoCaptureCamera2: Use closest supported resolution instead of requested resolution

This CL finds the closest supported format in allocate() from the requested resolution, and uses that to open the camera. This is consistent with how VideoCaptureCamera and other camera capture devices work.

BUG=541232
Committed: https://crrev.com/fff0ba0a41392542d3b6238ab928f96e1ddad7b0
Cr-Commit-Position: refs/heads/master@{#357066}

[magjed1, 2015-10-28 10:04:41]

Description was changed from

==========
Android VideoCaptureCamera2: Use closest supported resolution in allocate()
instead of requested resolution

This CL finds the closest supported format in allocate() from the requested
resolution, and uses that to open the camera. This is consistent with how
VideoCaptureCamera and other camera capture devices work.

BUG=541232
==========

to

==========
Android VideoCaptureCamera2: Use closest supported resolution in allocate()
instead of requested resolution

This CL finds the closest supported format in allocate() from the requested
resolution, and uses that to open the camera. This is consistent with how
VideoCaptureCamera and other camera capture devices work.

BUG=541232
==========

[magjed1, 2015-10-28 10:04:41]

magjed@google.com changed reviewers:
+ mcasas@chromium.org

[magjed1, 2015-10-28 10:05:27]

Description was changed from

==========
Android VideoCaptureCamera2: Use closest supported resolution in allocate()
instead of requested resolution

This CL finds the closest supported format in allocate() from the requested
resolution, and uses that to open the camera. This is consistent with how
VideoCaptureCamera and other camera capture devices work.

BUG=541232
==========

to

==========
Android VideoCaptureCamera2: Use closest supported resolution instead of
requested resolution

This CL finds the closest supported format in allocate() from the requested
resolution, and uses that to open the camera. This is consistent with how
VideoCaptureCamera and other camera capture devices work.

BUG=541232
==========

[magjed_chromium, 2015-10-28 10:37:26]

Miguel - Please take a look.

[mcasas, 2015-10-28 17:20:43]

Looking good, some comments.

https://codereview.chromium.org/1416253005/diff/1/media/base/android/java/src...
File media/base/android/java/src/org/chromium/media/VideoCaptureCamera2.java
(right):

https://codereview.chromium.org/1416253005/diff/1/media/base/android/java/src...
media/base/android/java/src/org/chromium/media/VideoCaptureCamera2.java:382:
size.getHeight(),
Do you mind sorting out this indenting plz?

https://codereview.chromium.org/1416253005/diff/1/media/base/android/java/src...
media/base/android/java/src/org/chromium/media/VideoCaptureCamera2.java:425: 
Add here 
if (minDiff == Integer.MAX_VALUE) {
    Log.e(TAG, "blabla");
    return false;
}

Actually this comes from VideoCapture.java [1] that has a similar
algorithm. I wonder if you could factor those two out -- perhaps 
not possible due to different Size classes used (Size here versus 
android.hardware.Camera.Size there).

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/media/base/android...

https://codereview.chromium.org/1416253005/diff/1/media/base/android/java/src...
media/base/android/java/src/org/chromium/media/VideoCaptureCamera2.java:428:
closestSupportedSize.getHeight(), frameRate, format);
nit: I'd just inline |ImageFormat.YUV_420_888| here.

[magjed_chromium, 2015-10-29 11:59:09]

Miguel - Please take another look.

https://codereview.chromium.org/1416253005/diff/1/media/base/android/java/src...
File media/base/android/java/src/org/chromium/media/VideoCaptureCamera2.java
(right):

https://codereview.chromium.org/1416253005/diff/1/media/base/android/java/src...
media/base/android/java/src/org/chromium/media/VideoCaptureCamera2.java:382:
size.getHeight(),
On 2015/10/28 17:20:43, mcasas wrote:
> Do you mind sorting out this indenting plz?

Done, I used clang format on this block.

https://codereview.chromium.org/1416253005/diff/1/media/base/android/java/src...
media/base/android/java/src/org/chromium/media/VideoCaptureCamera2.java:425: 
On 2015/10/28 17:20:43, mcasas wrote:
> Add here 
> if (minDiff == Integer.MAX_VALUE) {
>     Log.e(TAG, "blabla");
>     return false;
> }
> 
> Actually this comes from VideoCapture.java [1] that has a similar
> algorithm. I wonder if you could factor those two out -- perhaps 
> not possible due to different Size classes used (Size here versus 
> android.hardware.Camera.Size there).
> 
> [1]
>
https://code.google.com/p/chromium/codesearch#chromium/src/media/base/android...

Done, added |minDiff| check.

Yeah, I looked at VideoCaptureCamera.java when I wrote this, but I don't think
it's worth refactoring.

https://codereview.chromium.org/1416253005/diff/1/media/base/android/java/src...
media/base/android/java/src/org/chromium/media/VideoCaptureCamera2.java:428:
closestSupportedSize.getHeight(), frameRate, format);
On 2015/10/28 17:20:43, mcasas wrote:
> nit: I'd just inline |ImageFormat.YUV_420_888| here.

Done.

[mcasas, 2015-10-29 22:51:37]

lgtm

[magjed_chromium, 2015-10-30 08:39:11]

The CQ bit was checked by magjed@chromium.org

[commit-bot: I haz the power, 2015-10-30 08:39:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1416253005/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1416253005/20001

[commit-bot: I haz the power, 2015-10-30 08:46:34]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-10-30 08:47:16]

Patchset 2 (id:??) landed as
https://crrev.com/fff0ba0a41392542d3b6238ab928f96e1ddad7b0
Cr-Commit-Position: refs/heads/master@{#357066}