Android MediaCodec: catch exceptions to stop() and other methods.

media/base/android/java/src/org/chromium/media/MediaCodecBridge.java

Index: media/base/android/java/src/org/chromium/media/MediaCodecBridge.java
diff --git a/media/base/android/java/src/org/chromium/media/MediaCodecBridge.java b/media/base/android/java/src/org/chromium/media/MediaCodecBridge.java
index ecc90018c2082a81a8690e084c629be3028ee3b7..089581cea862eabc44ac432ae0901d5fb7be156b 100644
--- a/media/base/android/java/src/org/chromium/media/MediaCodecBridge.java
+++ b/media/base/android/java/src/org/chromium/media/MediaCodecBridge.java
@@ -296,13 +296,23 @@ class MediaCodecBridge {
 
     @CalledByNative
     private void stop() {
-        mMediaCodec.stop();
+        try {
+            mMediaCodec.stop();
+        } catch (IllegalStateException e) {
+            Log.e(TAG, "Failed to stop MediaCodec", e);
+        }

[watk, 2016/10/28 21:49:56]

Should we be returning MEDIA_CODEC_ERROR like for the other calls?

[braveyao, 2016/10/28 22:12:57]

'stop()' will be only called in AndroidVideoEncodeAccelerator.Destroy() at
present, before it kills itself. So no one can really handle the failure here, I
suppose.
Also according to Android doc, this exception means mediaCodec is already in
release state. So it's OK we do nothing.

[watk, 2016/10/28 22:17:57]

ok, sg for stop(). 

     }
 
     @TargetApi(Build.VERSION_CODES.KITKAT)
     @CalledByNative
     private String getName() {
-        return mMediaCodec.getName();
+        String codecName = "unknown";
+        try {
+            codecName = mMediaCodec.getName();
+        } catch (IllegalStateException e) {
+            Log.e(TAG, "Cannot get codec name", e);
+        }
+        return codecName;
     }
 
     @CalledByNative
@@ -370,7 +380,11 @@ class MediaCodecBridge {
 
         Bundle b = new Bundle();
         b.putInt(MediaCodec.PARAMETER_KEY_VIDEO_BITRATE, targetBps);
-        mMediaCodec.setParameters(b);
+        try {
+            mMediaCodec.setParameters(b);
+        } catch (IllegalStateException e) {
+            Log.e(TAG, "Failed to set MediaCodec parameters", e);

[watk, 2016/10/28 22:17:57]

I feel like this should return an error? If the codec is in an illegal state the
client should stop using it and release it right? We shouldn't try to continue.

[braveyao, 2016/10/28 22:34:43]

Yes these two are tricky. Personally I feel it's OK to keep it as is.
First, the caller doesn't need to care the result of these two methods.
Second, if there is IllegalStateException, then the queue/dequeue processing
will definitely encounter same exception immediately and they will reset or end
the call.
WDYT?

[watk, 2016/10/28 22:37:39]

Sure, I defer to your judgement on these ones. I'm not too familiar with the
encoding path.

+        }
         Log.v(TAG,
                 "setVideoBitrate: input " + bps + "bps@" + frameRate + ", targetBps " + targetBps);
     }
@@ -380,7 +394,11 @@ class MediaCodecBridge {
     private void requestKeyFrameSoon() {
         Bundle b = new Bundle();
         b.putInt(MediaCodec.PARAMETER_KEY_REQUEST_SYNC_FRAME, 0);
-        mMediaCodec.setParameters(b);
+        try {
+            mMediaCodec.setParameters(b);
+        } catch (IllegalStateException e) {
+            Log.e(TAG, "Failed to set MediaCodec parameters", e);

[watk, 2016/10/28 22:17:57]

I feel like this should return an error too.

[braveyao, 2016/10/28 22:34:43]

ditto

+        }
     }
 
     @CalledByNative