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

media/cdm/cenc_utils.cc

Index: media/cdm/cenc_utils.cc
diff --git a/media/cdm/cenc_utils.cc b/media/cdm/cenc_utils.cc
index 1e33cca361337898f0371281632af05e8b7d0759..8021d8fb9c286d01ecb5c5357713d5044a97de5b 100644
--- a/media/cdm/cenc_utils.cc
+++ b/media/cdm/cenc_utils.cc
@@ -15,13 +15,6 @@ namespace media {
 // 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) {
@@ -70,27 +63,29 @@ bool ValidatePsshInput(const std::vector<uint8_t>& input) {
   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,