Hold video frame in Bitmap instead of keeping a ByteBuffer reference.

remoting/client/jni/chromoting_jni_runtime.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "remoting/client/jni/chromoting_jni_runtime.h"
 
 #include "base/android/base_jni_registrar.h"
 #include "base/android/jni_android.h"
 #include "base/android/scoped_java_ref.h"
 #include "base/memory/singleton.h"
@@ skipping 153 matching lines @@
       host_jstr,
       id_arr,
       secret_arr);
 
   // Because we passed them as arguments, their corresponding Java objects were
   // GCd as soon as the managed method returned, so we mustn't release it here.
 }
 
 void ChromotingJniRuntime::UpdateImageBuffer(int width,
                                              int height,
-                                             jobject buffer) {
+                                             uint8* data) {
   DCHECK(display_task_runner_->BelongsToCurrentThread());
 
   JNIEnv* env = base::android::AttachCurrentThread();
-  env->SetStaticIntField(
+
+  base::android::ScopedJavaLocalRef<jobject> buffer(env,
+      env->NewDirectByteBuffer(data, width * height * kBytesPerPixel));

[Sergey Ulanov, 2013/09/25 23:32:08]

frame stride may not be equal width*kBytesPerPixel (though it's not the case
with BasicDesktopFrame). I think you also need to pass frame stride to
setVideoFrame.

[solb, 2013/09/26 14:21:36]

I can see the argument for this, but at the same time, the setVideoFrame()
method's choice of BitmapConfig assumes kBytesPerPixel will always be 4, and
supporting different values would require additional code in that method to
handle cases we don't need.  At the very most, we'd be using the passed value to
print an error/warning if it didn't match our expectations.

[Lambros, 2013/09/26 22:29:37]

It's possible to deal with strides that are different from the width, but it may
be less efficient. The Bitmap ctors that accept a stride argument also require
passing an int[] array holding the pixels. That means the new Bitmap will read
and process the pixels from int[], as well as during copyPixelsFromBuffer().

Also, Bitmap expects a stride measured in pixels but DesktopFrame exposes its
stride in bytes. So we'd have to divide it by 4 before passing it to Java (and
DCHECK that it's a multiple of 4).

I'm not sure if it's worth supporting different strides, so I've just added a
DCHECK that the stride is width * kBytesPerPixel.

+
+  env->CallStaticVoidMethod(
       class_,
-      env->GetStaticFieldID(class_, "sWidth", "I"),
-      width);
-  env->SetStaticIntField(
-      class_,
-      env->GetStaticFieldID(class_, "sHeight", "I"),
-      height);
-  env->SetStaticObjectField(
-      class_,
-      env->GetStaticFieldID(class_, "sBuffer", "Ljava/nio/ByteBuffer;"),
-      buffer);
+      env->GetStaticMethodID(
+          class_,
+          "setVideoFrame",
+          "(IILjava/nio/ByteBuffer;)V"),
+      width,
+      height,
+      buffer.obj());
 }
 
 void ChromotingJniRuntime::UpdateCursorShape(
     const protocol::CursorShapeInfo& cursor_shape) {
   DCHECK(display_task_runner_->BelongsToCurrentThread());
 
   // const_cast<> is safe as long as the Java updateCursorShape() method copies
   // the data out of the buffer without mutating it, and doesn't keep any
   // reference to the buffer afterwards. Unfortunately, there seems to be no way
   // to create a read-only ByteBuffer from a pointer-to-const.
@@ skipping 25 matching lines @@
       class_,
       env->GetStaticMethodID(class_, "redrawGraphicsInternal", "()V"));
 }
 
 void ChromotingJniRuntime::DetachFromVmAndSignal(base::WaitableEvent* waiter) {
   base::android::DetachFromVM();
   waiter->Signal();
 }
 
 }  // namespace remoting