Fix a bug that mime type isn't passed when checking Codec capabilities

media/base/android/media_codec_util.cc

Index: media/base/android/media_codec_util.cc
diff --git a/media/base/android/media_codec_util.cc b/media/base/android/media_codec_util.cc
index 0365aa3686357b670a8ba4f59c6ef2257c47e77d..d592ac0a6bb129999d91304ab78e48c448ebe03f 100644
--- a/media/base/android/media_codec_util.cc
+++ b/media/base/android/media_codec_util.cc
@@ -23,27 +23,46 @@ using base::android::ConvertUTF8ToJavaString;
 using base::android::JavaIntArrayToIntVector;
 using base::android::ScopedJavaLocalRef;
 
+namespace {

[ddorwin, 2016/04/18 22:09:09]

nit: empty line at beginning and end of namespaces.

[qinmin, 2016/04/19 23:27:11]

Done.

+const char kMP4AMimeType[] = "audio/mp4a-latm";

[ddorwin, 2016/04/18 22:09:09]

I think we generally do kMp4a... (The MP4 examples don't support this, but I
believe that would be correct. Vp9 is definitely done more that way.

ditto for Avc, Vp8, Vp9 and the others I commented below.

[qinmin, 2016/04/19 23:27:10]

Done.

+const char kOPUSMimeType[] = "audio/opus";

[ddorwin, 2016/04/18 22:09:09]

Opus

[qinmin, 2016/04/19 23:27:10]

Done.

+const char kVORBISMimeType[] = "audio/vorbis";

[ddorwin, 2016/04/18 22:09:08]

Vorbis

[qinmin, 2016/04/19 23:27:10]

Done.

+const char kAVCMimeType[] = "video/avc";
+const char kHVCMimeType[] = "video/hevc";

[ddorwin, 2016/04/18 22:09:09]

kHevc... I haven't seen HVC used as an acronym.

[qinmin, 2016/04/19 23:27:11]

Done.

+const char kVP8MimeType[] = "video/x-vnd.on2.vp8";
+const char kVP9MimeType[] = "video/x-vnd.on2.vp9";
+}
+
 namespace media {

[ddorwin, 2016/04/18 22:09:09]

nit: Put the anonymous namespace in this one. No impact here, but it's generally
the preferred pattern I believe.

[qinmin, 2016/04/19 23:27:10]

Done.

 
 static std::string CodecTypeToAndroidMimeType(const std::string& codec) {
   // TODO(xhwang): Shall we handle more detailed strings like "mp4a.40.2"?
   if (codec == "avc1")
-    return "video/avc";
+    return kAVCMimeType;
   if (codec == "hvc1")
-    return "video/hevc";
+    return kHVCMimeType;
   if (codec == "mp4a")
-    return "audio/mp4a-latm";
+    return kMP4AMimeType;
   if (codec == "vp8" || codec == "vp8.0")
-    return "video/x-vnd.on2.vp8";
+    return kVP8MimeType;
   if (codec == "vp9" || codec == "vp9.0")
-    return "video/x-vnd.on2.vp9";
+    return kVP9MimeType;
   if (codec == "vorbis")
-    return "audio/vorbis";
+    return kVORBISMimeType;
   if (codec == "opus")
-    return "audio/opus";
+    return kOPUSMimeType;
+  DLOG(WARNING) << "Cannot convert codec to Android MIME type: " << codec;
   return std::string();
 }
 
+static bool IsMimeTypeSupported(const std::string& mime_type) {

[ddorwin, 2016/04/18 22:09:09]

|android_mime_type|

[qinmin, 2016/04/19 23:27:10]

Done. However, this function should checks whether the mime_type is an android
supported mime_type, so using mime_type is more proper.

+  std::vector<std::string> supported{

[ddorwin, 2016/04/18 22:09:09]

nit: space before '{' ?

[qinmin, 2016/04/19 23:27:10]

I added the space initially, but "git cl media format" removed that space on
purpose.

+      kMP4AMimeType, kOPUSMimeType, kVORBISMimeType, kAVCMimeType,
+      kHVCMimeType,  kVP8MimeType,  kVP9MimeType};

[ddorwin, 2016/04/18 22:09:09]

Note: Initializer lists are in the process of being allowed [1]. Since this is
Android only, this seems fine.

[1]
https://groups.google.com/a/chromium.org/forum/#!searchin/cxx/Initialization$...

[qinmin, 2016/04/19 23:27:10]

good to know, thanks

+  return std::find(supported.begin(), supported.end(), mime_type) !=

[ddorwin, 2016/04/18 22:09:09]

This is a bit simpler if you use a base::hash_set above, though that might be
more size and complexity in implementation. Up to you.

[qinmin, 2016/04/19 23:27:11]

Since this is only used in DCHECK, i will leave this as it is now.

+         supported.end();
+}
+
 static std::string GetDefaultCodecName(const std::string& mime_type,
                                        MediaCodecDirection direction) {
   DCHECK(MediaCodecUtil::IsMediaCodecAvailable());
@@ -54,10 +73,12 @@ static std::string GetDefaultCodecName(const std::string& mime_type,
   return ConvertJavaStringToUTF8(env, j_codec_name.obj());
 }
 
-static bool IsDecoderSupportedByDevice(const std::string& mime_type) {
+static bool IsDecoderSupportedByDevice(const std::string& android_mime_type) {
   DCHECK(MediaCodecUtil::IsMediaCodecAvailable());
+  DCHECK(IsMimeTypeSupported(android_mime_type));
   JNIEnv* env = AttachCurrentThread();
-  ScopedJavaLocalRef<jstring> j_mime = ConvertUTF8ToJavaString(env, mime_type);
+  ScopedJavaLocalRef<jstring> j_mime =
+      ConvertUTF8ToJavaString(env, android_mime_type);
   return Java_MediaCodecUtil_isDecoderSupportedForDevice(env, j_mime.obj());
 }
 
@@ -116,13 +137,14 @@ bool MediaCodecUtil::CanDecode(const std::string& codec, bool is_secure) {
 }
 
 // static
-bool MediaCodecUtil::IsKnownUnaccelerated(const std::string& mime_type,
+bool MediaCodecUtil::IsKnownUnaccelerated(const std::string& android_mime_type,
                                           MediaCodecDirection direction) {
   if (!IsMediaCodecAvailable())
     return true;
 
-  std::string codec_name = GetDefaultCodecName(mime_type, direction);
-  DVLOG(1) << __FUNCTION__ << "Default codec for " << mime_type << " : "
+  DCHECK(IsMimeTypeSupported(android_mime_type));

[ddorwin, 2016/04/18 22:09:09]

We can still do debug checks of input even regardless of 142. IOW, make this the
first line.

[qinmin, 2016/04/19 23:27:11]

Done.

+  std::string codec_name = GetDefaultCodecName(android_mime_type, direction);
+  DVLOG(1) << __FUNCTION__ << "Default codec for " << android_mime_type << " : "
            << codec_name << ", direction: " << direction;
   if (!codec_name.size())
     return true;
@@ -131,10 +153,10 @@ bool MediaCodecUtil::IsKnownUnaccelerated(const std::string& mime_type,
   // MediaTek hardware vp9 is known crashy, see http://crbug.com/446974 and
   // http://crbug.com/597836.
   if (base::StartsWith(codec_name, "OMX.MTK.", base::CompareCase::SENSITIVE)) {
-    if (mime_type == "video/x-vnd.on2.vp8")
+    if (android_mime_type.compare(kVP8MimeType) == 0)

[ddorwin, 2016/04/18 22:09:09]

Is there a reason you changed from just using ==?

[qinmin, 2016/04/19 23:27:11]

They are essentially the same, personal preference thing. Anyway, reverted to
use ==.

       return true;
 
-    if (mime_type == "video/x-vnd.on2.vp9")
+    if (android_mime_type.compare(kVP9MimeType) == 0)
       return base::android::BuildInfo::GetInstance()->sdk_int() < 21;
 
     return false;
@@ -178,7 +200,7 @@ bool MediaCodecUtil::RegisterMediaCodecUtil(JNIEnv* env) {
 
 // static
 bool MediaCodecUtil::IsVp8DecoderAvailable() {
-  return IsMediaCodecAvailable() && IsDecoderSupportedByDevice("vp8");
+  return IsMediaCodecAvailable() && IsDecoderSupportedByDevice(kVP8MimeType);
 }
 
 // static
@@ -190,7 +212,7 @@ bool MediaCodecUtil::IsVp8EncoderAvailable() {
 
 // static
 bool MediaCodecUtil::IsVp9DecoderAvailable() {
-  return IsMediaCodecAvailable() && IsDecoderSupportedByDevice("vp9");
+  return IsMediaCodecAvailable() && IsDecoderSupportedByDevice(kVP9MimeType);
 }
 
 // static