Refactor VP9 codec string parsing

media/base/video_codecs.cc

Index: media/base/video_codecs.cc
diff --git a/media/base/video_codecs.cc b/media/base/video_codecs.cc
index 20d49b58b7c3529cc520c82fd6ff7115da996802..1fd2c45205075f08975ebd24f2752ae450cb59cc 100644
--- a/media/base/video_codecs.cc
+++ b/media/base/video_codecs.cc
@@ -84,6 +84,90 @@ std::string GetProfileName(VideoCodecProfile profile) {
   return "";
 }
 
+bool ParseNewStyleVp9CodecID(const std::string& codec_id,
+                             VideoCodecProfile* profile,
+                             uint8_t* level_idc) {
+  std::vector<std::string> fields = base::SplitString(
+      codec_id, ".", base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL);
+
+  // TODO(kqyang): The spec specifies 8 fields. We do not allow missing or extra
+  // fields. See crbug.com/667834.
+  if (fields.size() != 8)
+    return false;
+
+  if (fields[0] != "vp09")
+    return false;
+
+  std::vector<int> values;
+  for (size_t i = 1; i < fields.size(); ++i) {
+    // Missing value is not allowed.
+    if (fields[i] == "")
+      return false;
+    int value;
+    if (!base::StringToInt(fields[i], &value))
+      return false;
+    if (value < 0)
+      return false;
+    values.push_back(value);
+  }
+
+  const int profile_idc = values[0];
+  switch (profile_idc) {
+    case 0:
+      *profile = VP9PROFILE_PROFILE0;
+      break;
+    case 1:
+      *profile = VP9PROFILE_PROFILE1;
+      break;
+    case 2:
+      *profile = VP9PROFILE_PROFILE2;
+      break;
+    case 3:
+      *profile = VP9PROFILE_PROFILE3;
+      break;
+    default:
+      return false;
+  }
+

[ddorwin, 2016/11/29 23:48:51]

Do we have a separate bug to actually check the values? We should and included
it in a TODO here.

[kqyang, 2016/11/30 02:31:17]

No. We haven't decided whether we want to validate every fields. How about
settle crbug.com/667834 first and if needed we can open additional bugs later?

[ddorwin, 2016/12/02 18:55:54]

We can avoid commenting here, but there should be a bug tracking that decision
and any subsequent work. That bug should block the launch bug.

[kqyang, 2016/12/02 21:15:07]

We can use crbug.com/667834 to track the decision and subsequent work.

+  *level_idc = values[1];

[ddorwin, 2016/11/29 23:48:51]

An easy check is that this can't be less than 10, right? Probably with a comment
about the multiplication.

[kqyang, 2016/11/30 02:31:17]

This mapping (actual level = level / 10) does not seem to be available publicly
anywhere right now. 

I think we should include the mapping in VP9 in ISO-BMFF spec. We can then do
additional validations here after finalizing the spec.

[ddorwin, 2016/12/02 18:55:54]

This appears to be the one value where we don't check the value at all.
Regardless of whether we probe the codec implementation, we'd check the validity
of the string.
Maybe we should have a TODO to add checks for valid values after
crbug.com/667834 is resolved.

[kqyang, 2016/12/02 21:15:07]

Done.

+
+  const int bit_depth = values[2];
+  if (bit_depth != 8 && bit_depth != 10 && bit_depth != 12)
+    return false;
+
+  const int color_space = values[3];
+  if (color_space > 7)
+    return false;
+
+  const int chroma_subsampling = values[4];
+  if (chroma_subsampling > 3)
+    return false;
+
+  const int transfer_function = values[5];
+  if (transfer_function > 1)
+    return false;
+
+  const int video_full_range_flag = values[6];
+  if (video_full_range_flag > 1)
+    return false;
+
+  return true;
+}
+
+bool ParseLegacyVp9CodecID(const std::string& codec_id,
+                           VideoCodecProfile* profile,
+                           uint8_t* level_idc) {
+  if (codec_id == "vp9" || codec_id == "vp9.0") {
+    // Profile is not included in the codec string. Assuming profile 0 to be
+    // backward compatible.
+    *profile = VP9PROFILE_PROFILE0;
+    // Use 0 to indicate unknown level.
+    *level_idc = 0;
+    return true;
+  }
+  return false;
+}
+
 bool ParseAVCCodecId(const std::string& codec_id,
                      VideoCodecProfile* profile,
                      uint8_t* level_idc) {
@@ -292,14 +376,16 @@ VideoCodec StringToVideoCodec(const std::string& codec_id) {
     return kUnknownVideoCodec;
   VideoCodecProfile profile = VIDEO_CODEC_PROFILE_UNKNOWN;
   uint8_t level = 0;
-  if (ParseAVCCodecId(codec_id, &profile, &level))
-    return kCodecH264;
   if (codec_id == "vp8" || codec_id == "vp8.0")
     return kCodecVP8;
-  if (codec_id == "vp9" || codec_id == "vp9.0")
+  if (ParseNewStyleVp9CodecID(codec_id, &profile, &level) ||
+      ParseLegacyVp9CodecID(codec_id, &profile, &level)) {
     return kCodecVP9;
+  }
   if (codec_id == "theora")
     return kCodecTheora;
+  if (ParseAVCCodecId(codec_id, &profile, &level))
+    return kCodecH264;
 #if BUILDFLAG(ENABLE_HEVC_DEMUXING)
   if (ParseHEVCCodecId(codec_id, &profile, &level))
     return kCodecHEVC;