Fix wrong selection of VP8 video decoder on some Samsung devices.

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

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.media;
 
 import android.media.AudioFormat;
 import android.media.AudioManager;
 import android.media.AudioTrack;
 import android.media.MediaCodec;
@@ skipping 204 matching lines @@
     private static MediaCodecBridge create(String mime, boolean isSecure, int direction) {
         // Creation of ".secure" codecs sometimes crash instead of throwing exceptions
         // on pre-JBMR2 devices.
         if (isSecure && Build.VERSION.SDK_INT < Build.VERSION_CODES.JELLY_BEAN_MR2) {
             return null;
         }
         MediaCodec mediaCodec = null;
         boolean adaptivePlaybackSupported = false;
         try {
             // |isSecure| only applies to video decoders.
-            if (mime.startsWith("video") && isSecure && direction == MEDIA_CODEC_DECODER) {
+            if (mime.startsWith("video") && direction == MEDIA_CODEC_DECODER) {
                 String decoderName = getDecoderNameForMime(mime);
                 if (decoderName == null) {
                     return null;
                 }
-                if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT) {
-                    // To work around an issue that we cannot get the codec info from the secure
-                    // decoder, create an insecure decoder first so that we can query its codec
-                    // info. http://b/15587335.
-                    MediaCodec insecureCodec = MediaCodec.createByCodecName(decoderName);
-                    adaptivePlaybackSupported = codecSupportsAdaptivePlayback(insecureCodec, mime);
-                    insecureCodec.release();
+                if (isSecure) {
+                    if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT) {
+                          // To work around an issue that we cannot get the codec info from the
+                          // secure decoder, create an insecure decoder first so that we can query
+                          // its codec info. http://b/15587335.
+                          MediaCodec insecureCodec = MediaCodec.createByCodecName(decoderName);
+                          adaptivePlaybackSupported =
+                                  codecSupportsAdaptivePlayback(insecureCodec, mime);
+                          insecureCodec.release();
+                    }
+                    mediaCodec = MediaCodec.createByCodecName(decoderName + ".secure");
+                } else {
+                    mediaCodec = MediaCodec.createByCodecName(decoderName);

[qinmin, 2014/09/26 05:00:28]

Are you sure this will not cause any other issues?
AFAIK, Calling createDecoderByType() will automatically switch to software
decoder if hardware decoder is currently being used. However, if we call
createByCodecName() twice on the hardware decoder, the 2nd call will fail.

[AlexGlaznev, 2014/09/26 18:01:37]

No, I am not 100% sure. But I know, that current way of codec selection is
causing issues on majority of Samsung devices.
Yes, you are right - probably not on a second call, but after all HW codec
instances will be used - probably after 7 - 12 calls. Current Android GTS tests
require at least 3 encoder and 5 decoder instances to run in parallel (all
instances are created using createByCodecName).

But the drawback of using createDecoderByType is that it can pick up non HW
accelerated or buggy codec (and this is what is happening for Samsung devices)
or fall back to this bad codec after all available instances of HW codec will be
used. 

So I think there could be two possible solutions:

1. Continue to use CreateEncoder{Decoder}ByType, but change
MediaCodecBridge::IsKnownUnaccelerated to obtain the name of the default codec
(it will require opening codec instance with CreateEncoder{Decoder}ByType and
calling MediaCodec.getName()) instead of the top codec in MediaCodecList. Then
android_video_decoder_accelerator.cc will not try to create and use MediaCodec
for devices with default OMX.google or OMX.SEC codecs.

2. Approach in this CL - for video decoder use CreateByCodecName and expect that
it will fail after all HW codec instances will be used.

Please advise what do you think will be a better choice.

[qinmin, 2014/09/26 22:45:55]

Can you explain why CreateDecoderByType() will end up with buggy codecs?
I checked findMatchingCodecs() in OMXCodec.cpp, that's how CreateDecoderByType()
is implemented in native site. I saw that it also uses MediaCodecList to
retrieve a list of codecs, and return the list to ACodec, which normally picks
the first one.
This implementation is pretty much the same as getDecoderNameForMime() here. So
I don't know why changing from CreateDecoderByType() to createByCodecName()
solves the issue.

On 2014/09/26 18:01:37, AlexGlaznev wrote:

+                    adaptivePlaybackSupported = codecSupportsAdaptivePlayback(mediaCodec, mime);
                 }
-                mediaCodec = MediaCodec.createByCodecName(decoderName + ".secure");
             } else {
                 if (direction == MEDIA_CODEC_ENCODER) {
                     mediaCodec = MediaCodec.createEncoderByType(mime);
                 } else {
                     mediaCodec = MediaCodec.createDecoderByType(mime);
                     adaptivePlaybackSupported = codecSupportsAdaptivePlayback(mediaCodec, mime);
                 }
             }
         } catch (Exception e) {
             Log.e(TAG, "Failed to create MediaCodec: " +  mime + ", isSecure: "
@@ skipping 392 matching lines @@
                 return AudioFormat.CHANNEL_OUT_QUAD;
             case 6:
                 return AudioFormat.CHANNEL_OUT_5POINT1;
             case 8:
                 return AudioFormat.CHANNEL_OUT_7POINT1;
             default:
                 return AudioFormat.CHANNEL_OUT_DEFAULT;
         }
     }
 }