Have GetKeyIds() return the key IDs from the first matching 'pssh' box

media/cdm/cenc_utils.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/cdm/cenc_utils.h"
 
 #include "base/stl_util.h"
 #include "media/formats/mp4/box_definitions.h"
 #include "media/formats/mp4/box_reader.h"
 
 namespace media {
 
 // The initialization data for encrypted media files using the ISO Common
 // Encryption ('cenc') protection scheme may contain one or more protection
 // system specific header ('pssh') boxes.
 // ref: https://w3c.github.io/encrypted-media/cenc-format.html
 
-// SystemID for the Common System.
-// https://w3c.github.io/encrypted-media/cenc-format.html#common-system
-const uint8_t kCommonSystemId[] = { 0x10, 0x77, 0xef, 0xec,
-                                    0xc0, 0xb2, 0x4d, 0x02,
-                                    0xac, 0xe3, 0x3c, 0x1e,
-                                    0x52, 0xe2, 0xfb, 0x4b };
-
 static bool ReadAllPsshBoxes(

[ddorwin, 2015/06/25 21:18:12]

Document this function. For example, returns true if |input| contains only valid
'pssh' boxes.

[jrummell, 2015/06/25 22:35:58]

Done.

     const std::vector<uint8_t>& input,
     std::vector<mp4::FullProtectionSystemSpecificHeader>* pssh_boxes) {
   DCHECK(!input.empty());
 
   // Verify that |input| contains only 'pssh' boxes. ReadAllChildren() is
   // templated, so it checks that each box in |input| matches the box type of
   // the parameter (in this case mp4::ProtectionSystemSpecificHeader is a
   // 'pssh' box). mp4::ProtectionSystemSpecificHeader doesn't validate the
   // 'pssh' contents, so this simply verifies that |input| only contains
   // 'pssh' boxes and nothing else.
@@ skipping 27 matching lines @@
 
 bool ValidatePsshInput(const std::vector<uint8_t>& input) {
   // No 'pssh' boxes is considered valid.
   if (input.empty())
     return true;
 
   std::vector<mp4::FullProtectionSystemSpecificHeader> children;
   return ReadAllPsshBoxes(input, &children);
 }
 
-bool GetKeyIdsForCommonSystemId(const std::vector<uint8_t>& input,
-                                KeyIdList* key_ids) {
+bool GetKeyIds(const std::vector<uint8_t>& input,
+               const std::vector<uint8_t>& system_id,

[ddorwin, 2015/06/25 21:18:12]

It's fine to have this generic implementation internally, but would we ever want
to call this with any other system_id? Exposing it sort of implies there is a
reason to be looking at the key IDs for other systems.

[jrummell, 2015/06/25 22:35:58]

Added in case some future implementation wants to pass the key IDs in the KIDs
field rather than the data.

[ddorwin, 2015/06/25 22:49:23]

I think this adds the risk of YAGNI and misuse, both of which seem more likely.
As I said, I think this internal refactoring is fine, but we should probably
make this method static and expose GetKeyIdsForCencCommonSystem() or something
like that. _If_ we need the generic functionality, we can just make this not
static.

Also, aes_decryptor should not need to know the Common SystemID as it does
currently in this CL.

[jrummell, 2015/06/30 00:49:12]

Done.

+               KeyIdList* key_ids) {
+  // If there are no 'pssh' boxes then no key IDs found.
+  if (input.empty())
+    return false;
+
+  std::vector<mp4::FullProtectionSystemSpecificHeader> children;
+  if (!ReadAllPsshBoxes(input, &children))
+    return false;
+
+  // Check all children for an appropriate 'pssh' box, returning the
+  // key IDs found.
   KeyIdList result;
-  std::vector<uint8_t> common_system_id(
-      kCommonSystemId, kCommonSystemId + arraysize(kCommonSystemId));
-
-  if (!input.empty()) {
-    std::vector<mp4::FullProtectionSystemSpecificHeader> children;
-    if (!ReadAllPsshBoxes(input, &children))
-      return false;
-
-    // Check all children for an appropriate 'pssh' box, concatenating any
-    // key IDs found.
-    for (const auto& child : children) {
-      if (child.system_id == common_system_id && child.key_ids.size() > 0)
-        result.insert(result.end(), child.key_ids.begin(), child.key_ids.end());
+  for (const auto& child : children) {
+    if (child.system_id == system_id) {
+      key_ids->assign(child.key_ids.begin(), child.key_ids.end());
+      return key_ids->size() > 0;
     }
   }
 
-  key_ids->swap(result);
-  return key_ids->size() > 0;
+  // No matching 'pssh' box found.
+  return false;
 }
 
 bool GetPsshData(const std::vector<uint8_t>& input,
                  const std::vector<uint8_t>& system_id,
                  std::vector<uint8_t>* pssh_data) {
   if (input.empty())
     return false;
 
   std::vector<mp4::FullProtectionSystemSpecificHeader> children;
   if (!ReadAllPsshBoxes(input, &children))
     return false;
 
   // Check all children for an appropriate 'pssh' box, returning |data| from
   // the first one found.
   for (const auto& child : children) {
     if (child.system_id == system_id) {
       pssh_data->assign(child.data.begin(), child.data.end());
       return true;
     }
   }
 
   // No matching 'pssh' box found.
   return false;
 }
 
 }  // namespace media