MSE: Relax H.264 Baseline mimetype profile recognition

net/base/mime_util.cc

Index: net/base/mime_util.cc
diff --git a/net/base/mime_util.cc b/net/base/mime_util.cc
index 37e630fb031a1c0b2fa1eff1f29bc79c919a7000..8f7b51422aa95202197dbbe9656a5d77b6215e4e 100644
--- a/net/base/mime_util.cc
+++ b/net/base/mime_util.cc
@@ -924,6 +924,20 @@ void MimeUtil::RemoveProprietaryMediaTypesAndCodecsForTests() {
   allow_proprietary_codecs_ = false;
 }
 
+// Returns true iff |profile_str| conforms to hex string "42y0": y >= 8. Taken

[DaleCurtis, 2014/10/03 21:54:07]

Since this is just 8,9 -- why not explicitly enumerate them instead of using a
function?

[wolenetz, 2014/10/03 22:27:52]

(replying again, in context):
This is actually 8,9,A,...,F (hex string).

[DaleCurtis, 2014/10/03 22:33:56]

Ahh, sorry the CL description makes it clear, but this is not clear that y can
be [8..F].  Can you clarify the comment?

[wolenetz, 2014/10/03 22:42:10]

Done.

[xhwang, 2014/10/03 23:08:06]

It's hex, so it can be 'c', 'e' etc.

+// from Annex A and constraint_set0 in ISO-14496-10.

[xhwang, 2014/10/03 23:08:06]

Can you mention 7.3.2.1  and 7.4.2.1 in the spec. These are key (in addition to
Annex A) to the understanding of this format.

Also, it'd be great if you could summarize the spec about how the format is
defined. It's always a pain to read this code without looking at the spec.

Also, as discussed offline, the spec is hard to understand. What we "choose" to
do is basically check the "42" part and make sure constraint_set0 is set.

[wolenetz, 2014/10/04 01:36:40]

Done.

+static bool IsValidH264BaselineProfile(const std::string& profile_str) {
+  uint32 constraint_set_bits;
+  if (profile_str.size() != 4 ||
+      profile_str.substr(0, 2) != "42" ||
+      profile_str.substr(3, 1) != "0" ||

[xhwang, 2014/10/03 23:08:06]

The substr is a bit overkill; will this work?

return 
profile_str[0] == '4' &&
profile_str[1] == '2' &&
profile_str[2] & 0x8  &&
!(profile_str[2] & 0xF1) &&
profile_str[3] == '0';

[xhwang, 2014/10/03 23:10:28]

Sorry, you still need to convert string to int, for profile_str[2]... but you
shouldn't really need substr...

[wolenetz, 2014/10/03 23:51:04]

Masking with 0xF1 instead of 0xFF means that we would reply "maybe", not
"probably" for an otherwise H.264 baseline profile that has constraint_set3_flag
set. I'm not clear that constraint_set3_flag being set disqualifies a bitstream
from claiming baseline profile (assuming it has profile_idc=0x42=66 and
constraint_set0_flag set). Am I missing something in why you chose 0xF1 here?

[wolenetz, 2014/10/04 01:36:40]

I'm keeping to just >= 8 (which requires constraint_set0_flag, and ignores the
other 3 flags).

+      !base::HexStringToUInt(profile_str.substr(2, 1), &constraint_set_bits)) {
+    return false;
+  }
+
+  return (constraint_set_bits & 0x8);

[DaleCurtis, 2014/10/03 22:33:56]

Just use >= 8. Let the compiler handle such an optimization :)

[wolenetz, 2014/10/03 22:42:10]

Done.

+}
+
 static bool IsValidH264Level(const std::string& level_str) {
   uint32 level;
   if (level_str.size() != 2 || !base::HexStringToUInt(level_str, &level))
@@ -939,9 +953,9 @@ static bool IsValidH264Level(const std::string& level_str) {
 }
 
 // Handle parsing H.264 codec IDs as outlined in RFC 6381

[xhwang, 2014/10/03 23:08:06]

The suffix strings are really defined in ISO-14496-10...

[wolenetz, 2014/10/04 01:36:40]

Done.

-//   avc1.42E0xx - H.264 Baseline
-//   avc1.4D40xx - H.264 Main
-//   avc1.6400xx - H.264 High
+//   avc1.42y0xx, y >= 8 - H.264 Baseline
+//   avc1.4D40xx         - H.264 Main
+//   avc1.6400xx         - H.264 High
 //
 //   avc1.xxxxxx & avc3.xxxxxx are considered ambiguous forms that
 //   are trying to signal H.264 Baseline.
@@ -956,7 +970,7 @@ static bool ParseH264CodecID(const std::string& codec_id,
   }
 
   std::string profile = StringToUpperASCII(codec_id.substr(5, 4));
-  if (profile == "42E0") {
+  if (IsValidH264BaselineProfile(profile)) {
     *codec = MimeUtil::H264_BASELINE;
   } else if (profile == "4D40") {
     *codec = MimeUtil::H264_MAIN;