Better handling of error condition in MediaCodecVideoEncoder. BUG=b/36082499 Review-Url: https://codereview.webrtc.org/2748123002 Cr-Commit-Position: refs/heads/master@{#17246}
diff --git a/webrtc/base/java/src/org/webrtc/ThreadUtils.java b/webrtc/base/java/src/org/webrtc/ThreadUtils.java index efe1fbd..df2c2d0 100644 --- a/webrtc/base/java/src/org/webrtc/ThreadUtils.java +++ b/webrtc/base/java/src/org/webrtc/ThreadUtils.java
@@ -13,7 +13,6 @@ 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 @@ }); } - 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);
diff --git a/webrtc/sdk/android/api/org/webrtc/MediaCodecVideoEncoder.java b/webrtc/sdk/android/api/org/webrtc/MediaCodecVideoEncoder.java index 0c47c38..b726860 100644 --- a/webrtc/sdk/android/api/org/webrtc/MediaCodecVideoEncoder.java +++ b/webrtc/sdk/android/api/org/webrtc/MediaCodecVideoEncoder.java
@@ -454,6 +454,7 @@ 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 @@ } catch (IllegalStateException e) { Logging.e(TAG, "initEncode failed", e); + release(); return false; } return true; @@ -544,36 +546,47 @@ Logging.d(TAG, "Java releaseEncoder"); checkOnMediaCodecThread(); - // Run Mediacodec stop() and release() on separate thread since sometime - // Mediacodec.stop() may hang. - final CountDownLatch releaseDone = new CountDownLatch(1); + class CaughtException { + Exception e; + } + final CaughtException caughtException = new CaughtException(); + boolean stopHung = false; - Runnable runMediaCodecRelease = new Runnable() { - @Override - public void run() { - try { + if (mediaCodec != null) { + // Run Mediacodec stop() and release() on separate thread since sometime + // Mediacodec.stop() may hang. + final CountDownLatch releaseDone = new CountDownLatch(1); + + Runnable runMediaCodecRelease = new Runnable() { + @Override + public void run() { Logging.d(TAG, "Java releaseEncoder on release thread"); - mediaCodec.stop(); - mediaCodec.release(); + try { + 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)) { - Logging.e(TAG, "Media encoder release timeout"); - codecErrors++; - if (errorCallback != null) { - Logging.e(TAG, "Invoke codec error callback. Errors: " + codecErrors); - errorCallback.onMediaCodecVideoEncoderCriticalError(codecErrors); + releaseDone.countDown(); + } + }; + new Thread(runMediaCodecRelease).start(); + + 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 @@ 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"); }
diff --git a/webrtc/sdk/android/src/jni/androidmediaencoder_jni.cc b/webrtc/sdk/android/src/jni/androidmediaencoder_jni.cc index 8fc9e7b..92406ec 100644 --- a/webrtc/sdk/android/src/jni/androidmediaencoder_jni.cc +++ b/webrtc/sdk/android/src/jni/androidmediaencoder_jni.cc
@@ -456,6 +456,11 @@ // 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 @@ 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 @@ #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 @@ 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 @@ inited_ = false; } use_surface_ = false; - encode_task_.reset(nullptr); - weak_factory_.reset(nullptr); ALOGD << "EncoderRelease done."; return WEBRTC_VIDEO_CODEC_OK; }