Fix: Adding list of supported codecs for MP4 containers

content/renderer/renderer_webkitplatformsupport_impl.cc

Index: content/renderer/renderer_webkitplatformsupport_impl.cc
diff --git a/content/renderer/renderer_webkitplatformsupport_impl.cc b/content/renderer/renderer_webkitplatformsupport_impl.cc
index 91906df8dbfc6a5ea5e8767a53602f41fe9c0f80..94c067ef2e3a198ad7713d9c8b16463d559af0d0 100644
--- a/content/renderer/renderer_webkitplatformsupport_impl.cc
+++ b/content/renderer/renderer_webkitplatformsupport_impl.cc
@@ -419,8 +419,7 @@ RendererWebKitPlatformSupportImpl::MimeRegistry::supportsMediaMIMEType(
     std::string key_system_ascii =
         GetUnprefixedKeySystemName(base::UTF16ToASCII(key_system));
     std::vector<std::string> strict_codecs;
-    bool strip_suffix = !net::IsStrictMediaMimeType(mime_type_ascii);
-    net::ParseCodecString(ToASCIIOrEmpty(codecs), &strict_codecs, strip_suffix);
+    net::ParseCodecString(ToASCIIOrEmpty(codecs), &strict_codecs, true);

[amogh.bihani, 2014/05/16 11:25:29]

ddorwin@
After adding mp4 containers in strict media mime type list, I had to remove this
condition as EME does not consider extended codecs[1].
However, I think we do not need to change any thing there. When
IsSupportedKeySystemWithMediaMimeType() returns true, the code has to go through
line 437. Here the logic is implemented, so from the overall browser's
perspective, we might end up executing same commands twice.

Is it fine to keep that part as is or do we need to make changes there as well?
If yes, can we do that in another CL? I am not much familiar with that part and
might need some time.
----------
[1]
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/m...

[ddorwin, 2014/05/16 17:26:24]

This will allow "vp8.1", which it shouldn't.

EME should handle extended codecs in the same way as the rest of Chromium. This
specific code will eventually be removed, so it's not worth addressing now, but
we need to fix the permanent code elsewhere. Please file a bug.

Also, please add a test for "vp8.1" that demonstrates this failure and reference
the bug. (If we don't have a non-EME test for "vp8.1", we should add that too.

[AmoghBihani, 2014/05/16 18:13:51]

Well the test for vp8.1 will not pass as we next condition, i.e.
IsSupportedStrictMediaMimeType will return IsNotSupported.

So, IsSupportedKeySystemWithMediaMimeType is temporary or it being handled here
is temporary?

[ddorwin, 2014/05/16 18:49:49]

Okay. It would be good to add tests to make sure we don't regress. :)

IsSupportedKeySystemWithMediaMimeType() will live on, but it will be removed
from here. The permanent call is in supportsEncryptedMediaMIMEType() at line
485.

 
     if (!IsSupportedKeySystemWithMediaMimeType(
             mime_type_ascii, strict_codecs, key_system_ascii)) {
@@ -435,14 +434,8 @@ RendererWebKitPlatformSupportImpl::MimeRegistry::supportsMediaMIMEType(
     // Check if the codecs are a perfect match.
     std::vector<std::string> strict_codecs;
     net::ParseCodecString(ToASCIIOrEmpty(codecs), &strict_codecs, false);
-    if (net::IsSupportedStrictMediaMimeType(mime_type_ascii, strict_codecs))
-      return IsSupported;
-
-    // We support the container, but no codecs were specified.
-    if (codecs.isNull())
-      return MayBeSupported;
-
-    return IsNotSupported;
+    return static_cast<WebMimeRegistry::SupportsType> (
+        net::IsSupportedStrictMediaMimeType(mime_type_ascii,strict_codecs));

[ddorwin, 2014/05/16 17:26:24]

nit: space after ','

   }

[ddorwin, 2014/05/16 17:26:24]

Where is the "no codecs" case handled now?

[AmoghBihani, 2014/05/16 18:13:51]

That has gone in mime_util.cc line 820 of this patch.

 
   // If we don't recognize the codec, it's possible we support it.