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

media/base/mime_util_internal.h

 // Copyright 2012 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.
 
 #ifndef MEDIA_BASE_MIME_UTIL_INTERNAL_H_
 #define MEDIA_BASE_MIME_UTIL_INTERNAL_H_
 
 #include <map>
 #include <string>
 #include <vector>
 
 #include "base/containers/hash_tables.h"
 #include "base/macros.h"
+#include "media/base/media_export.h"
 #include "media/base/mime_util.h"
 
 namespace media {
 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();
 
   enum Codec {
     INVALID_CODEC,
     PCM,
     MP3,
     AC3,
     EAC3,
     MPEG2_AAC_LC,
     MPEG2_AAC_MAIN,
     MPEG2_AAC_SSR,
     MPEG4_AAC_LC,
     MPEG4_AAC_SBR_v1,
     MPEG4_AAC_SBR_PS_v2,
     VORBIS,
     OPUS,
     H264,
     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.
   bool IsSupportedMediaMimeType(const std::string& mime_type) const;
   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;
   struct CodecEntry {
     CodecEntry() : codec(INVALID_CODEC), is_ambiguous(true) {}
     CodecEntry(Codec c, bool ambiguous) : codec(c), is_ambiguous(ambiguous) {}
     Codec codec;
     bool is_ambiguous;
   };
   typedef std::map<std::string, CodecEntry> StringToCodecMappings;
 
   // 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.
   // Returns true if this method was able to map |codec_id| to a specific
   // Codec enum value. |codec| and |is_ambiguous| are only valid if true
   // is returned. Otherwise their value is undefined after the call.
   // |is_ambiguous| is true if |codec_id| did not have enough information to
   // unambiguously determine the proper Codec enum value. If |is_ambiguous|
   // is true |codec| contains the best guess for the intended Codec enum value.
   bool StringToCodec(const std::string& codec_id,
                      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;
 
   // Returns true and sets |*default_codec| if |mime_type| has a  default codec
   // associated with it. Returns false otherwise and the value of
   // |*default_codec| is undefined.
   bool GetDefaultCodecLowerCase(const std::string& mime_type_lower_case,
                                 Codec* default_codec) const;
 
   // 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_;
 
   // Keeps track of whether proprietary codec support should be
   // advertised to callers.
   bool allow_proprietary_codecs_;
 
   // Lookup table for string compare based string -> Codec mappings.
   StringToCodecMappings string_to_codec_map_;
 
   DISALLOW_COPY_AND_ASSIGN(MimeUtil);
 };
 
 }  // namespace internal
 }  // namespace media
 
 #endif  // MEDIA_BASE_MIME_UTIL_INTERNAL_H_