Move NV21 colorspace treatment from VideoCapture java to C++

media/video/capture/android/video_capture_device_android.cc

Index: media/video/capture/android/video_capture_device_android.cc
diff --git a/media/video/capture/android/video_capture_device_android.cc b/media/video/capture/android/video_capture_device_android.cc
index 152cfda08756a0c48ac217be9032789306f4ba35..a144ad1ca64148f2a8d40c39861e0aec1fccc23c 100644
--- a/media/video/capture/android/video_capture_device_android.cc
+++ b/media/video/capture/android/video_capture_device_android.cc
@@ -21,19 +21,6 @@ using base::android::MethodID;
 using base::android::JavaRef;
 using base::android::ScopedJavaLocalRef;
 
-namespace {
-
-int GetIntField(JNIEnv* env,
-                const JavaRef<jclass>& clazz,
-                const JavaRef<jobject>& instance,
-                const char* field_name) {
-  jfieldID field = GetFieldID(env, clazz, field_name, "I");
-  jint int_value = env->GetIntField(instance.obj(), field);
-  return int_value;
-}
-
-}  // namespace
-
 namespace media {
 
 // static
@@ -151,7 +138,8 @@ void VideoCaptureDeviceAndroid::Allocate(
       Java_VideoCapture_queryHeight(env, j_capture_.obj());
   current_settings_.frame_rate =
       Java_VideoCapture_queryFrameRate(env, j_capture_.obj());
-  current_settings_.color = PIXEL_FORMAT_YV12;
+  current_settings_.color = GetColorspace();
+  DCHECK_NE(current_settings_.color, media::PIXEL_FORMAT_UNKNOWN);
   CHECK(current_settings_.width > 0 && !(current_settings_.width % 2));
   CHECK(current_settings_.height > 0 && !(current_settings_.height % 2));
 
@@ -256,6 +244,30 @@ void VideoCaptureDeviceAndroid::OnFrameAvailable(
   env->ReleaseByteArrayElements(data, buffer, JNI_ABORT);
 }
 
+VideoPixelFormat VideoCaptureDeviceAndroid::GetColorspace() {
+  JNIEnv* env = AttachCurrentThread();
+  jclass cls = env->FindClass("android/graphics/ImageFormat");

[bulach, 2013/09/11 14:59:07]

please, never write JNI directly yourself.... ;))

afaict, you just want to map the value and share the constants between C++ and
java, right?

here's how to do it:
1. write a "ImageFormat.template", like:
https://code.google.com/p/chromium/codesearch#chromium/src/base/android/java/...

this file will be pre-processed and generated as a java file.

2. write the const values like:
https://code.google.com/p/chromium/codesearch#chromium/src/base/memory/memory...

3. make Java_VideoCapture_getColorspace map and return the value from (1)

4. use directly the enum here.

5. profit!! :)

reason being: although at first it seems slightly more complicated, it's
statically typed, so won't be a runtime error should
"android/graphics/ImageFormat" change, or NV12 field go missing, or whatever....
all these checks will be done at build time.. then the API between java and c++
is far clearer too, since Java_VideoCapture_getColorSpace will return the mapped
value with nice comments saying where the enum is coming from, etc..

+
+  int current_capture_colorspace =
+      Java_VideoCapture_getColorspace(env, j_capture_.obj());
+  if (current_capture_colorspace ==
+      env->GetStaticIntField(cls, env->GetStaticFieldID(cls, "YV12", "I"))) {
+    return media::PIXEL_FORMAT_YV12;
+  } else if (current_capture_colorspace ==
+             env->GetStaticIntField(cls,
+                                    env->GetStaticFieldID(cls, "NV21", "I"))) {
+    return media::PIXEL_FORMAT_NV21;
+  } else if (current_capture_colorspace ==
+             env->GetStaticIntField(cls,
+                                    env->GetStaticFieldID(cls, "YUY2", "I"))) {
+    return media::PIXEL_FORMAT_YUY2;
+  } else {
+    // NOTE(mcasas): Other formats include NV16, JPEG, RGB565, and are not
+    // supported see http://goo.gl/iPbdYj .
+    return media::PIXEL_FORMAT_UNKNOWN;
+  }
+}
+
 void VideoCaptureDeviceAndroid::SetErrorState(const std::string& reason) {
   LOG(ERROR) << "VideoCaptureDeviceAndroid::SetErrorState: " << reason;
   {