From 5c3da4b6e9bdc787a3c1f25c0ff0c16a4271fd26 Mon Sep 17 00:00:00 2001 From: Alex Glaznev Date: Fri, 30 Oct 2015 15:31:07 -0700 Subject: [PATCH] Call MediaCodec.stop() on separate thread. MediaCodec.stop() call may hang in some rear cases. To avoid application hang this call need to be done on separate thread and possible error reported back to application. Application may elect to continue executing and use another codec instance for encoding/decoding or stop the call and exit. BUG=b/24339249 R=magjed@webrtc.org Review URL: https://codereview.webrtc.org/1425143005 . Cr-Commit-Position: refs/heads/master@{#10467} --- .../java/android/org/webrtc/ThreadUtils.java | 26 +++++++++ .../org/webrtc/MediaCodecVideoDecoder.java | 54 +++++++++++++++--- .../org/webrtc/MediaCodecVideoEncoder.java | 56 ++++++++++++++++--- 3 files changed, 120 insertions(+), 16 deletions(-) diff --git a/talk/app/webrtc/java/android/org/webrtc/ThreadUtils.java b/talk/app/webrtc/java/android/org/webrtc/ThreadUtils.java index 0d8968aba9..b078cb8b19 100644 --- a/talk/app/webrtc/java/android/org/webrtc/ThreadUtils.java +++ b/talk/app/webrtc/java/android/org/webrtc/ThreadUtils.java @@ -28,9 +28,11 @@ package org.webrtc; import android.os.Handler; +import android.os.SystemClock; import java.util.concurrent.Callable; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; final class ThreadUtils { /** @@ -104,6 +106,30 @@ final class ThreadUtils { }); } + public static boolean awaitUninterruptibly(CountDownLatch barrier, long timeoutMs) { + final long startTimeMs = SystemClock.elapsedRealtime(); + long timeRemainingMs = timeoutMs; + boolean wasInterrupted = false; + boolean result = false; + do { + try { + result = barrier.await(timeRemainingMs, TimeUnit.MILLISECONDS); + break; + } catch (InterruptedException e) { + // Someone is asking us to return early at our convenience. We can't cancel this operation, + // but we should preserve the information and pass it along. + wasInterrupted = true; + final long elapsedTimeMs = SystemClock.elapsedRealtime() - startTimeMs; + timeRemainingMs = timeoutMs - elapsedTimeMs; + } + } while (timeRemainingMs > 0); + // Pass interruption information along. + if (wasInterrupted) { + Thread.currentThread().interrupt(); + } + return result; + } + /** * Post |callable| to |handler| and wait for the result. */ diff --git a/talk/app/webrtc/java/src/org/webrtc/MediaCodecVideoDecoder.java b/talk/app/webrtc/java/src/org/webrtc/MediaCodecVideoDecoder.java index 42af9c7fd0..ba94cc1e6d 100644 --- a/talk/app/webrtc/java/src/org/webrtc/MediaCodecVideoDecoder.java +++ b/talk/app/webrtc/java/src/org/webrtc/MediaCodecVideoDecoder.java @@ -43,13 +43,12 @@ import org.webrtc.Logging; import java.nio.ByteBuffer; import java.util.Arrays; import java.util.List; +import java.util.concurrent.CountDownLatch; import javax.microedition.khronos.egl.EGLContext; // Java-side of peerconnection_jni.cc:MediaCodecVideoDecoder. // This class is an implementation detail of the Java PeerConnection API. -// MediaCodec is thread-hostile so this class must be operated on a single -// thread. public class MediaCodecVideoDecoder { // This class is constructed, operated, and destroyed by its C++ incarnation, // so the class and its methods have non-public visibility. The API this @@ -66,9 +65,13 @@ public class MediaCodecVideoDecoder { } private static final int DEQUEUE_INPUT_TIMEOUT = 500000; // 500 ms timeout. + private static final int MEDIA_CODEC_RELEASE_TIMEOUT_MS = 5000; // Timeout for codec releasing. // Active running decoder instance. Set in initDecode() (called from native code) // and reset to null in release() call. private static MediaCodecVideoDecoder runningInstance = null; + private static MediaCodecVideoDecoderErrorCallback errorCallback = null; + private static int codecErrors = 0; + private Thread mediaCodecThread; private MediaCodec mediaCodec; private ByteBuffer[] inputBuffers; @@ -105,6 +108,18 @@ public class MediaCodecVideoDecoder { private MediaCodecVideoDecoder() { } + // MediaCodec error handler - invoked when critical error happens which may prevent + // further use of media codec API. Now it means that one of media codec instances + // is hanging and can no longer be used in the next call. + public static interface MediaCodecVideoDecoderErrorCallback { + void onMediaCodecVideoDecoderCriticalError(int codecErrors); + } + + public static void setErrorCallback(MediaCodecVideoDecoderErrorCallback errorCallback) { + Logging.d(TAG, "Set error callback"); + MediaCodecVideoDecoder.errorCallback = errorCallback; + } + // Helper struct for findVp8Decoder() below. private static class DecoderProperties { public DecoderProperties(String codecName, int colorFormat) { @@ -273,12 +288,36 @@ public class MediaCodecVideoDecoder { private void release() { Logging.d(TAG, "Java releaseDecoder"); checkOnMediaCodecThread(); - try { - mediaCodec.stop(); - mediaCodec.release(); - } catch (IllegalStateException e) { - Logging.e(TAG, "release failed", e); + + // 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() { + try { + Logging.d(TAG, "Java releaseDecoder on release thread"); + mediaCodec.stop(); + mediaCodec.release(); + Logging.d(TAG, "Java releaseDecoder on release thread done"); + } catch (Exception e) { + Logging.e(TAG, "Media decoder release failed", e); + } + releaseDone.countDown(); + } + }; + new Thread(runMediaCodecRelease).start(); + + if (!ThreadUtils.awaitUninterruptibly(releaseDone, MEDIA_CODEC_RELEASE_TIMEOUT_MS)) { + Logging.e(TAG, "Media decoder release timeout"); + codecErrors++; + if (errorCallback != null) { + Logging.e(TAG, "Invoke codec error callback. Errors: " + codecErrors); + errorCallback.onMediaCodecVideoDecoderCriticalError(codecErrors); + } } + mediaCodec = null; mediaCodecThread = null; runningInstance = null; @@ -354,6 +393,7 @@ public class MediaCodecVideoDecoder { private Object dequeueOutputBuffer(int dequeueTimeoutUs) throws IllegalStateException, MediaCodec.CodecException { checkOnMediaCodecThread(); + // Drain the decoder until receiving a decoded buffer or hitting // MediaCodec.INFO_TRY_AGAIN_LATER. final MediaCodec.BufferInfo info = new MediaCodec.BufferInfo(); diff --git a/talk/app/webrtc/java/src/org/webrtc/MediaCodecVideoEncoder.java b/talk/app/webrtc/java/src/org/webrtc/MediaCodecVideoEncoder.java index f3f03c1d20..988d62a3ff 100644 --- a/talk/app/webrtc/java/src/org/webrtc/MediaCodecVideoEncoder.java +++ b/talk/app/webrtc/java/src/org/webrtc/MediaCodecVideoEncoder.java @@ -40,11 +40,10 @@ import org.webrtc.Logging; import java.nio.ByteBuffer; import java.util.Arrays; import java.util.List; +import java.util.concurrent.CountDownLatch; // Java-side of peerconnection_jni.cc:MediaCodecVideoEncoder. // This class is an implementation detail of the Java PeerConnection API. -// MediaCodec is thread-hostile so this class must be operated on a single -// thread. public class MediaCodecVideoEncoder { // This class is constructed, operated, and destroyed by its C++ incarnation, // so the class and its methods have non-public visibility. The API this @@ -60,10 +59,14 @@ public class MediaCodecVideoEncoder { VIDEO_CODEC_H264 } + private static final int MEDIA_CODEC_RELEASE_TIMEOUT_MS = 5000; // Timeout for codec releasing. private static final int DEQUEUE_TIMEOUT = 0; // Non-blocking, no wait. // Active running encoder instance. Set in initDecode() (called from native code) // and reset to null in release() call. private static MediaCodecVideoEncoder runningInstance = null; + private static MediaCodecVideoEncoderErrorCallback errorCallback = null; + private static int codecErrors = 0; + private Thread mediaCodecThread; private MediaCodec mediaCodec; private ByteBuffer[] outputBuffers; @@ -108,6 +111,18 @@ public class MediaCodecVideoEncoder { private MediaCodecVideoEncoder() { } + // MediaCodec error handler - invoked when critical error happens which may prevent + // further use of media codec API. Now it means that one of media codec instances + // is hanging and can no longer be used in the next call. + public static interface MediaCodecVideoEncoderErrorCallback { + void onMediaCodecVideoEncoderCriticalError(int codecErrors); + } + + public static void setErrorCallback(MediaCodecVideoEncoderErrorCallback errorCallback) { + Logging.d(TAG, "Set error callback"); + MediaCodecVideoEncoder.errorCallback = errorCallback; + } + // Helper struct for findHwEncoder() below. private static class EncoderProperties { public EncoderProperties(String codecName, int colorFormat) { @@ -130,8 +145,7 @@ public class MediaCodecVideoEncoder { if (mime.equals(H264_MIME_TYPE)) { List exceptionModels = Arrays.asList(H264_HW_EXCEPTION_MODELS); if (exceptionModels.contains(Build.MODEL)) { - Logging.w(TAG, "Model: " + Build.MODEL + - " has black listed H.264 encoder."); + Logging.w(TAG, "Model: " + Build.MODEL + " has black listed H.264 encoder."); return null; } } @@ -306,12 +320,36 @@ public class MediaCodecVideoEncoder { private void release() { Logging.d(TAG, "Java releaseEncoder"); checkOnMediaCodecThread(); - try { - mediaCodec.stop(); - mediaCodec.release(); - } catch (IllegalStateException e) { - Logging.e(TAG, "release failed", e); + + // 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() { + try { + Logging.d(TAG, "Java releaseEncoder on release thread"); + mediaCodec.stop(); + mediaCodec.release(); + 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); + } } + mediaCodec = null; mediaCodecThread = null; runningInstance = null;