Fix bugs in HardwareVideoDecoder reinitialization.

webrtc/sdk/android/src/java/org/webrtc/HardwareVideoDecoder.java

Index: webrtc/sdk/android/src/java/org/webrtc/HardwareVideoDecoder.java
diff --git a/webrtc/sdk/android/src/java/org/webrtc/HardwareVideoDecoder.java b/webrtc/sdk/android/src/java/org/webrtc/HardwareVideoDecoder.java
index ffa78ad6bbd09e65343f5d70f2428c77f3ea9fc4..5f8a101ac2f73885090e0d190ea41a59697f3471 100644
--- a/webrtc/sdk/android/src/java/org/webrtc/HardwareVideoDecoder.java
+++ b/webrtc/sdk/android/src/java/org/webrtc/HardwareVideoDecoder.java
@@ -124,6 +124,7 @@ class HardwareVideoDecoder
   // thread.
   private DecodedTextureMetadata renderedTextureMetadata;
 
+  private final Object callbackLock = new Object();

[magjed_webrtc, 2017/08/24 10:52:24]

Please get rid of the lock and use SurfaceTextureHelper to synchronize instead,
e.g. stopListening/startListening has the guarantee you need.

[sakal, 2017/08/24 11:24:00]

Done.

   // Decoding proceeds asynchronously.  This callback returns decoded frames to the caller.
   private Callback callback;
 
@@ -144,11 +145,21 @@ class HardwareVideoDecoder
   @Override
   public VideoCodecStatus initDecode(Settings settings, Callback callback) {
     this.decoderThreadChecker = new ThreadChecker();
-    return initDecodeInternal(settings.width, settings.height, callback);
+
+    synchronized (callbackLock) {
+      this.callback = callback;
+    }
+    if (sharedContext != null) {
+      surfaceTextureHelper = SurfaceTextureHelper.create("decoder-texture-thread", sharedContext);
+      surface = new Surface(surfaceTextureHelper.getSurfaceTexture());
+      surfaceTextureHelper.startListening(this);
+    }
+    return initDecodeInternal(settings.width, settings.height);
   }
 
-  private VideoCodecStatus initDecodeInternal(int width, int height, Callback callback) {
+  private VideoCodecStatus initDecodeInternal(int width, int height) {
     decoderThreadChecker.checkIsOnValidThread();
+    Logging.d(TAG, "initDecodeInternal");
     if (outputThread != null) {
       Logging.e(TAG, "initDecodeInternal called while the codec is already running");
       return VideoCodecStatus.ERROR;
@@ -156,7 +167,6 @@ class HardwareVideoDecoder
 
     // Note:  it is not necessary to initialize dimensions under the lock, since the output thread
     // is not running.
-    this.callback = callback;
     this.width = width;
     this.height = height;
 
@@ -175,10 +185,6 @@ class HardwareVideoDecoder
       MediaFormat format = MediaFormat.createVideoFormat(codecType.mimeType(), width, height);
       if (sharedContext == null) {
         format.setInteger(MediaFormat.KEY_COLOR_FORMAT, colorFormat);
-      } else {
-        surfaceTextureHelper = SurfaceTextureHelper.create("decoder-texture-thread", sharedContext);
-        surface = new Surface(surfaceTextureHelper.getSurfaceTexture());
-        surfaceTextureHelper.startListening(this);
       }
       codec.configure(format, surface, null, 0);
       codec.start();
@@ -187,19 +193,22 @@ class HardwareVideoDecoder
       release();
       return VideoCodecStatus.ERROR;
     }
-
     running = true;
     outputThread = createOutputThread();
     outputThread.start();
 
+    Logging.d(TAG, "initDecodeInternal done");
     return VideoCodecStatus.OK;
   }
 
   @Override
   public VideoCodecStatus decode(EncodedImage frame, DecodeInfo info) {
     decoderThreadChecker.checkIsOnValidThread();
-    if (codec == null || callback == null) {
-      return VideoCodecStatus.UNINITIALIZED;
+    synchronized (callbackLock) {
+      if (codec == null || callback == null) {
+        Logging.d(TAG, "decode uninitalized, codec: " + codec + ", callback: " + callback);
+        return VideoCodecStatus.UNINITIALIZED;
+      }
     }
 
     if (frame.buffer == null) {
@@ -299,6 +308,25 @@ class HardwareVideoDecoder
     // TODO(sakal): This is not called on the correct thread but is still called synchronously.
     // Re-enable the check once this is called on the correct thread.
     // decoderThreadChecker.checkIsOnValidThread();
+    Logging.d(TAG, "release");
+    try {
+      return releaseInternal();
+    } finally {
+      synchronized (callbackLock) {
+        callback = null;
+      }
+      frameInfos.clear();
+      if (surface != null) {
+        surface.release();
+        surface = null;
+        surfaceTextureHelper.stopListening();
+        surfaceTextureHelper.dispose();
+        surfaceTextureHelper = null;
+      }
+    }
+  }
+
+  private VideoCodecStatus releaseInternal() {
     if (!running) {
       Logging.d(TAG, "release: Decoder is not running.");
       return VideoCodecStatus.OK;
@@ -320,16 +348,7 @@ class HardwareVideoDecoder
       }
     } finally {
       codec = null;
-      callback = null;
       outputThread = null;
-      frameInfos.clear();
-      if (surface != null) {
-        surface.release();
-        surface = null;
-        surfaceTextureHelper.stopListening();
-        surfaceTextureHelper.dispose();
-        surfaceTextureHelper = null;
-      }
     }
     return VideoCodecStatus.OK;
   }
@@ -340,7 +359,7 @@ class HardwareVideoDecoder
     if (status != VideoCodecStatus.OK) {
       return status;
     }
-    return initDecodeInternal(newWidth, newHeight, callback);
+    return initDecodeInternal(newWidth, newHeight);
   }
 
   private Thread createOutputThread() {
@@ -423,7 +442,11 @@ class HardwareVideoDecoder
 
     VideoFrame frame = new VideoFrame(oesBuffer, renderedTextureMetadata.rotation,
         renderedTextureMetadata.presentationTimestampUs * 1000);
-    callback.onDecodedFrame(frame, renderedTextureMetadata.decodeTimeMs, null /* qp */);
+    synchronized (callbackLock) {
+      if (callback != null) {
+        callback.onDecodedFrame(frame, renderedTextureMetadata.decodeTimeMs, null /* qp */);
+      }
+    }
     frame.release();
   }
 
@@ -647,7 +670,9 @@ class HardwareVideoDecoder
       shutdownException = e;
     }
     codec = null;
-    callback = null;
+    synchronized (callbackLock) {
+      callback = null;
+    }
     outputThread = null;
     frameInfos.clear();
     Logging.d(TAG, "Release on output thread done");