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/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,
     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_BASELINE,
     H264_MAIN,
     H264_HIGH,
     HEVC_MAIN,
     VP8,
     VP9,
-    THEORA
+    THEORA,
+    LAST_CODEC = THEORA
   };
 
   // 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 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:
-  friend struct base::DefaultLazyInstanceTraits<MimeUtil>;
-
   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;
 
-  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.
   // 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/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;
 
   // 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;
+  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.
   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_