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

remoting/client/jni/jni_frame_consumer.h

 // 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.
 
 #ifndef REMOTING_CLIENT_JNI_JNI_FRAME_CONSUMER_H_
 #define REMOTING_CLIENT_JNI_JNI_FRAME_CONSUMER_H_
 
 #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
 
 namespace remoting {
 class ChromotingJniRuntime;
 class FrameProducer;
 
 // FrameConsumer implementation that draws onto a JNI direct byte buffer.
 class JniFrameConsumer : public FrameConsumer {
@@ skipping 10 matching lines @@
   virtual void ApplyBuffer(const webrtc::DesktopSize& view_size,
                            const webrtc::DesktopRect& clip_area,
                            webrtc::DesktopFrame* buffer,
                            const webrtc::DesktopRegion& region) OVERRIDE;
   virtual void ReturnBuffer(webrtc::DesktopFrame* buffer) OVERRIDE;
   virtual void SetSourceSize(const webrtc::DesktopSize& source_size,
                              const webrtc::DesktopVector& dpi) OVERRIDE;
   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_;
 
   FrameProducer* frame_producer_;
   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);
 };
 
 }  // namespace remoting
 
 #endif