Byte-swap the video frame pixels before passing them to Java.

When a complete video frame is decoded, this CL converts the pixels from
BGRA to a format suitable for loading into a Java Bitmap directly. This
removes the need to create a temporary int[] array in Java.

Review URL: https://codereview.chromium.org/23677011

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@226405 0039d316-1c4b-4281-b951-d872f2087c98

12 files changed, 143 insertions, 48 deletions

diff --git a/remoting/android/java/src/org/chromium/chromoting/jni/JniInterface.java b/remoting/android/java/src/org/chromium/chromoting/jni/JniInterface.java
index bd2a25e..9816970 100644
--- a/remoting/android/java/src/org/chromium/chromoting/jni/JniInterface.java
+++ b/remoting/android/java/src/org/chromium/chromoting/jni/JniInterface.java
@@ -142,6 +142,9 @@ public class JniInterface {
     /** Screen height of the video feed. */
     private static int sHeight = 0;
 
+    /** Bitmap holding the latest screen image. */
+    private static Bitmap sBitmap = null;
+
     /** Buffer holding the video feed. */
     private static ByteBuffer sBuffer = null;
 
@@ -310,12 +313,19 @@ public class JniInterface {
             return null;
         }
 
-        int[] frame = new int[sWidth * sHeight];
-
-        sBuffer.order(ByteOrder.LITTLE_ENDIAN);
-        sBuffer.asIntBuffer().get(frame, 0, frame.length);
+        // This is synchronized only to silence a findbugs warning about incorrect initialization of
+        // |sBitmap|.
+        // TODO(lambroslambrou): Annotate this class as @NotThreadSafe to prevent similar warnings
+        // in future.
+        synchronized (JniInterface.class) {
+            if (sBitmap == null || sBitmap.getWidth() != sWidth || sBitmap.getHeight() != sHeight) {
+                sBitmap = Bitmap.createBitmap(sWidth, sHeight, Bitmap.Config.ARGB_8888);
+            }
+        }
 
-        return Bitmap.createBitmap(frame, 0, sWidth, sWidth, sHeight, Bitmap.Config.ARGB_8888);
+        sBuffer.rewind();
+        sBitmap.copyPixelsFromBuffer(sBuffer);
+        return sBitmap;
     }
 
     /**
diff --git a/remoting/client/frame_consumer.h b/remoting/client/frame_consumer.h
index 4df7595..165cedd 100644
--- a/remoting/client/frame_consumer.h
+++ b/remoting/client/frame_consumer.h
@@ -19,6 +19,13 @@ namespace remoting {
 
 class FrameConsumer {
  public:
+
+  // List of supported pixel formats needed by various platforms.
+  enum PixelFormat {
+    FORMAT_BGRA,  // Used by the Pepper plugin.
+    FORMAT_RGBA,  // Used for Android's Bitmap class.
+  };
+
   // Accepts a buffer to be painted to the screen. The buffer's dimensions and
   // relative position within the frame are specified by |clip_area|. Only
   // pixels falling within |region| and the current clipping area are painted.
@@ -41,6 +48,9 @@ class FrameConsumer {
   virtual void SetSourceSize(const webrtc::DesktopSize& source_size,
                              const webrtc::DesktopVector& dpi) = 0;
 
+  // Returns the preferred pixel encoding for the platform.
+  virtual PixelFormat GetPixelFormat() = 0;
+
  protected:
   FrameConsumer() {}
   virtual ~FrameConsumer() {}
diff --git a/remoting/client/frame_consumer_proxy.cc b/remoting/client/frame_consumer_proxy.cc
index 4c7d79f..a1b3895 100644
--- a/remoting/client/frame_consumer_proxy.cc
+++ b/remoting/client/frame_consumer_proxy.cc
@@ -14,8 +14,11 @@
 namespace remoting {
 
 FrameConsumerProxy::FrameConsumerProxy(
-    scoped_refptr<base::SingleThreadTaskRunner> task_runner)
-    : task_runner_(task_runner) {
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner,
+    const base::WeakPtr<FrameConsumer>& frame_consumer)
+    : frame_consumer_(frame_consumer),
+      task_runner_(task_runner) {
+  pixel_format_ = frame_consumer_->GetPixelFormat();
 }
 
 void FrameConsumerProxy::ApplyBuffer(const webrtc::DesktopSize& view_size,
@@ -57,11 +60,8 @@ void FrameConsumerProxy::SetSourceSize(
     frame_consumer_->SetSourceSize(source_size, source_dpi);
 }
 
-void FrameConsumerProxy::Attach(
-    const base::WeakPtr<FrameConsumer>& frame_consumer) {
-  DCHECK(task_runner_->BelongsToCurrentThread());
-  DCHECK(frame_consumer_.get() == NULL);
-  frame_consumer_ = frame_consumer;
+FrameConsumer::PixelFormat FrameConsumerProxy::GetPixelFormat() {
+  return pixel_format_;
 }
 
 FrameConsumerProxy::~FrameConsumerProxy() {
diff --git a/remoting/client/frame_consumer_proxy.h b/remoting/client/frame_consumer_proxy.h
index 21e8fe4..f32a313 100644
--- a/remoting/client/frame_consumer_proxy.h
+++ b/remoting/client/frame_consumer_proxy.h
@@ -26,7 +26,8 @@ class FrameConsumerProxy
  public:
   // Constructs a proxy for |frame_consumer| which will trampoline invocations
   // to |frame_consumer_message_loop|.
-  FrameConsumerProxy(scoped_refptr<base::SingleThreadTaskRunner> task_runner);
+  FrameConsumerProxy(scoped_refptr<base::SingleThreadTaskRunner> task_runner,
+                     const base::WeakPtr<FrameConsumer>& frame_consumer);
 
   // FrameConsumer implementation.
   virtual void ApplyBuffer(const webrtc::DesktopSize& view_size,
@@ -36,10 +37,7 @@ class FrameConsumerProxy
   virtual void ReturnBuffer(webrtc::DesktopFrame* buffer) OVERRIDE;
   virtual void SetSourceSize(const webrtc::DesktopSize& source_size,
                              const webrtc::DesktopVector& dpi) OVERRIDE;
-
-  // Attaches to |frame_consumer_|.
-  // This must only be called from |frame_consumer_message_loop_|.
-  void Attach(const base::WeakPtr<FrameConsumer>& frame_consumer);
+  virtual PixelFormat GetPixelFormat() OVERRIDE;
 
  private:
   friend class base::RefCountedThreadSafe<FrameConsumerProxy>;
@@ -48,6 +46,8 @@ class FrameConsumerProxy
   base::WeakPtr<FrameConsumer> frame_consumer_;
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
 
+  PixelFormat pixel_format_;
+
   DISALLOW_COPY_AND_ASSIGN(FrameConsumerProxy);
 };
 
diff --git a/remoting/client/jni/chromoting_jni_instance.cc b/remoting/client/jni/chromoting_jni_instance.cc
index 84d94d6..f819a5f 100644
--- a/remoting/client/jni/chromoting_jni_instance.cc
+++ b/remoting/client/jni/chromoting_jni_instance.cc
@@ -228,11 +228,11 @@ void ChromotingJniInstance::SetCursorShape(
 void ChromotingJniInstance::ConnectToHostOnDisplayThread() {
   DCHECK(jni_runtime_->display_task_runner()->BelongsToCurrentThread());
 
-  frame_consumer_ = new FrameConsumerProxy(jni_runtime_->display_task_runner());
   view_.reset(new JniFrameConsumer(jni_runtime_));
   view_weak_factory_.reset(new base::WeakPtrFactory<JniFrameConsumer>(
       view_.get()));
-  frame_consumer_->Attach(view_weak_factory_->GetWeakPtr());
+  frame_consumer_ = new FrameConsumerProxy(jni_runtime_->display_task_runner(),
+                                           view_weak_factory_->GetWeakPtr());
 
   jni_runtime_->network_task_runner()->PostTask(
       FROM_HERE,
diff --git a/remoting/client/jni/jni_frame_consumer.cc b/remoting/client/jni/jni_frame_consumer.cc
index 3b4e763..36a8184 100644
--- a/remoting/client/jni/jni_frame_consumer.cc
+++ b/remoting/client/jni/jni_frame_consumer.cc
@@ -109,6 +109,10 @@ void JniFrameConsumer::SetSourceSize(const webrtc::DesktopSize& source_size,
       &JniFrameConsumer::AllocateBuffer, base::Unretained(this)));
 }
 
+FrameConsumer::PixelFormat JniFrameConsumer::GetPixelFormat() {
+  return FORMAT_RGBA;
+}
+
 void JniFrameConsumer::AllocateBuffer() {
   // Only do anything if we're not being destructed.
   if (!in_dtor_) {
diff --git a/remoting/client/jni/jni_frame_consumer.h b/remoting/client/jni/jni_frame_consumer.h
index 14155a1..1509720 100644
--- a/remoting/client/jni/jni_frame_consumer.h
+++ b/remoting/client/jni/jni_frame_consumer.h
@@ -37,6 +37,7 @@ class JniFrameConsumer : public FrameConsumer {
   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:
   // Variables are to be used from the display thread.
diff --git a/remoting/client/plugin/chromoting_instance.cc b/remoting/client/plugin/chromoting_instance.cc
index 6cecbaa..fd92f5f 100644
--- a/remoting/client/plugin/chromoting_instance.cc
+++ b/remoting/client/plugin/chromoting_instance.cc
@@ -603,10 +603,16 @@ void ChromotingInstance::ConnectWithConfig(const ClientConfig& config,
 
   jingle_glue::JingleThreadWrapper::EnsureForCurrentMessageLoop();
 
+
+  view_.reset(new PepperView(this, &context_));
+  view_weak_factory_.reset(
+      new base::WeakPtrFactory<FrameConsumer>(view_.get()));
+
   // RectangleUpdateDecoder runs on a separate thread so for now we wrap
   // PepperView with a ref-counted proxy object.
   scoped_refptr<FrameConsumerProxy> consumer_proxy =
-      new FrameConsumerProxy(plugin_task_runner_);
+      new FrameConsumerProxy(plugin_task_runner_,
+                             view_weak_factory_->GetWeakPtr());
 
   host_connection_.reset(new protocol::ConnectionToHost(true));
   scoped_ptr<AudioPlayer> audio_player(new PepperAudioPlayer(this));
@@ -614,10 +620,8 @@ void ChromotingInstance::ConnectWithConfig(const ClientConfig& config,
                                      host_connection_.get(), this,
                                      consumer_proxy, audio_player.Pass()));
 
-  view_.reset(new PepperView(this, &context_, client_->GetFrameProducer()));
-  view_weak_factory_.reset(
-      new base::WeakPtrFactory<FrameConsumer>(view_.get()));
-  consumer_proxy->Attach(view_weak_factory_->GetWeakPtr());
+  view_->Initialize(client_->GetFrameProducer());
+
   if (!plugin_view_.is_null()) {
     view_->SetView(plugin_view_);
   }
diff --git a/remoting/client/plugin/pepper_view.cc b/remoting/client/plugin/pepper_view.cc
index a0f6ac4..16a66a0 100644
--- a/remoting/client/plugin/pepper_view.cc
+++ b/remoting/client/plugin/pepper_view.cc
@@ -61,11 +61,10 @@ const size_t kMaxPendingBuffersCount = 2;
 }  // namespace
 
 PepperView::PepperView(ChromotingInstance* instance,
-                       ClientContext* context,
-                       FrameProducer* producer)
+                       ClientContext* context)
   : instance_(instance),
     context_(context),
-    producer_(producer),
+    producer_(NULL),
     merge_buffer_(NULL),
     dips_to_device_scale_(1.0f),
     dips_to_view_scale_(1.0f),
@@ -73,7 +72,6 @@ PepperView::PepperView(ChromotingInstance* instance,
     is_initialized_(false),
     frame_received_(false),
     callback_factory_(this) {
-  InitiateDrawing();
 }
 
 PepperView::~PepperView() {
@@ -91,6 +89,15 @@ PepperView::~PepperView() {
   }
 }
 
+void PepperView::Initialize(FrameProducer* producer) {
+  producer_ = producer;
+  webrtc::DesktopFrame* buffer = AllocateBuffer();
+  while (buffer) {
+    producer_->DrawBuffer(buffer);
+    buffer = AllocateBuffer();
+  }
+}
+
 void PepperView::SetView(const pp::View& view) {
   bool view_changed = false;
 
@@ -150,7 +157,7 @@ void PepperView::SetView(const pp::View& view) {
 
   if (view_changed) {
     producer_->SetOutputSizeAndClip(view_size_, clip_area_);
-    InitiateDrawing();
+    Initialize(producer_);
   }
 }
 
@@ -171,7 +178,7 @@ void PepperView::ApplyBuffer(const webrtc::DesktopSize& view_size,
   // the properly scaled data.
   if (!view_size_.equals(view_size)) {
     FreeBuffer(buffer);
-    InitiateDrawing();
+    Initialize(producer_);
   } else {
     FlushBuffer(clip_area, buffer, region);
   }
@@ -187,7 +194,7 @@ void PepperView::ReturnBuffer(webrtc::DesktopFrame* buffer) {
     producer_->DrawBuffer(buffer);
   } else {
     FreeBuffer(buffer);
-    InitiateDrawing();
+    Initialize(producer_);
   }
 }
 
@@ -205,6 +212,10 @@ void PepperView::SetSourceSize(const webrtc::DesktopSize& source_size,
   instance_->SetDesktopSize(source_size, source_dpi);
 }
 
+FrameConsumer::PixelFormat PepperView::GetPixelFormat() {
+  return FORMAT_BGRA;
+}
+
 webrtc::DesktopFrame* PepperView::AllocateBuffer() {
   if (buffers_.size() >= kMaxPendingBuffersCount)
     return NULL;
@@ -235,14 +246,6 @@ void PepperView::FreeBuffer(webrtc::DesktopFrame* buffer) {
   delete buffer;
 }
 
-void PepperView::InitiateDrawing() {
-  webrtc::DesktopFrame* buffer = AllocateBuffer();
-  while (buffer) {
-    producer_->DrawBuffer(buffer);
-    buffer = AllocateBuffer();
-  }
-}
-
 void PepperView::FlushBuffer(const webrtc::DesktopRect& clip_area,
                              webrtc::DesktopFrame* buffer,
                              const webrtc::DesktopRegion& region) {
diff --git a/remoting/client/plugin/pepper_view.h b/remoting/client/plugin/pepper_view.h
index 4027b620..2b4e69b 100644
--- a/remoting/client/plugin/pepper_view.h
+++ b/remoting/client/plugin/pepper_view.h
@@ -35,13 +35,15 @@ class FrameProducer;
 
 class PepperView : public FrameConsumer {
  public:
-  // Constructs a PepperView for the |instance|. The |instance|, |context|
-  // and |producer| must outlive this class.
-  PepperView(ChromotingInstance* instance,
-             ClientContext* context,
-             FrameProducer* producer);
+  // Constructs a PepperView for the |instance|. The |instance| and |context|
+  // must outlive this class.
+  PepperView(ChromotingInstance* instance, ClientContext* context);
   virtual ~PepperView();
 
+  // Allocates buffers and passes them to the FrameProducer to render into until
+  // the maximum number of buffers are in-flight.
+  void Initialize(FrameProducer* producer);
+
   // FrameConsumer implementation.
   virtual void ApplyBuffer(const webrtc::DesktopSize& view_size,
                            const webrtc::DesktopRect& clip_area,
@@ -50,6 +52,7 @@ class PepperView : public FrameConsumer {
   virtual void ReturnBuffer(webrtc::DesktopFrame* buffer) OVERRIDE;
   virtual void SetSourceSize(const webrtc::DesktopSize& source_size,
                              const webrtc::DesktopVector& dpi) OVERRIDE;
+  virtual PixelFormat GetPixelFormat() OVERRIDE;
 
   // Updates the PepperView's size & clipping area, taking into account the
   // DIP-to-device scale factor.
@@ -74,10 +77,6 @@ class PepperView : public FrameConsumer {
   // Frees a frame buffer previously allocated by AllocateBuffer.
   void FreeBuffer(webrtc::DesktopFrame* buffer);
 
-  // Allocates buffers and passes them to the FrameProducer to render into until
-  // the maximum number of buffers are in-flight.
-  void InitiateDrawing();
-
   // Renders the parts of |buffer| identified by |region| to the view.  If the
   // clip area of the view has changed since the buffer was generated then
   // FrameProducer is supplied the missed parts of |region|.  The FrameProducer
diff --git a/remoting/client/rectangle_update_decoder.cc b/remoting/client/rectangle_update_decoder.cc
index af123e8..3ff2be1 100644
--- a/remoting/client/rectangle_update_decoder.cc
+++ b/remoting/client/rectangle_update_decoder.cc
@@ -16,6 +16,7 @@
 #include "remoting/codec/video_decoder_vp8.h"
 #include "remoting/client/frame_consumer.h"
 #include "remoting/protocol/session_config.h"
+#include "third_party/libyuv/include/libyuv/convert_argb.h"
 #include "third_party/webrtc/modules/desktop_capture/desktop_frame.h"
 
 using base::Passed;
@@ -24,6 +25,55 @@ using remoting::protocol::SessionConfig;
 
 namespace remoting {
 
+// This class wraps a VideoDecoder and byte-swaps the pixels for compatibility
+// with the android.graphics.Bitmap class.
+// TODO(lambroslambrou): Refactor so that the VideoDecoder produces data
+// in the right byte-order, instead of swapping it here.
+class RgbToBgrVideoDecoderFilter : public VideoDecoder {
+ public:
+  RgbToBgrVideoDecoderFilter(scoped_ptr<VideoDecoder> parent)
+      : parent_(parent.Pass()) {
+  }
+
+  virtual void Initialize(const webrtc::DesktopSize& screen_size) OVERRIDE {
+    parent_->Initialize(screen_size);
+  }
+
+  virtual bool DecodePacket(const VideoPacket& packet) OVERRIDE {
+    return parent_->DecodePacket(packet);
+  }
+
+  virtual void Invalidate(const webrtc::DesktopSize& view_size,
+                          const webrtc::DesktopRegion& region) OVERRIDE {
+    return parent_->Invalidate(view_size, region);
+  }
+
+  virtual void RenderFrame(const webrtc::DesktopSize& view_size,
+                           const webrtc::DesktopRect& clip_area,
+                           uint8* image_buffer,
+                           int image_stride,
+                           webrtc::DesktopRegion* output_region) OVERRIDE {
+    parent_->RenderFrame(view_size, clip_area, image_buffer, image_stride,
+                         output_region);
+
+    for (webrtc::DesktopRegion::Iterator i(*output_region); !i.IsAtEnd();
+         i.Advance()) {
+      webrtc::DesktopRect rect = i.rect();
+      uint8* pixels = image_buffer + (rect.top() * image_stride) +
+        (rect.left() * kBytesPerPixel);
+      libyuv::ABGRToARGB(pixels, image_stride, pixels, image_stride,
+                         rect.width(), rect.height());
+    }
+  }
+
+  virtual const webrtc::DesktopRegion* GetImageShape() OVERRIDE {
+    return parent_->GetImageShape();
+  }
+
+ private:
+  scoped_ptr<VideoDecoder> parent_;
+};
+
 RectangleUpdateDecoder::RectangleUpdateDecoder(
     scoped_refptr<base::SingleThreadTaskRunner> main_task_runner,
     scoped_refptr<base::SingleThreadTaskRunner> decode_task_runner,
@@ -39,6 +89,13 @@ RectangleUpdateDecoder::~RectangleUpdateDecoder() {
 }
 
 void RectangleUpdateDecoder::Initialize(const SessionConfig& config) {
+  if (!decode_task_runner_->BelongsToCurrentThread()) {
+    decode_task_runner_->PostTask(
+        FROM_HERE, base::Bind(&RectangleUpdateDecoder::Initialize, this,
+                              config));
+    return;
+  }
+
   // Initialize decoder based on the selected codec.
   ChannelConfig::Codec codec = config.video_config().codec;
   if (codec == ChannelConfig::CODEC_VERBATIM) {
@@ -48,6 +105,12 @@ void RectangleUpdateDecoder::Initialize(const SessionConfig& config) {
   } else {
     NOTREACHED() << "Invalid Encoding found: " << codec;
   }
+
+  if (consumer_->GetPixelFormat() == FrameConsumer::FORMAT_RGBA) {
+    scoped_ptr<VideoDecoder> wrapper(
+        new RgbToBgrVideoDecoderFilter(decoder_.Pass()));
+    decoder_ = wrapper.Pass();
+  }
 }
 
 void RectangleUpdateDecoder::DecodePacket(scoped_ptr<VideoPacket> packet,
diff --git a/remoting/remoting.gyp b/remoting/remoting.gyp
index 60e0325..ed450af 100644
--- a/remoting/remoting.gyp
+++ b/remoting/remoting.gyp
@@ -2542,6 +2542,7 @@
         'remoting_base',
         'remoting_jingle_glue',
         'remoting_protocol',
+        '../third_party/libyuv/libyuv.gyp:libyuv',
         '../third_party/webrtc/modules/modules.gyp:desktop_capture',
       ],
       'sources': [