Fix mime type mappings when the unified media pipeline is enabled.

media/base/mime_util_internal.cc

Index: media/base/mime_util_internal.cc
diff --git a/media/base/mime_util_internal.cc b/media/base/mime_util_internal.cc
index 6e935b3cc295ab42283f8b56381be2c2a26ddecb..01fb865c7101d90184e320d37af9b1af56bc06f1 100644
--- a/media/base/mime_util_internal.cc
+++ b/media/base/mime_util_internal.cc
@@ -8,10 +8,12 @@
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "build/build_config.h"
+#include "media/base/media.h"
 #include "media/media_features.h"
 
 #if defined(OS_ANDROID)
 #include "base/android/build_info.h"
+#include "media/base/android/media_codec_util.h"
 #endif
 
 namespace media {
@@ -317,12 +319,22 @@ static bool ParseHEVCCodecID(const std::string& codec_id,
 #endif
 
 MimeUtil::MimeUtil() : allow_proprietary_codecs_(false) {
+#if defined(OS_ANDROID)
+  platform_info_.has_platform_decoder = HasPlatformDecoderSupport();
+  platform_info_.has_unified_media_pipeline = IsUnifiedMediaPipelineEnabled();
+  platform_info_.has_opus = MediaCodecUtil::PlatformHasOpusSupport();
+  platform_info_.has_vp8 = MediaCodecUtil::IsVp8Blacklisted();

[ddorwin, 2016/02/17 21:18:39]

!

[ddorwin, 2016/02/17 21:18:39]

VP8 is different from Opus and VP9. The VP8 blacklisting only applies to
MediaCodec, right? Whereas the Opus and VP9 checks apply to MediaPlayer too. The
functions are on MediaCodecUtil, which makes the relevance to MediaPlayer
confusing...

The difference - without being communicated by the names - also confused me when
reviewing the code below.

[DaleCurtis, 2016/02/18 03:58:09]

Done and changed to has_encrypted_vp8 since that's the only context it's used
in.

+  platform_info_.has_vp9 = MediaCodecUtil::PlatformHasVp9Support();
+#endif
+
   InitializeMimeTypeMaps();
 }
 
 SupportsType MimeUtil::AreSupportedCodecs(
     const CodecSet& supported_codecs,
-    const std::vector<std::string>& codecs) const {
+    const std::vector<std::string>& codecs,
+    const std::string& mime_type_lower_case,
+    bool is_encrypted) const {
   DCHECK(!supported_codecs.empty());
   DCHECK(!codecs.empty());
 
@@ -333,7 +345,7 @@ SupportsType MimeUtil::AreSupportedCodecs(
     if (!StringToCodec(codecs[i], &codec, &is_ambiguous))
       return IsNotSupported;
 
-    if (!IsCodecSupported(codec) ||
+    if (!IsCodecSupported(codec, mime_type_lower_case, is_encrypted) ||
         supported_codecs.find(codec) == supported_codecs.end()) {
       return IsNotSupported;
     }
@@ -410,7 +422,8 @@ void MimeUtil::ParseCodecString(const std::string& codecs,
 
 SupportsType MimeUtil::IsSupportedMediaFormat(
     const std::string& mime_type,
-    const std::vector<std::string>& codecs) const {
+    const std::vector<std::string>& codecs,
+    bool is_encrypted) const {
   const std::string mime_type_lower_case = base::ToLowerASCII(mime_type);
   MediaFormatMappings::const_iterator it_media_format_map =
       media_format_map_.find(mime_type_lower_case);
@@ -419,8 +432,8 @@ SupportsType MimeUtil::IsSupportedMediaFormat(
 
   if (it_media_format_map->second.empty()) {
     // We get here if the mimetype does not expect a codecs parameter.
-    return (codecs.empty() &&
-            IsDefaultCodecSupportedLowerCase(mime_type_lower_case))
+    return (codecs.empty() && IsDefaultCodecSupportedLowerCase(
+                                  mime_type_lower_case, is_encrypted))
                ? IsSupported
                : IsNotSupported;
   }
@@ -434,7 +447,9 @@ SupportsType MimeUtil::IsSupportedMediaFormat(
     if (!GetDefaultCodecLowerCase(mime_type_lower_case, &default_codec))
       return MayBeSupported;
 
-    return IsCodecSupported(default_codec) ? IsSupported : IsNotSupported;
+    return IsCodecSupported(default_codec, mime_type_lower_case, is_encrypted)
+               ? IsSupported
+               : IsNotSupported;
   }
 
 #if BUILDFLAG(ENABLE_MSE_MPEG2TS_STREAM_PARSER)
@@ -443,11 +458,13 @@ SupportsType MimeUtil::IsSupportedMediaFormat(
     for (const auto& codec_id : codecs) {
       codecs_to_check.push_back(TranslateLegacyAvc1CodecIds(codec_id));
     }
-    return AreSupportedCodecs(it_media_format_map->second, codecs_to_check);
+    return AreSupportedCodecs(it_media_format_map->second, codecs_to_check,
+                              mime_type_lower_case, is_encrypted);
   }
 #endif
 
-  return AreSupportedCodecs(it_media_format_map->second, codecs);
+  return AreSupportedCodecs(it_media_format_map->second, codecs,
+                            mime_type_lower_case, is_encrypted);
 }
 
 void MimeUtil::RemoveProprietaryMediaTypesAndCodecs() {
@@ -457,6 +474,10 @@ void MimeUtil::RemoveProprietaryMediaTypesAndCodecs() {
   allow_proprietary_codecs_ = false;
 }
 
+void MimeUtil::SetPlatformCodecInfoForTests(const PlatformCodecInfo& info) {

[ddorwin, 2016/02/17 21:18:38]

#ifdef

[DaleCurtis, 2016/02/18 03:58:08]

See previous comment.

+  platform_info_ = info;
+}
+
 bool MimeUtil::StringToCodec(const std::string& codec_id,
                              Codec* codec,
                              bool* is_ambiguous) const {
@@ -479,11 +500,13 @@ bool MimeUtil::StringToCodec(const std::string& codec_id,
   return ParseH264CodecID(codec_id, codec, is_ambiguous);
 }
 
-bool MimeUtil::IsCodecSupported(Codec codec) const {
+bool MimeUtil::IsCodecSupported(Codec codec,
+                                const std::string& mime_type_lower_case,
+                                bool is_encrypted) const {
   DCHECK_NE(codec, INVALID_CODEC);
 
 #if defined(OS_ANDROID)
-  if (!IsCodecSupportedOnAndroid(codec))
+  if (!IsCodecSupportedOnAndroid(codec, mime_type_lower_case, is_encrypted))
     return false;
 #endif
 
@@ -538,45 +561,90 @@ bool MimeUtil::GetDefaultCodecLowerCase(const std::string& mime_type_lower_case,
 }
 
 bool MimeUtil::IsDefaultCodecSupportedLowerCase(

[ddorwin, 2016/02/17 21:18:39]

Unrelated to this CL:
These ...DefaultCodec...LowerCase functions seem poorly named. It's the MIME
type that is lower case, but it's never mentioned.

...DefaultCodec...{In|For}MimeTypeLowerCase?

Also, the implementation(s) should probably DCHECK that the string is lower case
[ASCII].

[DaleCurtis, 2016/02/18 03:58:09]

Acknowledged.

-    const std::string& mime_type_lower_case) const {
+    const std::string& mime_type_lower_case,
+    bool is_encrypted) const {
   Codec default_codec = Codec::INVALID_CODEC;
   if (!GetDefaultCodecLowerCase(mime_type_lower_case, &default_codec))
     return false;
-  return IsCodecSupported(default_codec);
+  return IsCodecSupported(default_codec, mime_type_lower_case, is_encrypted);
 }
 
 #if defined(OS_ANDROID)
-bool MimeUtil::IsCodecSupportedOnAndroid(Codec codec) const {
+bool MimeUtil::IsCodecSupportedOnAndroid(

[ddorwin, 2016/02/17 21:18:38]

Much simpler now. Thanks!

[DaleCurtis, 2016/02/18 03:58:09]

Acknowledged.

+    Codec codec,
+    const std::string& mime_type_lower_case,
+    bool is_encrypted) const {
+  // Encrypted block support is never available without platform decoders.
+  if (is_encrypted && !platform_info_.has_platform_decoder)
+    return false;
+
+  // NOTE: We do not account for Media Source Extensions (MSE) within these
+  // checks since it has its own isTypeSupported() which will handle platform
+  // specific codec rejections.  See http://crbug.com/587303.
+
   switch (codec) {
+    // The following codecs are never supported.
+    case AC3:
+    case EAC3:
     case INVALID_CODEC:
+    case THEORA:
       return false;
 
+    // The following codecs may be supported depending on platform abilities.

[ddorwin, 2016/02/17 21:18:38]

nit: This comment appears to apply to these 6 codecs, but I think it applies to
all. (Actually, this is not strictly true for the first 6.)

Perhaps:
// The remaining...
<empty line>

[DaleCurtis, 2016/02/18 03:58:09]

Done.

     case PCM:
     case MP3:

[ddorwin, 2016/02/17 21:18:38]

Note: The results for proprietary audio codecs are incorrect when
has_unified_media_pipeline && ffmpeg_branding!=Chrome*
If there was a preprocessor define for the latter, this would be easy to
address, but I don't believe there is. (proprietary_codecs is always set for
Android, so we can't check that here.) There are various other bugs that might
make this easier to address.

[DaleCurtis, 2016/02/18 03:58:09]

This is correct, but should be a temporary issue once sorted out with legal, so
I won't bother muddling this code any further.

     case MPEG4_AAC_LC:
     case MPEG4_AAC_SBR_v1:
     case MPEG4_AAC_SBR_PS_v2:
     case VORBIS:
+      // These codecs are always supported; via a platform decoder (when used
+      // with MSE/EME), a software decoder (the unified pipeline), or with
+      // MediaPlayer.
+      DCHECK(!is_encrypted || platform_info_.has_platform_decoder);
+      return true;
+
     case H264_BASELINE:

[ddorwin, 2016/02/17 21:18:39]

We should try to keep audio and video (especially VPx) together.

These were out of order because we had a big block that all returned true. Now
that we have logic, I think we should group them and try to keep the order as
consistent with the enum definition as possible.

[DaleCurtis, 2016/02/18 03:58:09]

Done.

     case H264_MAIN:
     case H264_HIGH:
-    case VP8:
+      // The unified pipeline requires platform support for h264.
+      if (platform_info_.has_unified_media_pipeline)
+        return platform_info_.has_platform_decoder;
+
+      // When MediaPlayer is used, h264 is always supported.
+      DCHECK(!is_encrypted || platform_info_.has_platform_decoder);
       return true;
 
-    case AC3:
-    case EAC3:
-      // TODO(servolk): Revisit this for AC3/EAC3 support on AndroidTV
-      return false;
+    case VP8:
+      if (is_encrypted)

[ddorwin, 2016/02/17 21:18:39]

I wonder if we should swap the logic here to be more consistent with VP9,
especially if we simplify VP9 per my comments there.

[DaleCurtis, 2016/02/18 03:58:09]

This logic is not quite the same since MediaPlayer can always play vp8, but we
want to excluded encrypted vp8 if MediaCodec/PlatformDecoders are not available.
I've reworked it to be consistent with the others though and renamed has_vp8 to
supports_encrypted_vp8.

+        return platform_info_.has_vp8;
+
+      // MediaPlayer or the unified pipeline can always play vp8 via software.

[ddorwin, 2016/02/17 21:18:39]

nit: This wording is ambiguous. MP does not use SW.

[DaleCurtis, 2016/02/18 03:58:09]

Done.

+      return true;

[ddorwin, 2016/02/17 21:18:39]

Note: This is not true for MSE and !has_unified_media_pipeline, but MSE doesn't
currently use this code.

[DaleCurtis, 2016/02/18 03:58:09]

Done.

 
     case MPEG2_AAC_LC:
     case MPEG2_AAC_MAIN:
     case MPEG2_AAC_SSR:
-      // MPEG-2 variants of AAC are not supported on Android.
-      return false;
+      // MPEG-2 variants of AAC are not supported on Android unless the unified
+      // media pipeline can be used. These codecs will be decoded in software.
+      return !is_encrypted && platform_info_.has_unified_media_pipeline;
 
     case OPUS:
-      // Opus is supported only in Lollipop+ (API Level 21).
-      return base::android::BuildInfo::GetInstance()->sdk_int() >= 21;
+      // If clear, the unified pipeline can always decode OPUS in software.
+      if (!is_encrypted && platform_info_.has_unified_media_pipeline)
+        return true;
+
+      // Otherwise, platform support is required.
+      if (!platform_info_.has_opus)
+        return false;
+
+      // Android does not support opus in ogg containers.

[ddorwin, 2016/02/17 21:18:38]

"The Android platform..."?
"Android MediaPlayer..."? (Not sure if MediaCodec would support Opus extracted
from Ogg, but I think that's irrelevant here since MSE and EME do not support
it.)

[DaleCurtis, 2016/02/18 03:58:09]

Done.

+      if (base::EndsWith(mime_type_lower_case, "ogg",
+                         base::CompareCase::SENSITIVE)) {
+        return false;
+      }
+
+      DCHECK(!is_encrypted || platform_info_.has_platform_decoder);

[ddorwin, 2016/02/17 21:18:39]

This is one place where the differences in the has_<codec> values is confusing.
has_opus does not imply has_platform_decoder (but we would have bailed on this
condition above).

[DaleCurtis, 2016/02/18 03:58:09]

Hopefully has_xxx vs supports_xxx helps this confusion.

+      return true;
 
     case HEVC_MAIN:
 #if BUILDFLAG(ENABLE_HEVC_DEMUXING)
@@ -587,12 +655,18 @@ bool MimeUtil::IsCodecSupportedOnAndroid(Codec codec) const {
       return false;
 #endif
 
-    case VP9:
-      // VP9 is supported only in KitKat+ (API Level 19).
-      return base::android::BuildInfo::GetInstance()->sdk_int() >= 19;
+    case VP9: {
+      // If clear, the unified pipeline can always decode VP9 in software.
+      if (!is_encrypted && platform_info_.has_unified_media_pipeline)
+        return true;
 
-    case THEORA:
-      return false;
+      // Otherwise, platform support is required.
+      if (!platform_info_.has_vp9)

[ddorwin, 2016/02/17 21:18:39]

At this point, can we just return the value of this?

[DaleCurtis, 2016/02/18 03:58:09]

Done.

+        return false;
+
+      DCHECK(!is_encrypted || platform_info_.has_platform_decoder);
+      return true;
+    }
   }
 
   return false;