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

media/base/mime_util_internal.h

Index: media/base/mime_util_internal.h
diff --git a/media/base/mime_util_internal.h b/media/base/mime_util_internal.h
index 5750ef775c07088cfbe67f42735f5e8d36ff08a6..a46ba3a8cd73189f45749a7c73d0dcbfcfb6dfa4 100644
--- a/media/base/mime_util_internal.h
+++ b/media/base/mime_util_internal.h
@@ -11,6 +11,7 @@
 
 #include "base/containers/hash_tables.h"
 #include "base/macros.h"
+#include "media/base/media_export.h"
 #include "media/base/mime_util.h"
 
 namespace media {
@@ -18,7 +19,7 @@ namespace internal {
 
 // Internal utility class for handling mime types.  Should only be invoked by
 // tests and the functions within mime_util.cc -- NOT for direct use by others.
-class MimeUtil {
+class MEDIA_EXPORT MimeUtil {
  public:
   MimeUtil();
   ~MimeUtil();
@@ -41,7 +42,20 @@ class MimeUtil {
     HEVC_MAIN,
     VP8,
     VP9,
-    THEORA
+    THEORA,
+    LAST_CODEC = THEORA
+  };
+
+  // Platform configuration structure.  Controls which codecs are supported at
+  // runtime.  Also used by tests to simulate platform differences.
+  struct PlatformInfo {
+    bool has_platform_decoders = false;
+
+    bool supports_encrypted_vp8 = false;
+    bool supports_opus = false;
+    bool supports_vp9 = false;
+
+    bool using_unified_media_pipeline = false;

[ddorwin, 2016/02/18 20:37:45]

nit: We don't actually know whether the unified pipeline will be used because
there are reasons we may fall back at load time. Thus, _enabled or something
like that might be better.

[DaleCurtis, 2016/02/19 01:35:47]

Went with |is_unified_media_pipeline_enabled|

   };
 
   // See mime_util.h for more information on these methods.
@@ -49,12 +63,22 @@ class MimeUtil {
   void ParseCodecString(const std::string& codecs,
                         std::vector<std::string>* codecs_out,
                         bool strip);
-  SupportsType IsSupportedMediaFormat(
-      const std::string& mime_type,
-      const std::vector<std::string>& codecs) const;
+  SupportsType IsSupportedMediaFormat(const std::string& mime_type,
+                                      const std::vector<std::string>& codecs,
+                                      bool is_encrypted) const;
 
   void RemoveProprietaryMediaTypesAndCodecs();
 
+  void SetPlatformInfoForTests(const PlatformInfo& info);
+
+  // Checks special Android only codec restrictions.
+  bool IsCodecSupportedOnAndroidForTests(
+      Codec codec,
+      const std::string& mime_type_lower_case,
+      bool is_encrypted) const {
+    return IsCodecSupportedOnAndroid(codec, mime_type_lower_case, is_encrypted);
+  }
+
  private:
   typedef base::hash_set<int> CodecSet;
   typedef std::map<std::string, CodecSet> MediaFormatMappings;
@@ -69,13 +93,15 @@ class MimeUtil {
   // For faster lookup, keep hash sets.
   void InitializeMimeTypeMaps();
 
-  // Returns IsSupported if all codec IDs in |codecs| are unambiguous
-  // and are supported by the platform. MayBeSupported is returned if
-  // at least one codec ID in |codecs| is ambiguous but all the codecs
-  // are supported by the platform. IsNotSupported is returned if at
-  // least one codec ID  is not supported by the platform.
+  // Returns IsSupported if all codec IDs in |codecs| are unambiguous and are
+  // supported in |mime_type_lower_case|. IsNotSupported is returned if

[ddorwin, 2016/02/18 20:37:45]

Did you mean to drop the MayBeSupported sentence?

[DaleCurtis, 2016/02/19 01:35:47]

Nope, restored.

+  // |mime_type_lower_case| is not supported or at least one is not supported in
+  // |mime_type_lower_case|. |is_encrypted| means the codec will be used with
+  // encrypted blocks.
   SupportsType 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;
 
   // Converts a codec ID into an Codec enum value and indicates
   // whether the conversion was ambiguous.
@@ -89,10 +115,13 @@ class MimeUtil {
                      Codec* codec,
                      bool* is_ambiguous) const;
 
-  // Returns true if |codec| is supported by the platform.
-  // Note: This method will return false if the platform supports proprietary
-  // codecs but |allow_proprietary_codecs_| is set to false.
-  bool IsCodecSupported(Codec codec) const;
+  // Returns true if |codec| is supported when contained in
+  // |mime_type_lower_case|. Note: This method will always return false if
+  // |allow_proprietary_codecs_| is set to false. |is_encrypted| means the codec

[ddorwin, 2016/02/18 20:37:44]

I still think you are missing a qualification in this sentence: "... false for
proprietary codecs if..."

[DaleCurtis, 2016/02/19 01:35:47]

Ah, I see what you were saying. Done.

+  // will be used with encrypted blocks.
+  bool IsCodecSupported(Codec codec,
+                        const std::string& mime_type_lower_case,
+                        bool is_encrypted) const;
 
   // Returns true if |codec| refers to a proprietary codec.
   bool IsCodecProprietary(Codec codec) const;
@@ -105,13 +134,17 @@ class MimeUtil {
 
   // Returns true if |mime_type_lower_case| has a default codec associated with
   // it and IsCodecSupported() returns true for that particular codec.
-  bool IsDefaultCodecSupportedLowerCase(
-      const std::string& mime_type_lower_case) const;
+  // |is_encrypted| means the codec will be used with encrypted blocks.
+  bool IsDefaultCodecSupportedLowerCase(const std::string& mime_type_lower_case,
+                                        bool is_encrypted) const;
 
-#if defined(OS_ANDROID)
   // Checks special Android only codec restrictions.
-  bool IsCodecSupportedOnAndroid(Codec codec) const;
-#endif
+  bool IsCodecSupportedOnAndroid(Codec codec,

[ddorwin, 2016/02/18 20:37:45]

It is weird that we have multiple "OnAndroid" functions being built on all
platforms. I understand the testing aspect, but it's probably just as much
cognitive burden as the ifdefs.

Also, platform_info_ is compiled on all platforms but never populated or used
except on Android. Shouldn't it either be used for all platforms, which would
add unnecessary complexity to most platforms, or not present at all?

After looking at the implementation, I propose a hybrid:
* Rename IsCodecSupportedOnAndroid IsCodecSupportedOnPlatform.
* Add a const PlatformInfo& parameter.
* Remove it from the class or make it static.
* #ifdef platform_info_ so it is only present on Android - the only platform
that populates or uses it.
* Inside the Android guards in the .cc file, pass platform_info_ to
IsCodecSupportedOnPlatform.
* Remove SetPlatformInfoForTests as it is no longer necessary.

Win-win-win: You get to test your code on all platforms, we don't have confusion
on non-Android platforms, and we remove test-only function(s).

Still, has_platform_decoders and using_unified_media_pipeline and the way they
are used are very Android-specific. :( Perhaps the struct should be
PlatformInfo.

Note: PlatformInfo might need to be moved back outside the class.

[DaleCurtis, 2016/02/19 01:35:47]

I had a similar idea this morning. Done.

+                                 const std::string& mime_type_lower_case,
+                                 bool is_encrypted) const;
+
+  // Indicates the support of various codecs within the platform.
+  PlatformInfo platform_info_;
 
   // A map of mime_types and hash map of the supported codecs for the mime_type.
   MediaFormatMappings media_format_map_;