VideoCapturerAndroid, handle cvo correctly

talk/app/webrtc/java/jni/native_handle_impl.cc

Index: talk/app/webrtc/java/jni/native_handle_impl.cc
diff --git a/talk/app/webrtc/java/jni/native_handle_impl.cc b/talk/app/webrtc/java/jni/native_handle_impl.cc
index 583a0380263659096660934cae8699babd5c7511..d9337a7548be79d2d5d6d9dbe7e32f679a5e15b1 100644
--- a/talk/app/webrtc/java/jni/native_handle_impl.cc
+++ b/talk/app/webrtc/java/jni/native_handle_impl.cc
@@ -30,9 +30,66 @@
 #include "webrtc/base/checks.h"
 #include "webrtc/base/keep_ref_until_done.h"
 #include "webrtc/base/scoped_ref_ptr.h"
+#include "webrtc/base/logging.h"
 
 using webrtc::NativeHandleBuffer;
 
+namespace {
+
+const float ROTATE_90[16] =
+    {0.0, 1.0, 0.0, 0.0,
+    -1.0, 0.0, 0.0, 0.0,
+     0.0, 0.0, 1.0, 0.0,
+     1.0, 0.0 , 0.0, 1.0};
+const float ROTATE_180[16] =
+    {-1.0, 0.0, 0.0, 0.0,
+     0.0, -1.0, 0.0, 0.0,
+     0.0, 0.0, 1.0, 0.0,
+     1.0, 1.0, 0.0, 1.0};
+const float ROTATE_270[16] =
+    {0.0, -1.0, 0.0, 0.0,
+     1.0, 0.0, 0.0, 0.0,
+     0.0, 0.0, 1.0, 0.0,
+     0.0, 1.0, 0.0, 1.0};
+
+void InPlaceMultiplyMatrix(float a[16], const float b[16]) {

[nisse-chromium (ooo August 14), 2015/12/08 08:46:34]

Please explain operation. Is it A <-- A * B or A <-- B * A (in math. matrix
notation)? Is it using row-major or column-major order?

[perkj_chrome, 2015/12/09 21:00:40]

not relevant now?

+    float ab[16];
+    int ab_ofs = 0;
+    int b_ofs = 0;
+    for (int i = 0; i < 4; i++) {
+      ab[ab_ofs + 0] =
+          (b[b_ofs + 0] * a[0]) +
+          (b[b_ofs + 1] * a[4]) +
+          (b[b_ofs + 2] * a[8]) +
+          (b[b_ofs + 3] * a[12]) ;
+
+      ab[ab_ofs + 1] =
+          (b[b_ofs + 0] * a[1]) +
+          (b[b_ofs + 1] * a[5]) +
+          (b[b_ofs + 2] * a[9]) +
+          (b[b_ofs + 3] * a[13]) ;
+
+      ab[ab_ofs + 2] =
+          (b[b_ofs + 0] * a[2]) +
+          (b[b_ofs + 1] * a[6]) +
+          (b[b_ofs + 2] * a[10]) +
+          (b[b_ofs + 3] * a[14]) ;
+
+      ab[ab_ofs + 3] =
+          (b[b_ofs + 0] * a[3]) +
+          (b[b_ofs + 1] * a[7]) +
+          (b[b_ofs + 2] * a[11]) +
+          (b[b_ofs + 3] * a[15]) ;
+      ab_ofs += 4;

[nisse-chromium (ooo August 14), 2015/12/08 08:46:34]

Move these updates up to the for(;;...). Maybe rename *_ofs to *_row (if
matrices are row-major).

[perkj_chrome, 2015/12/09 21:00:40]

Acknowledged.

+      b_ofs += 4;
+    }
+
+    memcpy(static_cast<void*>(a), static_cast<const void*>(ab),
+        16*sizeof(float));

[nisse-chromium (ooo August 14), 2015/12/08 08:46:34]

Replace the size expression with sizeof(ab). (Unfortunately, sizeof(a) won't
work, since a really is a pointer; the argument array syntax is only sugar).

Regarding the whole function, I see that we only touch one half of the matrix,
so maybe something simpler and more explicit would actually be shorter. Like, an
outer switch, and then in each case, loop over the columns of a, read out the
two affected values, write back rotated values in place.

[perkj_chrome, 2015/12/09 21:00:40]

Done.

+}
+
+}  // anonymouse namespace
+
 namespace webrtc_jni {
 
 NativeHandleImpl::NativeHandleImpl(JNIEnv* jni,
@@ -68,22 +125,45 @@ AndroidTextureBuffer::NativeToI420Buffer() {
   return nullptr;
 }
 
-rtc::scoped_refptr<AndroidTextureBuffer> AndroidTextureBuffer::CropAndScale(
-    int cropped_input_width,
-    int cropped_input_height,
-    int dst_widht,
-    int dst_height) {
+rtc::scoped_refptr<AndroidTextureBuffer>
+AndroidTextureBuffer::CropScaleAndRotate(int cropped_input_width,

[nisse-chromium (ooo August 14), 2015/12/08 08:46:34]

Maybe we could come up with a better name. TransformFrame or something?

Maybe drop the crop-related arguments, until that is actually implemented. It's
not clear to me how they're intended to be used, and for cropping, won't you
also need some offsets for positioning of the rectangle you cut out?

[perkj_chrome, 2015/12/09 21:00:40]

If you don't object too much I will call this ScaleAndRotate in that case.

[nisse-chromium (ooo August 14), 2015/12/10 09:04:57]

Good enough.

+                                         int cropped_input_height,
+                                         int dst_widht,
+                                         int dst_height,
+                                         webrtc::VideoRotation rotation) {
   // TODO(perkj) Implement cropping.
   RTC_CHECK_EQ(cropped_input_width, width_);
   RTC_CHECK_EQ(cropped_input_height, height_);
+  if (width() == dst_widht && height() == dst_height &&
+      rotation == webrtc::kVideoRotation_0) {
+    return this;
+  }
+
+  int rotated_width = (rotation % 180 == 0) ? dst_widht : dst_height;
+  int rotated_height = (rotation % 180 == 0) ? dst_height : dst_widht;
+
+  rtc::scoped_refptr<AndroidTextureBuffer> buffer(
+      new rtc::RefCountedObject<AndroidTextureBuffer>(
+          rotated_width, rotated_height, native_handle_,
+          rtc::KeepRefUntilDone(this)));
+
+  switch (rotation) {
+  case webrtc::kVideoRotation_0:
+    break;
+  case webrtc::kVideoRotation_90:
+    InPlaceMultiplyMatrix(buffer->native_handle_.sampling_matrix, ROTATE_90);
+    break;
+  case webrtc::kVideoRotation_180:
+    InPlaceMultiplyMatrix(buffer->native_handle_.sampling_matrix,
+        ROTATE_180);
+    break;
+  case webrtc::kVideoRotation_270:
+    InPlaceMultiplyMatrix(buffer->native_handle_.sampling_matrix,
+        ROTATE_270);
+    break;
+  }
 
-  // Here we use Bind magic to add a reference count to |this| until the newly
-  // created AndroidTextureBuffer is destructed. ScaledFrameNotInUse will be
-  // called that happens and when it finishes, the reference count to |this|

[nisse-chromium (ooo August 14), 2015/12/08 08:46:34]

Keep this comment, if it is still relevant?

[perkj_chrome, 2015/12/09 21:00:40]

Done.

-  // will be decreased by one.
-  return new rtc::RefCountedObject<AndroidTextureBuffer>(
-      dst_widht, dst_height, native_handle_,
-      rtc::KeepRefUntilDone(this));
+  return buffer;
 }
 
 }  // namespace webrtc_jni