Android CaptureAPI Types

I extended the current CaptureAPIType exposure on chrome://media-internals page to work on ANDROID with the enum types defined below.

    enum CaptureApiType {
      API1,
      API2_LEGACY,
      API2_FULL,
      API2_LIMITED,
      TANGO,
      API_TYPE_UNKNOWN
    };

Enums are sourced from media/video/capture/video_capture_device.h file and auto-generated on the Android Java side by using java_cpp_enum scripts. They correspond to the same int values, and this is how it is communicated to cpp side on media/video/capture/android/video_capture_device_factory_android.cc.

                        +------------+
                        |VideoCapture|
                        +--+-------+-+
                           |       |
          +----------------+-+   +-+-----------------+
          |VideoCaptureCamera|   |VideoCaptureCamera2|
          +---+------------+-+   +-------------------+
              |            |
+-------------+-----+  ----+-------------+
|VideoCaptureAndroid|  |VideoCaptureTango|
+-------------------+  +-----------------+

On Java side, camera class structure is as above. CaptureApiType is defined by static getCaptureApiType() methods in each child.

Note that media/video/capture/android/imageformat_list.h is deleted as it is currently not used. Instead, enums defined on media/video/capture/android/video_capture_device_factory_android.h were autogenerated on Java side using java_cpp_enum scripts.

BUG=458743
TEST=Built with gyp and deployed:
- on Nexus 7, shows API2_LEGACY
- on Nexus 5, shows API2_LIMITED for front camera and API2_FULL for back camera
I added Android tests to MediaInternalsVideoCaptureDeviceTest.* and tested on Nexus 7.
Looked for the adding unittests on video_capture_device_unittest but ANDROID tests are currently disabled with a TODO specified[0]. I will investigate further, but let me know if you have suggestions.

[0] https://code.google.com/p/chromium/codesearch#chromium/src/media/video/capture/video_capture_device_unittest.cc&l=42

Committed: https://crrev.com/86e26b25645c2661089e72df2e2625745dc6e758
Cr-Commit-Position: refs/heads/master@{#322066}

[emircan, 2015-03-18 19:32:09]

Patchset #1 (id:1) has been deleted

[emircan, 2015-03-18 19:32:16]

Patchset #1 (id:20001) has been deleted

[emircan, 2015-03-18 19:37:28]

Patchset #1 (id:40001) has been deleted

[emircan, 2015-03-18 20:03:44]

emircan@chromium.org changed reviewers:
+ mcasas@chromium.org, perkj@chromium.org

[emircan, 2015-03-18 22:37:20]

PTAL.

[mcasas, 2015-03-19 01:55:14]

Almost there: A few things.

https://codereview.chromium.org/1014943005/diff/80001/android_webview/libwebv...
File android_webview/libwebviewchromium.gypi (right):

https://codereview.chromium.org/1014943005/diff/80001/android_webview/libwebv...
android_webview/libwebviewchromium.gypi:33:
'../media/media.gyp:media_android_capturapitype',
Why not "_java" here and in l.34?

Also, there's an "e" missing between "captur" and "apitype".

https://codereview.chromium.org/1014943005/diff/80001/media/base/android/java...
File media/base/android/java/src/org/chromium/media/VideoCaptureAndroid.java
(right):

https://codereview.chromium.org/1014943005/diff/80001/media/base/android/java...
media/base/android/java/src/org/chromium/media/VideoCaptureAndroid.java:63:
return null;
Why the change? See the third box of 
https://source.android.com/source/code-style.html#use-standard-brace-style

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

https://codereview.chromium.org/1014943005/diff/80001/media/base/android/java...
media/base/android/java/src/org/chromium/media/VideoCaptureCamera2.java:310:
return CaptureApiType.API_TYPE_UNKNOWN;
In Java style, if blocks must have {} even if they're just one line

https://source.android.com/source/code-style.html#use-standard-brace-style

https://codereview.chromium.org/1014943005/diff/80001/media/base/android/java...
File media/base/android/java/src/org/chromium/media/VideoCaptureTango.java
(right):

https://codereview.chromium.org/1014943005/diff/80001/media/base/android/java...
media/base/android/java/src/org/chromium/media/VideoCaptureTango.java:72: return
CaptureApiType.API_TYPE_UNKNOWN;
If it's not Tango, it's Camera API 1.

https://codereview.chromium.org/1014943005/diff/80001/media/media.gyp
File media/media.gyp (right):

https://codereview.chromium.org/1014943005/diff/80001/media/media.gyp#newcode...
media/media.gyp:1815: 'media_android_capturapitype',
Re. my comments about the typo, missing "e".

https://codereview.chromium.org/1014943005/diff/80001/media/video/capture/and...
File media/video/capture/android/imageformat_list.h (left):

https://codereview.chromium.org/1014943005/diff/80001/media/video/capture/and...
media/video/capture/android/imageformat_list.h:18:
DEFINE_ANDROID_IMAGEFORMAT(ANDROID_IMAGEFORMAT_UNKNOWN, 0)
Update the CL description to indicate why are you removing this file.

Is there any other auto-generated magic that could help us with these
enums? (Note that they don't need to coincide with the Java enum
values, just need to be the same on both sides of JNI).

https://codereview.chromium.org/1014943005/diff/80001/media/video/capture/vid...
File media/video/capture/video_capture_device.cc (right):

https://codereview.chromium.org/1014943005/diff/80001/media/video/capture/vid...
media/video/capture/video_capture_device.cc:98: return "API1";
The human readable strings (and their counterparts in 
media_internals_unittest.cc could be more readable, sth like
"Camera API v1", "v2 Full" etc. (They are, essentially, read by
humans).

[emircan, 2015-03-19 19:52:23]

I added a fix for FakeVCD tests.

Please take a look.

https://codereview.chromium.org/1014943005/diff/80001/android_webview/libwebv...
File android_webview/libwebviewchromium.gypi (right):

https://codereview.chromium.org/1014943005/diff/80001/android_webview/libwebv...
android_webview/libwebviewchromium.gypi:33:
'../media/media.gyp:media_android_capturapitype',
On 2015/03/19 01:55:14, mcasas wrote:
> Why not "_java" here and in l.34?
> 
> Also, there's an "e" missing between "captur" and "apitype".

Done.

The build targets inside /base/ do not have *_java naming either, so I wasn't
sure if it folder-based suffix convention and there isn't any clear
documentation around. So, I decided to keep it the way it is.

https://codereview.chromium.org/1014943005/diff/80001/media/base/android/java...
File media/base/android/java/src/org/chromium/media/VideoCaptureAndroid.java
(right):

https://codereview.chromium.org/1014943005/diff/80001/media/base/android/java...
media/base/android/java/src/org/chromium/media/VideoCaptureAndroid.java:63:
return null;
On 2015/03/19 01:55:14, mcasas wrote:
> Why the change? See the third box of 
> https://source.android.com/source/code-style.html#use-standard-brace-style

Done.

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

https://codereview.chromium.org/1014943005/diff/80001/media/base/android/java...
media/base/android/java/src/org/chromium/media/VideoCaptureCamera2.java:310:
return CaptureApiType.API_TYPE_UNKNOWN;
On 2015/03/19 01:55:14, mcasas wrote:
> In Java style, if blocks must have {} even if they're just one line
> 
> https://source.android.com/source/code-style.html#use-standard-brace-style

Done.

https://codereview.chromium.org/1014943005/diff/80001/media/base/android/java...
File media/base/android/java/src/org/chromium/media/VideoCaptureTango.java
(right):

https://codereview.chromium.org/1014943005/diff/80001/media/base/android/java...
media/base/android/java/src/org/chromium/media/VideoCaptureTango.java:72: return
CaptureApiType.API_TYPE_UNKNOWN;
On 2015/03/19 01:55:14, mcasas wrote:
> If it's not Tango, it's Camera API 1.

Done.

https://codereview.chromium.org/1014943005/diff/80001/media/media.gyp
File media/media.gyp (right):

https://codereview.chromium.org/1014943005/diff/80001/media/media.gyp#newcode...
media/media.gyp:1815: 'media_android_capturapitype',
On 2015/03/19 01:55:14, mcasas wrote:
> Re. my comments about the typo, missing "e".

Done.

https://codereview.chromium.org/1014943005/diff/80001/media/video/capture/and...
File media/video/capture/android/imageformat_list.h (left):

https://codereview.chromium.org/1014943005/diff/80001/media/video/capture/and...
media/video/capture/android/imageformat_list.h:18:
DEFINE_ANDROID_IMAGEFORMAT(ANDROID_IMAGEFORMAT_UNKNOWN, 0)
On 2015/03/19 01:55:13, mcasas wrote:
> Update the CL description to indicate why are you removing this file.
> 
> Is there any other auto-generated magic that could help us with these
> enums? (Note that they don't need to coincide with the Java enum
> values, just need to be the same on both sides of JNI).

I added an explanation to why it is removed.

We could remove the current assigned values on these enums and they will
automatically be assigned {0, 1, 2,.., n} and would still work. However, they
wouldn't correspond to the values specified on the Android documentation
specified in the comments.
http://developer.android.com/reference/android/graphics/ImageFormat.html

https://codereview.chromium.org/1014943005/diff/80001/media/video/capture/vid...
File media/video/capture/video_capture_device.cc (right):

https://codereview.chromium.org/1014943005/diff/80001/media/video/capture/vid...
media/video/capture/video_capture_device.cc:98: return "API1";
On 2015/03/19 01:55:14, mcasas wrote:
> The human readable strings (and their counterparts in 
> media_internals_unittest.cc could be more readable, sth like
> "Camera API v1", "v2 Full" etc. (They are, essentially, read by
> humans).

I see. I am naming them "Camera API2 Full" like specified on
https://source.android.com/devices/camera/versioning.html

[mcasas, 2015-03-20 17:46:42]

I think we can improve the wording, i.e.

API2_LEGACY --> API_V2_LEGACY

and 

"Camera API2" --> "Camera API v2"

Other than that LGTM.

[emircan, 2015-03-20 21:12:04]

On 2015/03/20 17:46:42, mcasas wrote:
> I think we can improve the wording, i.e.
> 
> API2_LEGACY --> API_V2_LEGACY
> 
> and 
> 
> "Camera API2" --> "Camera API v2"
> 
> Other than that LGTM.

Acknowledged. I followed the naming on Android documentation:
https://source.android.com/devices/camera/versioning.html . I think keeping it
this way would be better for Android devs.

[emircan, 2015-03-20 21:12:48]

emircan@chromium.org changed reviewers:
+ dalecurtis@chromium.org

[emircan, 2015-03-20 21:12:49]

I still need OWNERS review. dalecurtis@ please take a look.

[DaleCurtis, 2015-03-21 01:18:33]

dalecurtis@chromium.org changed reviewers:
+ qinmin@chromium.org

[DaleCurtis, 2015-03-21 01:18:35]

+qinmin for android review.

[qinmin, 2015-03-23 20:37:13]

On 2015/03/21 01:18:35, DaleCurtis wrote:
> +qinmin for android review.

android change LGTM

[emircan, 2015-03-23 20:51:45]

emircan@chromium.org changed reviewers:
+ boliu@chromium.org

[emircan, 2015-03-23 20:51:46]

I still need OWNERS review for android_webview/* and content/* files.

boliu@ and dalecurtis@ please take a look.

[boliu, 2015-03-23 21:24:35]

On 2015/03/23 20:51:46, emircan wrote:
> I still need OWNERS review for android_webview/* and content/* files.
> 
> boliu@ and dalecurtis@ please take a look.

I'm happy to tell you that we don't care about the android_aosp bot anymore.
It's no longer on cq or main waterfall, and in the process of being removed
right now. See crbug.com/468723. So you don't need these changes in
android_webview

[DaleCurtis, 2015-03-23 23:05:19]

rs lgtm

[emircan, 2015-03-24 19:38:01]

The CQ bit was checked by emircan@chromium.org

[emircan, 2015-03-24 19:38:02]

The patchset sent to the CQ was uploaded after l-g-t-m from qinmin@chromium.org,
dalecurtis@chromium.org, mcasas@chromium.org
Link to the patchset: https://codereview.chromium.org/1014943005/#ps150001
(title: "Removed android_webview changes.")

[commit-bot: I haz the power, 2015-03-24 19:39:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1014943005/150001

[commit-bot: I haz the power, 2015-03-24 20:30:01]

Committed patchset #6 (id:150001)

[commit-bot: I haz the power, 2015-03-24 20:31:09]

Patchset 6 (id:??) landed as
https://crrev.com/86e26b25645c2661089e72df2e2625745dc6e758
Cr-Commit-Position: refs/heads/master@{#322066}