Refactor VP9 codec string parsing

media/base/video_codecs.cc

 // Copyright 2015 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 "media/base/video_codecs.h"
 
 #include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
@@ skipping 66 matching lines @@
       return "vp9 profile1";
     case VP9PROFILE_PROFILE2:
       return "vp9 profile2";
     case VP9PROFILE_PROFILE3:
       return "vp9 profile3";
   }
   NOTREACHED();
   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) {
   // Make sure we have avc1.xxxxxx or avc3.xxxxxx , where xxxxxx are hex digits
   if (!base::StartsWith(codec_id, "avc1.", base::CompareCase::SENSITIVE) &&
       !base::StartsWith(codec_id, "avc3.", base::CompareCase::SENSITIVE)) {
     return false;
   }
   uint32_t elem = 0;
   if (codec_id.size() != 11 ||
@@ skipping 188 matching lines @@
 }
 #endif
 
 VideoCodec StringToVideoCodec(const std::string& codec_id) {
   std::vector<std::string> elem = base::SplitString(
       codec_id, ".", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
   if (elem.empty())
     return kUnknownVideoCodec;
   VideoCodecProfile profile = VIDEO_CODEC_PROFILE_UNKNOWN;
   uint8_t level = 0;
+  if (codec_id == "vp8" || codec_id == "vp8.0")
+    return kCodecVP8;
+  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 (codec_id == "vp8" || codec_id == "vp8.0")
-    return kCodecVP8;
-  if (codec_id == "vp9" || codec_id == "vp9.0")
-    return kCodecVP9;
-  if (codec_id == "theora")
-    return kCodecTheora;
 #if BUILDFLAG(ENABLE_HEVC_DEMUXING)
   if (ParseHEVCCodecId(codec_id, &profile, &level))
     return kCodecHEVC;
 #endif
   return kUnknownVideoCodec;
 }
 
 }  // namespace media