MSE: Relax H.264 Baseline mimetype profile recognition

net/base/mime_util.cc

 // Copyright (c) 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.
 
 #include <algorithm>
 #include <iterator>
 #include <map>
 #include <string>
 
 #include "base/containers/hash_tables.h"
@@ skipping 906 matching lines @@
 }
 
 void MimeUtil::RemoveProprietaryMediaTypesAndCodecsForTests() {
   for (size_t i = 0; i < arraysize(proprietary_media_types); ++i) {
     non_image_map_.erase(proprietary_media_types[i]);
     media_map_.erase(proprietary_media_types[i]);
   }
   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))
     return false;
 
   // Valid levels taken from Table A-1 in ISO-14496-10.
   // Essentially |level_str| is toHex(10 * level).
   return ((level >= 10 && level <= 13) ||
           (level >= 20 && level <= 22) ||
           (level >= 30 && level <= 32) ||
           (level >= 40 && level <= 42) ||
           (level >= 50 && level <= 51));
 }
 
 // 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.
 static bool ParseH264CodecID(const std::string& codec_id,
                              MimeUtil::Codec* codec,
                              bool* is_ambiguous) {
   // Make sure we have avc1.xxxxxx or avc3.xxxxxx
   if (codec_id.size() != 11 ||
       (!StartsWithASCII(codec_id, "avc1.", true) &&
        !StartsWithASCII(codec_id, "avc3.", true))) {
     return false;
   }
 
   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;
   } else if (profile == "6400") {
     *codec = MimeUtil::H264_HIGH;
   } else {
     *codec = MimeUtil::H264_BASELINE;
     *is_ambiguous = true;
     return true;
   }
@@ skipping 392 matching lines @@
   post_data->append("\r\n" + value + "\r\n");
 }
 
 void AddMultipartFinalDelimiterForUpload(const std::string& mime_boundary,
                                          std::string* post_data) {
   DCHECK(post_data);
   post_data->append("--" + mime_boundary + "--\r\n");
 }
 
 }  // namespace net