Media: Do not support new codecs with legacy MIME type names.

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 e9b38f1191801c55e0e7d4e7092cfaf7a1c92140..59b7c595963bc700a0b258ac8abfd0fa6e41c10e 100644
--- a/media/base/mime_util_internal.cc
+++ b/media/base/mime_util_internal.cc
@@ -31,22 +31,6 @@ struct MediaFormat {
 // Strings used as the |codecs_list| only need one valid unambiguous variant for
 // each supported MimeUtil::Codec enum value. Each codec string is parsed and
 // mapped to corresponding MimeUtil::Codec value. See https://crbug.com/461009.
-#if defined(USE_PROPRIETARY_CODECS)
-static const char kMP4AudioCodecsExpression[] =
-#if BUILDFLAG(ENABLE_AC3_EAC3_AUDIO_DEMUXING)
-    "ac-3,ec-3,"  // AC-3 and E-AC-3.
-#endif
-    "mp4a.66,mp4a.69,mp4a.40.2";  // MPEG-2 AAC, MP3, and MPEG-4 AAC.
-static const char kMP4VideoCodecsExpression[] =
-    "avc1.42E00A,"
-#if BUILDFLAG(ENABLE_HEVC_DEMUXING)
-    "hev1.1.6.L93.B0,"
-#endif
-#if BUILDFLAG(ENABLE_AC3_EAC3_AUDIO_DEMUXING)
-    "ac-3,ec-3,"  // AC-3 and E-AC-3.
-#endif
-    "mp4a.66,mp4a.69,mp4a.40.2";  // MPEG-2 AAC, MP3, and MPEG-4 AAC.
-#endif  // USE_PROPRIETARY_CODECS
 
 // A list of media types (https://en.wikipedia.org/wiki/Media_type) and
 // corresponding media codecs supported by these types/containers.
@@ -72,18 +56,7 @@ static const MediaFormat kFormatCodecMappings[] = {
     {"audio/mp3", PROPRIETARY, ""},
     {"audio/x-mp3", PROPRIETARY, ""},
     {"audio/aac", PROPRIETARY, ""},  // AAC / ADTS
-    {"audio/mp4", PROPRIETARY, kMP4AudioCodecsExpression},
-    {"audio/x-m4a", PROPRIETARY, kMP4AudioCodecsExpression},
-    {"video/mp4", PROPRIETARY, kMP4VideoCodecsExpression},
-    {"video/x-m4v", PROPRIETARY, kMP4VideoCodecsExpression},
-#if BUILDFLAG(ENABLE_MSE_MPEG2TS_STREAM_PARSER)
-    {"video/mp2t", PROPRIETARY, kMP4VideoCodecsExpression},
-#endif
-#if defined(OS_ANDROID)
-    // HTTP Live Streaming (HLS)
-    {"application/x-mpegurl", PROPRIETARY, kMP4VideoCodecsExpression},
-    {"application/vnd.apple.mpegurl", PROPRIETARY, kMP4VideoCodecsExpression}
-#endif
+// Most MPEG MIME types are added in InitializeMimeTypeMaps().

[ddorwin, 2016/03/17 20:52:54]

We now have this information scattered. Should we move this entire table to
InitializeMimeTypeMaps? it seems better to have them together. Also, this would
allow us to clean up the OS_ANDROID code above in a similar way.

 #endif  // USE_PROPRIETARY_CODECS
 };
 
@@ -101,7 +74,7 @@ static const CodecIDMappings kUnambiguousCodecStringMap[] = {
     // avc1/avc3.XXXXXX may be unambiguous; handled by ParseAVCCodecId().
     // hev1/hvc1.XXXXXX may be unambiguous; handled by ParseHEVCCodecID().
     {"mp3", MimeUtil::MP3},
-    // Following is the list of RFC 6381 compliant audio codecs:
+    // Following is the list of RFC 6381 compliant audio codec strings:
     //   mp4a.66     - MPEG-2 AAC MAIN
     //   mp4a.67     - MPEG-2 AAC LC
     //   mp4a.68     - MPEG-2 AAC SSR
@@ -299,7 +272,6 @@ SupportsType MimeUtil::AreSupportedCodecs(
 }
 
 void MimeUtil::InitializeMimeTypeMaps() {
-// Initialize the supported media types.
 #if defined(USE_PROPRIETARY_CODECS)
   allow_proprietary_codecs_ = true;
 #endif
@@ -316,21 +288,68 @@ void MimeUtil::InitializeMimeTypeMaps() {
 
   // Initialize the supported media formats.
   for (size_t i = 0; i < arraysize(kFormatCodecMappings); ++i) {
-    std::vector<std::string> mime_type_codecs;
-    ParseCodecString(kFormatCodecMappings[i].codecs_list, &mime_type_codecs,
-                     false);
-
-    CodecSet codecs;
-    for (size_t j = 0; j < mime_type_codecs.size(); ++j) {
-      Codec codec = INVALID_CODEC;
-      bool is_ambiguous = true;
-      CHECK(StringToCodec(mime_type_codecs[j], &codec, &is_ambiguous));
-      DCHECK(!is_ambiguous);
-      codecs.insert(codec);
-    }
+    AddContainerWithCodecs(kFormatCodecMappings[i].mime_type,
+                           kFormatCodecMappings[i].codecs_list);
+  }
 
-    media_format_map_[kFormatCodecMappings[i].mime_type] = codecs;
+#if defined(USE_PROPRIETARY_CODECS)

[DaleCurtis, 2016/03/17 21:00:21]

Could have an InitializeProprietaryCodecMimeTypeMaps() here.

[ddorwin, 2016/03/17 21:44:25]

Yes, though that would separate the lists of types even further, and it may not
always be appropriate (i.e. https://crbug.com/327115).

+  // Initialize the supported media formats not in kFormatCodecMappings.
+  // TODO: Should we move all mappings to this style?
+  const std::string aac = "mp4a.66,mp4a.40.2";  // MPEG-2 AAC and MPEG-4 AAC.

[DaleCurtis, 2016/03/17 21:00:21]

You can keep these as const char* up above and just initialize the std::string
versions for globing here; that might make the readability better.

[ddorwin, 2016/03/17 21:44:25]

I've uploaded this change, but I plan to revert - I think this is worse because
it further separates those literals from others and I have to add std::string()
in places.

This is really a minor issue, though, because https://crbug.com/461009 should
eliminate the need for theses.

+  const std::string avc = "avc1.42E00A";
+  const std::string avc_and_aac = avc + "," + aac;
+  const std::string mp4_audio_codecs =
+#if BUILDFLAG(ENABLE_AC3_EAC3_AUDIO_DEMUXING)
+      "ac-3,ec-3,"  // AC-3 and E-AC-3.
+#endif
+      "mp4a.69," +  // MP3
+      aac;
+  const std::string mp4_video_codecs =
+#if BUILDFLAG(ENABLE_HEVC_DEMUXING)
+      "hev1.1.6.L93.B0," +
+#endif
+      avc;
+  const std::string mp4_codecs = mp4_video_codecs + "," + mp4_audio_codecs;
+
+  // TODO: Fix before committing. This breaks
+  // RemoveProprietaryMediaTypesAndCodecs() because the containers are not in
+  // kFormatCodecMappings with PROPRIETARY.

[ddorwin, 2016/03/17 20:52:54]

I will add this parameter to AddContainerWithCodecs() and use it to build a
vector of proprietary MIME types to be removed (rather than iterating through
kFormatCodecMappings to identify them).

+  AddContainerWithCodecs("audio/mp4", mp4_audio_codecs);

[ddorwin, 2016/03/17 20:52:55]

Each entry remaining in kFormatCodecMappings would look something like this, but
with a literal. Those should be a lot cleaner than this.

Still, the strings above will be in the middle of such entries unless we want
multiple #ifdef's.

+  AddContainerWithCodecs("video/mp4", mp4_codecs);
+  // These strings are supported for backwards compatibility only and thus only
+  // support the codecs needed for compatibility.
+  AddContainerWithCodecs("audio/x-m4a", aac);
+  AddContainerWithCodecs("video/x-m4v", avc_and_aac);
+
+#if BUILDFLAG(ENABLE_MSE_MPEG2TS_STREAM_PARSER)
+  // TODO(ddorwin): Exactly which codecs should be supported?
+  AddContainerWithCodecs("video/mp2t", mp4_codecs);
+#endif
+#if defined(OS_ANDROID)
+  // HTTP Live Streaming (HLS)
+  AddContainerWithCodecs("application/x-mpegurl", avc_and_aac);
+  AddContainerWithCodecs("application/vnd.apple.mpegurl", avc_and_aac);
+#endif
+#endif  // USE_PROPRIETARY_CODECS
+}
+
+// TODO(ddorwin): Replace |codecs_list| with a vector of MimeUtil::Codec values.
+// See https://crbug.com/461009
+void MimeUtil::AddContainerWithCodecs(const std::string& mime_type,
+                                      const std::string& codecs_list) {
+  std::vector<std::string> mime_type_codecs;
+  ParseCodecString(codecs_list, &mime_type_codecs, false);
+
+  CodecSet codecs;
+  for (size_t j = 0; j < mime_type_codecs.size(); ++j) {
+    Codec codec = INVALID_CODEC;
+    bool is_ambiguous = true;
+    CHECK(StringToCodec(mime_type_codecs[j], &codec, &is_ambiguous));
+    DCHECK(!is_ambiguous);
+    codecs.insert(codec);
   }
+
+  media_format_map_[mime_type] = codecs;
 }
 
 bool MimeUtil::IsSupportedMediaMimeType(const std::string& mime_type) const {
@@ -408,6 +427,7 @@ SupportsType MimeUtil::IsSupportedMediaFormat(
                             mime_type_lower_case, is_encrypted);
 }
 
+// TODO: Stop using kFormatCodecMappings since it no longer contains all types.
 void MimeUtil::RemoveProprietaryMediaTypesAndCodecs() {
   for (size_t i = 0; i < arraysize(kFormatCodecMappings); ++i)
     if (kFormatCodecMappings[i].format_type == PROPRIETARY)