Fix bugs in HardwareVideoDecoder reinitialization.

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

 /*
  *  Copyright 2017 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 106 matching lines @@
       this.rotation = rotation;
       this.presentationTimestampUs = presentationTimestampUs;
       this.decodeTimeMs = decodeTimeMs;
     }
   }
 
   // Metadata for the last frame rendered to the texture.  Only accessed on the texture helper's
   // 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;
 
   private MediaCodec codec = null;
 
   HardwareVideoDecoder(
       String codecName, VideoCodecType codecType, int colorFormat, EglBase.Context sharedContext) {
     if (!isSupportedColorFormat(colorFormat)) {
       throw new IllegalArgumentException("Unsupported color format: " + colorFormat);
     }
     this.codecName = codecName;
     this.codecType = codecType;
     this.colorFormat = colorFormat;
     this.sharedContext = sharedContext;
     this.frameInfos = new LinkedBlockingDeque<>();
   }
 
   @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;
     }
 
     // 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;
 
     stride = width;
     sliceHeight = height;
     hasDecodedFirstFrame = false;
     keyFrameRequired = true;
 
     try {
       codec = MediaCodec.createByCodecName(codecName);
     } catch (IOException | IllegalArgumentException e) {
       Logging.e(TAG, "Cannot create media decoder " + codecName);
       return VideoCodecStatus.ERROR;
     }
     try {
       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();
     } catch (IllegalStateException e) {
       Logging.e(TAG, "initDecode failed", e);
       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) {
       Logging.e(TAG, "decode() - no input data");
       return VideoCodecStatus.ERR_PARAMETER;
     }
 
     int size = frame.buffer.remaining();
     if (size == 0) {
       Logging.e(TAG, "decode() - input buffer empty");
@@ skipping 79 matching lines @@
   @Override
   public String getImplementationName() {
     return "HardwareVideoDecoder: " + codecName;
   }
 
   @Override
   public VideoCodecStatus release() {
     // 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;
     }
     try {
       // The outputThread actually stops and releases the codec once running is false.
       running = false;
       if (!ThreadUtils.joinUninterruptibly(outputThread, MEDIA_CODEC_RELEASE_TIMEOUT_MS)) {
         // Log an exception to capture the stack trace and turn it into a TIMEOUT error.
         Logging.e(TAG, "Media decoder release timeout", new RuntimeException());
         return VideoCodecStatus.TIMEOUT;
       }
       if (shutdownException != null) {
         // Log the exception and turn it into an error.  Wrap the exception in a new exception to
         // capture both the output thread's stack trace and this thread's stack trace.
         Logging.e(TAG, "Media decoder release error", new RuntimeException(shutdownException));
         shutdownException = null;
         return VideoCodecStatus.ERROR;
       }
     } 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;
   }
 
   private VideoCodecStatus reinitDecode(int newWidth, int newHeight) {
     decoderThreadChecker.checkIsOnValidThread();
     VideoCodecStatus status = release();
     if (status != VideoCodecStatus.OK) {
       return status;
     }
-    return initDecodeInternal(newWidth, newHeight, callback);
+    return initDecodeInternal(newWidth, newHeight);
   }
 
   private Thread createOutputThread() {
     return new Thread("HardwareVideoDecoder.outputThread") {
       @Override
       public void run() {
         outputThreadChecker = new ThreadChecker();
         while (running) {
           deliverDecodedFrame();
         }
@@ skipping 62 matching lines @@
   }
 
   @Override
   public void onTextureFrameAvailable(int oesTextureId, float[] transformMatrix, long timestampNs) {
     VideoFrame.TextureBuffer oesBuffer = surfaceTextureHelper.createTextureBuffer(
         renderedTextureMetadata.width, renderedTextureMetadata.height,
         RendererCommon.convertMatrixToAndroidGraphicsMatrix(transformMatrix));
 
     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();
   }
 
   private void deliverByteFrame(
       int result, MediaCodec.BufferInfo info, int rotation, Integer decodeTimeMs) {
     // Load dimensions from shared memory under the dimension lock.
     int width, height, stride, sliceHeight;
     synchronized (dimensionLock) {
       width = this.width;
       height = this.height;
@@ skipping 203 matching lines @@
       Logging.e(TAG, "Media decoder stop failed", e);
     }
     try {
       codec.release();
     } catch (Exception e) {
       Logging.e(TAG, "Media decoder release failed", e);
       // Propagate exceptions caught during release back to the main thread.
       shutdownException = e;
     }
     codec = null;
-    callback = null;
+    synchronized (callbackLock) {
+      callback = null;
+    }
     outputThread = null;
     frameInfos.clear();
     Logging.d(TAG, "Release on output thread done");
   }
 
   private void waitOutputBuffersReleasedOnOutputThread() {
     outputThreadChecker.checkIsOnValidThread();
     synchronized (activeOutputBuffersLock) {
       while (activeOutputBuffers > 0) {
         Logging.d(TAG, "Waiting for all frames to be released.");
@@ skipping 15 matching lines @@
 
   private boolean isSupportedColorFormat(int colorFormat) {
     for (int supported : MediaCodecUtils.DECODER_COLOR_FORMATS) {
       if (supported == colorFormat) {
         return true;
       }
     }
     return false;
   }
 }