Better handling of error condition in MediaCodecVideoEncoder.

BUG=b/36082499

Review-Url: https://codereview.webrtc.org/2748123002
Cr-Commit-Position: refs/heads/master@{#17246}

webrtc/base/java/src/org/webrtc/ThreadUtils.java

@@ -13,7 +13,6 @@ package org.webrtc;
 import android.os.Handler;
 import android.os.Looper;
 import android.os.SystemClock;
-
 import java.util.concurrent.Callable;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
@@ -211,7 +210,7 @@ public class ThreadUtils {
     });
   }
 
-  private static StackTraceElement[] concatStackTraces(
+  static StackTraceElement[] concatStackTraces(
       StackTraceElement[] inner, StackTraceElement[] outer) {
     final StackTraceElement[] combined = new StackTraceElement[inner.length + outer.length];
     System.arraycopy(inner, 0, combined, 0, inner.length);

webrtc/sdk/android/api/org/webrtc/MediaCodecVideoEncoder.java

@@ -454,6 +454,7 @@ public class MediaCodecVideoEncoder {
       this.type = type;
       if (mediaCodec == null) {
         Logging.e(TAG, "Can not create media encoder");
+        release();
         return false;
       }
       mediaCodec.configure(format, null, null, MediaCodec.CONFIGURE_FLAG_ENCODE);
@@ -471,6 +472,7 @@ public class MediaCodecVideoEncoder {
 
     } catch (IllegalStateException e) {
       Logging.e(TAG, "initEncode failed", e);
+      release();
       return false;
     }
     return true;
@@ -544,36 +546,47 @@ public class MediaCodecVideoEncoder {
     Logging.d(TAG, "Java releaseEncoder");
     checkOnMediaCodecThread();
 
-    // Run Mediacodec stop() and release() on separate thread since sometime
+    class CaughtException {
-    // Mediacodec.stop() may hang.
+      Exception e;
-    final CountDownLatch releaseDone = new CountDownLatch(1);
+    }
+    final CaughtException caughtException = new CaughtException();
+    boolean stopHung = false;
 
-    Runnable runMediaCodecRelease = new Runnable() {
+    if (mediaCodec != null) {
-      @Override
+      // Run Mediacodec stop() and release() on separate thread since sometime
-      public void run() {
+      // Mediacodec.stop() may hang.
-        try {
+      final CountDownLatch releaseDone = new CountDownLatch(1);
+
+      Runnable runMediaCodecRelease = new Runnable() {
+        @Override
+        public void run() {
           Logging.d(TAG, "Java releaseEncoder on release thread");
-          mediaCodec.stop();
+          try {
-          mediaCodec.release();
+            mediaCodec.stop();
+          } catch (Exception e) {
+            Logging.e(TAG, "Media encoder stop failed", e);
+          }
+          try {
+            mediaCodec.release();
+          } catch (Exception e) {
+            Logging.e(TAG, "Media encoder release failed", e);
+            caughtException.e = e;
+          }
           Logging.d(TAG, "Java releaseEncoder on release thread done");
-        } catch (Exception e) {
-          Logging.e(TAG, "Media encoder release failed", e);
-        }
-        releaseDone.countDown();
-      }
-    };
-    new Thread(runMediaCodecRelease).start();
 
-    if (!ThreadUtils.awaitUninterruptibly(releaseDone, MEDIA_CODEC_RELEASE_TIMEOUT_MS)) {
+          releaseDone.countDown();
-      Logging.e(TAG, "Media encoder release timeout");
+        }
-      codecErrors++;
+      };
-      if (errorCallback != null) {
+      new Thread(runMediaCodecRelease).start();
-        Logging.e(TAG, "Invoke codec error callback. Errors: " + codecErrors);
+
-        errorCallback.onMediaCodecVideoEncoderCriticalError(codecErrors);
+      if (!ThreadUtils.awaitUninterruptibly(releaseDone, MEDIA_CODEC_RELEASE_TIMEOUT_MS)) {
+        Logging.e(TAG, "Media encoder release timeout");
+        stopHung = true;
       }
+
+      mediaCodec = null;
     }
 
-    mediaCodec = null;
     mediaCodecThread = null;
     if (drawer != null) {
       drawer.release();
@@ -588,6 +601,25 @@ public class MediaCodecVideoEncoder {
       inputSurface = null;
     }
     runningInstance = null;
+
+    if (stopHung) {
+      codecErrors++;
+      if (errorCallback != null) {
+        Logging.e(TAG, "Invoke codec error callback. Errors: " + codecErrors);
+        errorCallback.onMediaCodecVideoEncoderCriticalError(codecErrors);
+      }
+      throw new RuntimeException("Media encoder release timeout.");
+    }
+
+    // Re-throw any runtime exception caught inside the other thread. Since this is an invoke, add
+    // stack trace for the waiting thread as well.
+    if (caughtException.e != null) {
+      final RuntimeException runtimeException = new RuntimeException(caughtException.e);
+      runtimeException.setStackTrace(ThreadUtils.concatStackTraces(
+          caughtException.e.getStackTrace(), runtimeException.getStackTrace()));
+      throw runtimeException;
+    }
+
     Logging.d(TAG, "Java releaseEncoder done");
   }
 

webrtc/sdk/android/src/jni/androidmediaencoder_jni.cc

@@ -456,6 +456,11 @@ bool MediaCodecVideoEncoder::EncodeTask::Run() {
   // about it and let the next app-called API method reveal the borkedness.
   encoder_->DeliverPendingOutputs(jni);
 
+  if (!encoder_) {
+    // Encoder can be destroyed in DeliverPendingOutputs.
+    return true;
+  }
+
   // Call log statistics here so it's called even if no frames are being
   // delivered.
   encoder_->LogStatistics(false);
@@ -545,8 +550,6 @@ int32_t MediaCodecVideoEncoder::InitEncodeInternal(int width,
   gof_idx_ = 0;
   last_frame_received_ms_ = -1;
   frames_received_since_last_key_ = kMinKeyFrameInterval;
-  weak_factory_.reset(new rtc::WeakPtrFactory<MediaCodecVideoEncoder>(this));
-  encode_task_.reset(new EncodeTask(weak_factory_->GetWeakPtr()));
 
   // We enforce no extra stride/padding in the format creation step.
   jobject j_video_codec_enum = JavaEnumFromIndexAndClassName(
@@ -620,6 +623,9 @@ int32_t MediaCodecVideoEncoder::InitEncodeInternal(int width,
 #endif
     inited_ = true;
   }
+  weak_factory_.reset(new rtc::WeakPtrFactory<MediaCodecVideoEncoder>(this));
+  encode_task_.reset(new EncodeTask(weak_factory_->GetWeakPtr()));
+
   return WEBRTC_VIDEO_CODEC_OK;
 }
 
@@ -884,6 +890,8 @@ int32_t MediaCodecVideoEncoder::Release() {
   ALOGD << "EncoderRelease: Frames received: " << frames_received_
         << ". Encoded: " << frames_encoded_
         << ". Dropped: " << frames_dropped_media_encoder_;
+  encode_task_.reset(nullptr);
+  weak_factory_.reset(nullptr);
   ScopedLocalRefFrame local_ref_frame(jni);
   for (size_t i = 0; i < input_buffers_.size(); ++i)
     jni->DeleteGlobalRef(input_buffers_[i]);
@@ -901,8 +909,6 @@ int32_t MediaCodecVideoEncoder::Release() {
     inited_ = false;
   }
   use_surface_ = false;
-  encode_task_.reset(nullptr);
-  weak_factory_.reset(nullptr);
   ALOGD << "EncoderRelease done.";
   return WEBRTC_VIDEO_CODEC_OK;
 }