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

remoting/android/java/src/org/chromium/chromoting/jni/JniInterface.java

 // 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.
 
 package org.chromium.chromoting.jni;
 
 import android.app.Activity;
 import android.app.AlertDialog;
 import android.app.ProgressDialog;
 import android.content.Context;
@@ skipping 92 matching lines @@
 
     /** Severs the connection and cleans up. */
     public static void disconnectFromHost() {
         synchronized(JniInterface.class) {
             if (!sLoaded || !sConnected) return;
 
             if (sProgressIndicator != null) {
                 sProgressIndicator.dismiss();
                 sProgressIndicator = null;
             }
+
+            // Drop the reference to free the Bitmap for GC.
+            sFrameBitmap = null;
         }
 
         disconnectNative();
         sSuccessCallback = null;
         sConnected = false;
     }
 
     /** Performs the native portion of the connection. */
     private static native void connectNative(String username, String authToken, String hostJid,
             String hostId, String hostPubkey, String pairId, String pairSecret);
 
     /** Performs the native portion of the cleanup. */
     private static native void disconnectNative();
 
     /** Position of cursor hotspot within cursor image. */
     public static Point getCursorHotspot() { return sCursorHotspot; }
 
     /** Returns the current cursor shape. */
     public static Bitmap getCursorBitmap() { return sCursorBitmap; }
 
     /*
      * Entry points *from* the native code.
      */
     /** Callback to signal whenever we need to redraw. */
     private static Runnable sRedrawCallback = null;
 
-    /** Screen width of the video feed. */
-    private static int sWidth = 0;
-
-    /** 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;
+    /** Bitmap holding a copy of the latest video frame. */
+    private static Bitmap sFrameBitmap = null;
 
     /** Position of cursor hot-spot. */
     private static Point sCursorHotspot = new Point();
 
     /** Bitmap holding the cursor shape. */
     private static Bitmap sCursorBitmap = null;
 
     /** Reports whenever the connection status changes. */
     private static void reportConnectionStatus(int state, int error) {
         if (state < SUCCESSFUL_CONNECTION && error == 0) {
@@ skipping 134 matching lines @@
         return true;
     }
 
     /** Performs the redrawing callback. This is a no-op if the window isn't visible. */
     private static void redrawGraphicsInternal() {
         if (sRedrawCallback != null)
             sRedrawCallback.run();
     }
 
     /**
-     * Obtains the image buffer.
-     * This should not be called from the UI thread. (We prefer the native graphics thread.)
+     * Returns a bitmap of the latest video frame. Called on the native graphics thread when
+     * DesktopView is repainted.
      */
-    public static Bitmap retrieveVideoFrame() {
+    public static Bitmap getVideoFrame() {
         if (Looper.myLooper() == Looper.getMainLooper()) {
             Log.w("jniiface", "Canvas being redrawn on UI thread");
         }
 
         if (!sConnected) {
             return null;
         }
 
-        // 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);
-            }
-        }
-
-        sBuffer.rewind();
-        sBitmap.copyPixelsFromBuffer(sBuffer);
-        return sBitmap;
+        return sFrameBitmap;
     }
 
     /**
+     * Sets a new video frame. Called from native code on the native graphics thread when a new
+     * frame is allocated.
+     */
+    private static void setVideoFrame(Bitmap bitmap) {
+        if (Looper.myLooper() == Looper.getMainLooper()) {
+            Log.w("jniiface", "Video frame updated on UI thread");
+        }
+
+        synchronized (JniInterface.class) {

[Yaron, 2013/10/09 06:42:30]

I believe this will trigger a findbugs error. We discouraging synchronizing on a
class object since they're effectively global and anyone can lock on it leading
to possible deadlocks. Instead, use a private member variable and lock on that.

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

There are already lots of these in this file. I've fixed this one, but fixing
all the others is probably beyond the scope of this CL. Filed
https://code.google.com/p/chromium/issues/detail?id=305770

+            sFrameBitmap = bitmap;
+        }
+    }
+
+    /**
+     * Creates a new Bitmap to hold video frame pixels. Called by native code which stores a global
+     * reference to the Bitmap and writes the decoded frame pixels to it.
+     */
+    private static Bitmap newBitmap(int width, int height) {
+        return Bitmap.createBitmap(width, height, Bitmap.Config.ARGB_8888);
+    }
+
+    /**
      * Updates the cursor shape. This is called from native code on the graphics thread when
      * receiving a new cursor shape from the host.
      */
     public static void updateCursorShape(int width, int height, int hotspotX, int hotspotY,
                                          ByteBuffer buffer) {
         sCursorHotspot = new Point(hotspotX, hotspotY);
 
         int[] data = new int[width * height];
         buffer.order(ByteOrder.LITTLE_ENDIAN);
         buffer.asIntBuffer().get(data, 0, data.length);
@@ skipping 23 matching lines @@
 
     /** Schedules a redraw on the native graphics thread. */
     private static native void scheduleRedrawNative();
 
     /** Passes mouse information to the native handling code. */
     private static native void mouseActionNative(int x, int y, int whichButton, boolean buttonDown);
 
     /** Passes key press information to the native handling code. */
     private static native void keyboardActionNative(int keyCode, boolean keyDown);
 }