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 f61baff327918f816b84677136bc7cf8bb44e4bb..ed2e8165bdeab6445f2eb6d5959320301a751321 100644
--- a/media/base/mime_util_internal.h
+++ b/media/base/mime_util_internal.h
@@ -10,17 +10,31 @@
 #include <vector>
 
 #include "base/containers/hash_tables.h"
-#include "base/lazy_instance.h"
 #include "base/macros.h"
+#include "media/base/media_export.h"
 #include "media/base/mime_util.h"
 
 namespace media {
 namespace internal {
 
+#if defined(OS_ANDROID)
+// Platform configuration structure.  Controls which codecs are supported at

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

Is there a reason this isn't nested in the class?

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

Done.

+// runtime.  Also used by tests to simulate platform differences.
+struct PlatformCodecInfo {
+  bool has_platform_decoder;

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

nit: decoder*s*?
Since it's Android, should we just say has_mediacodec?
Also, "platform" is redundant here and makes it look like the last three are
missing it.

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

I'd like to keep the name consistent with what we have in media.h, which does
not refer to MediaCodec.  Ditto for keeping "platform" in the name.  I'm opening
to alternative names though. To appease your concerns I renamed
has_{opus,vp8,vp9} to supports_() which I think makes the distinction clearer.

+  bool has_unified_media_pipeline;

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

nit: This isn't really a platform property. Perhaps change it to use_... and
move it to the top or bottom.

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

Done.

+  bool has_opus;
+  bool has_vp8;
+  bool has_vp9;

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

define default values?

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

Done.

+};
+#endif
+
 // 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();
+
   enum Codec {
     INVALID_CODEC,
     PCM,
@@ -41,7 +55,8 @@ class MimeUtil {
     HEVC_MAIN,
     VP8,
     VP9,
-    THEORA
+    THEORA,
+    LAST_CODEC = THEORA
   };
 
   // See mime_util.h for more information on these methods.
@@ -49,15 +64,25 @@ 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();
 
- private:
-  friend struct base::DefaultLazyInstanceTraits<MimeUtil>;
+  void SetPlatformCodecInfoForTests(const PlatformCodecInfo& info);

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

Move inside #ifdef.

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

Actually instead I removed all of the #ifdefs from here since this is code that
can be tested on any platform; which makes it easier to debug and find issues. I
think we should always strive for less-ifdefs when possible especially when it
comes to testing.

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

See my comment in the next PS.

 
+#if defined(OS_ANDROID)
+  // 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);
+  }
+#endif
+
+ private:
   typedef base::hash_set<int> CodecSet;
   typedef std::map<std::string, CodecSet> MediaFormatMappings;
   struct CodecEntry {
@@ -68,18 +93,19 @@ class MimeUtil {
   };
   typedef std::map<std::string, CodecEntry> StringToCodecMappings;
 
-  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 when contained in |mime_type_lower_case|. MayBeSupported is
+  // returned if at least one codec ID in |codecs| is ambiguous but all the
+  // codecs are supported. IsNotSupported is returned if at least one codec ID

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

To be absolutely clear:
... supported in |mime_type_lower_case|. IsNotSupported is returned if
|mime_type_lower_case| is not supported or at least one is not supported in
|mime_type_lower_case|.

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

Done.

+  // is not supported. |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,

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

consistency nit: Elsewhere, the mime_type parameter is first.

The fact that this function name is asking about "Codecs" makes it less clear
that this would be a better ordering, especially since we also have
|supported_codecs|, which is inconsistent with the others. Perhaps that should
be just before |is_encrypted|.

Anyway, up to you.

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

I intentionally left codecs first here since that's nominally what the method is
about. Per your previous comment I've made |is_encrypted| the least important :)

+                                  bool is_encrypted) const;
 
   // Converts a codec ID into an Codec enum value and indicates
   // whether the conversion was ambiguous.
@@ -93,10 +119,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/17 21:18:39]

You dropped part of the text from the Note. It will return false for proprietary
codes regardless of platform codec support.

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

Yes, I intentionally dropped that since the new wording is clearer. There's no
need to talk about platforms if |allow_proprietary_codecs_|=false will always
force it to return false :)

+  // will be used with encrypted blocks.
+  bool IsCodecSupported(Codec codec,
+                        const std::string& mime_type_lower_case,

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

ditto on order consistency

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

Acknowledged.

+                        bool is_encrypted) const;
 
   // Returns true if |codec| refers to a proprietary codec.
   bool IsCodecProprietary(Codec codec) const;
@@ -109,12 +138,18 @@ 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;
+  bool IsCodecSupportedOnAndroid(Codec codec,
+                                 const std::string& mime_type_lower_case,

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

ditto on order consistency

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

Acknowledged.

+                                 bool is_encrypted) const;
+
+  // Indicates the support of various codecs within the platform.
+  PlatformCodecInfo platform_info_;
 #endif
 
   // A map of mime_types and hash map of the supported codecs for the mime_type.