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

remoting/client/jni/jni_frame_consumer.h

Index: remoting/client/jni/jni_frame_consumer.h
diff --git a/remoting/client/jni/jni_frame_consumer.h b/remoting/client/jni/jni_frame_consumer.h
index 1509720a739da6059fb476110db9fd598ef705ae..9922adfe9fdbcc58899f30a83043a905734da08b 100644
--- a/remoting/client/jni/jni_frame_consumer.h
+++ b/remoting/client/jni/jni_frame_consumer.h
@@ -7,9 +7,15 @@
 
 #include "remoting/client/frame_consumer.h"
 
+#include "base/android/scoped_java_ref.h"
 #include "base/compiler_specific.h"
+#include "base/memory/scoped_ptr.h"
 #include "third_party/webrtc/modules/desktop_capture/desktop_geometry.h"
 
+namespace gfx {
+class JavaBitmap;
+}  // namespace gfx
+
 namespace webrtc {
 class DesktopFrame;
 }  // namespace webrtc
@@ -40,12 +46,17 @@ class JniFrameConsumer : public FrameConsumer {
   virtual PixelFormat GetPixelFormat() OVERRIDE;
 
  private:
+  // If not called from the destructor, allocates a new buffer of |view_size_|,
+  // informs Java about it, and tells the producer to draw onto it. Otherwise,
+  // this is a no-op.
+  void AllocateBuffer();
+
   // Variables are to be used from the display thread.
 
   // Used to obtain task runner references and make calls to Java methods.
   ChromotingJniRuntime* jni_runtime_;
 
-  // Whether we're currently in the constructor, and should deallocate the
+  // Whether we're currently in the destructor, and should deallocate the
   // buffer instead of passing it back to the producer.
   bool in_dtor_;
 
@@ -53,9 +64,14 @@ class JniFrameConsumer : public FrameConsumer {
   webrtc::DesktopSize view_size_;
   webrtc::DesktopRect clip_area_;
 
-  // If |provide_buffer_|, allocates a new buffer of |view_size_|, informs
-  // Java about it, and tells the producer to draw onto it. Otherwise, no-op.
-  void AllocateBuffer();
+  // Reference to the frame bitmap that is passed to Java when the frame is
+  // allocated. This provides easy access to the underlying pixels.
+  scoped_ptr<gfx::JavaBitmap> bitmap_;

[Sergey Ulanov, 2013/10/08 22:15:02]

JavaBitmap holds bitmap content locked. I think it shouldn't be class member.
Instead you create it only when you need to modify a bitmap object.

[Lambros, 2013/10/10 01:35:58]

We need to keep the Bitmap pixels locked as long as the FrameProducer is
decoding video data into that memory. So the lifetime of the JavaBitmap needs to
be at least as long.

Storing the JavaBitmap in a DesktopFrame-derived class might work, but seems a
bit overkill since we're only creating one buffer at a time?

+
+  // This global reference is required, instead of a local reference, so it
+  // remains valid for the lifetime of |bitmap_| - gfx::JavaBitmap does not
+  // create its own global reference internally.
+  base::android::ScopedJavaGlobalRef<jobject> bitmap_global_ref_;
 
   DISALLOW_COPY_AND_ASSIGN(JniFrameConsumer);
 };