Add MediaDrmBridge::AddKeySystemUuidMapping().

media/base/android/media_drm_bridge.cc

 // Copyright (c) 2013 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/android/media_drm_bridge.h"
 
 #include <algorithm>
 
 #include "base/android/build_info.h"
 #include "base/android/jni_array.h"
 #include "base/android/jni_string.h"
 #include "base/callback_helpers.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "base/strings/string_util.h"
 #include "jni/MediaDrmBridge_jni.h"
 #include "media/base/android/media_player_manager.h"
 
 #include "widevine_cdm_version.h"  // In SHARED_INTERMEDIATE_DIR.
 
 using base::android::AttachCurrentThread;
 using base::android::ConvertUTF8ToJavaString;
 using base::android::ConvertJavaStringToUTF8;
 using base::android::JavaByteArrayToByteVector;
 using base::android::ScopedJavaLocalRef;
 
 namespace media {
 
-static uint32 ReadUint32(const uint8_t* data) {
+namespace {

[xhwang, 2014/05/15 15:10:29]

In media code, we use "static" a lot. Some people prefer this to anonymous
namespaces.

[ycheo (away), 2014/05/16 04:53:38]

Got it. I'll revert it to static.

+
+uint32 ReadUint32(const uint8_t* data) {
   uint32 value = 0;
   for (int i = 0; i < 4; ++i)
     value = (value << 8) | data[i];
   return value;
 }
 
-static uint64 ReadUint64(const uint8_t* data) {
+uint64 ReadUint64(const uint8_t* data) {
   uint64 value = 0;
   for (int i = 0; i < 8; ++i)
     value = (value << 8) | data[i];
   return value;
 }
 
 // The structure of an ISO CENC Protection System Specific Header (PSSH) box is
 // as follows. (See ISO/IEC FDIS 23001-7:2011(E).)
 // Note: ISO boxes use big-endian values.
 //
@@ skipping 10 matching lines @@
 const int kBoxLargeSizeSize = 8;
 const int kPsshVersionFlagSize = 4;
 const int kPsshSystemIdSize = 16;
 const int kPsshDataSizeSize = 4;
 const uint32 kTencType = 0x74656e63;
 const uint32 kPsshType = 0x70737368;
 const uint8 kWidevineUuid[16] = {
     0xED, 0xEF, 0x8B, 0xA9, 0x79, 0xD6, 0x4A, 0xCE,
     0xA3, 0xC8, 0x27, 0xDC, 0xD5, 0x1D, 0x21, 0xED };
 
-static std::vector<uint8> GetUUID(const std::string& key_system) {
-  // For security reasons, we only do exact string comparisons here - we don't
-  // try to parse the |key_system| in any way.
-  if (key_system == kWidevineKeySystem) {
-    return std::vector<uint8>(kWidevineUuid,
-                              kWidevineUuid + arraysize(kWidevineUuid));
+typedef std::pair<std::string, std::vector<uint8> > KeySystemUuidPair;
+// Use std::vector<>, not std::map<> since the size of the table will be
+// expected to be resonably small.
+typedef std::vector<KeySystemUuidPair> KeySystemUuidTable;

[xhwang, 2014/05/15 15:10:29]

Since we use UUID a lot now, how about

typedef std::vector<uint8> UUID?

Then can you use a base::hash_map<std::string, UUID> instead of vector of pairs?

[ddorwin, 2014/05/15 19:00:45]

This would also allow s/Table/Map/. "Map" is used a lot in the media code. I
don't think "Table" is.

[ycheo (away), 2014/05/16 04:53:38]

Done.

[ycheo (away), 2014/05/16 04:53:38]

Done.

+KeySystemUuidTable* key_system_uuid_table = NULL;

[xhwang, 2014/05/15 15:10:29]

global variable needs to be named as g_foo

[ycheo (away), 2014/05/16 04:53:38]

Done.

+
+void InitializeKeySystemUuidTable() {
+  key_system_uuid_table = new KeySystemUuidTable();
+
+  key_system_uuid_table->push_back(make_pair(
+      kWidevineKeySystem,
+      std::vector<uint8>(kWidevineUuid,
+                         kWidevineUuid + arraysize(kWidevineUuid))));
+}
+
+KeySystemUuidTable::iterator FindKeySystem(const std::string& key_system) {

[ddorwin, 2014/05/15 19:00:45]

This name does not accurately describe the behavior. It "gets an iterator to a
pair in a table". That's an odd thing to return, especially since the callers
don't want an iterator or even the pair.

I think it should be called GetUuid() and either return false or an empty UUID
when not found.

[ycheo (away), 2014/05/16 04:53:38]

The method is removed by using hash_map.

+  KeySystemUuidTable::iterator it;
+  for (it = key_system_uuid_table->begin();
+       it != key_system_uuid_table->end(); ++it) {
+    // For security reasons, we only do exact string comparisons here - we don't
+    // try to parse the |key_system| in any way.
+    if (key_system == it->first)
+      return it;
   }
+  return it;
+}

[xhwang, 2014/05/15 15:10:29]

If you use a hashmap, you don't need this function.

[ycheo (away), 2014/05/16 04:53:38]

Done.

+
+std::vector<uint8> GetUUID(const std::string& key_system) {
+  if (key_system_uuid_table == NULL)

[ddorwin, 2014/05/15 19:00:45]

This code appears twice. Why not initialize the table in the constructor?

[ycheo (away), 2014/05/16 04:53:38]

Removed by using the lazy initializer.

+    InitializeKeySystemUuidTable();
+
+  KeySystemUuidTable::iterator it = FindKeySystem(key_system);
+  if (it != key_system_uuid_table->end())
+    return it->second;
   return std::vector<uint8>();

[xhwang, 2014/05/15 15:10:29]

nit: we like to handle abnormal cases first, i.e.

if (not found)
  return UUID();

return found_uuid;

[ycheo (away), 2014/05/16 04:53:38]

Done.

 }
 
 // Tries to find a PSSH box whose "SystemId" is |uuid| in |data|, parses the
 // "Data" of the box and put it in |pssh_data|. Returns true if such a box is
 // found and successfully parsed. Returns false otherwise.
 // Notes:
 // 1, If multiple PSSH boxes are found,the "Data" of the first matching PSSH box
 // will be set in |pssh_data|.
 // 2, Only PSSH and TENC boxes are allowed in |data|. TENC boxes are skipped.
-static bool GetPsshData(const uint8* data, int data_size,
-                        const std::vector<uint8>& uuid,
-                        std::vector<uint8>* pssh_data) {
+bool GetPsshData(const uint8* data, int data_size,
+                 const std::vector<uint8>& uuid,
+                 std::vector<uint8>* pssh_data) {
   const uint8* cur = data;
   const uint8* data_end = data + data_size;
   int bytes_left = data_size;
 
   while (bytes_left > 0) {
     const uint8* box_head = cur;
 
     if (bytes_left < kBoxHeaderSize)
       return false;
 
@@ skipping 54 matching lines @@
     if (box_end < cur + data_size)
       return false;
 
     pssh_data->assign(cur, cur + data_size);
     return true;
   }
 
   return false;
 }
 
-static MediaDrmBridge::SecurityLevel GetSecurityLevelFromString(
+MediaDrmBridge::SecurityLevel GetSecurityLevelFromString(
     const std::string& security_level_str) {
   if (0 == security_level_str.compare("L1"))
     return MediaDrmBridge::SECURITY_LEVEL_1;
   if (0 == security_level_str.compare("L3"))
     return MediaDrmBridge::SECURITY_LEVEL_3;
   DCHECK(security_level_str.empty());
   return MediaDrmBridge::SECURITY_LEVEL_NONE;
 }
 
-static std::string GetSecurityLevelString(
+std::string GetSecurityLevelString(
     MediaDrmBridge::SecurityLevel security_level) {
   switch (security_level) {
     case MediaDrmBridge::SECURITY_LEVEL_NONE:
       return "";
     case MediaDrmBridge::SECURITY_LEVEL_1:
       return "L1";
     case MediaDrmBridge::SECURITY_LEVEL_3:
       return "L3";
   }
   return "";
 }
 
 // Checks whether |key_system| is supported with |container_mime_type|. Only
 // checks |key_system| support if |container_mime_type| is empty.
 // TODO(xhwang): The |container_mime_type| is not the same as contentType in
 // the EME spec. Revisit this once the spec issue with initData type is
 // resolved.
-static bool IsKeySystemSupportedWithTypeImpl(
+bool IsKeySystemSupportedWithTypeImpl(
     const std::string& key_system,
     const std::string& container_mime_type) {
   if (!MediaDrmBridge::IsAvailable())
     return false;
 
   std::vector<uint8> scheme_uuid = GetUUID(key_system);
   if (scheme_uuid.empty())
     return false;
 
   JNIEnv* env = AttachCurrentThread();
   ScopedJavaLocalRef<jbyteArray> j_scheme_uuid =
       base::android::ToJavaByteArray(env, &scheme_uuid[0], scheme_uuid.size());
   ScopedJavaLocalRef<jstring> j_container_mime_type =
       ConvertUTF8ToJavaString(env, container_mime_type);
   return Java_MediaDrmBridge_isCryptoSchemeSupported(
       env, j_scheme_uuid.obj(), j_container_mime_type.obj());
 }
 
+}  // namespace
+
 // static
 bool MediaDrmBridge::IsAvailable() {
   return base::android::BuildInfo::GetInstance()->sdk_int() >= 19;
 }
 
 // static
 bool MediaDrmBridge::IsSecureDecoderRequired(SecurityLevel security_level) {
   DCHECK(IsAvailable());
   return SECURITY_LEVEL_1 == security_level;
 }
 
 // static
 bool MediaDrmBridge::IsSecurityLevelSupported(const std::string& key_system,
                                               SecurityLevel security_level) {
   if (!IsAvailable())
     return false;
 
   // Pass 0 as |cdm_id| and NULL as |manager| as they are not used in
   // creation time of MediaDrmBridge.
   scoped_ptr<MediaDrmBridge> media_drm_bridge =
       MediaDrmBridge::Create(0, key_system, GURL(), NULL);
   if (!media_drm_bridge)
     return false;
 
   return media_drm_bridge->SetSecurityLevel(security_level);
 }
 
+//static
+void MediaDrmBridge::AddKeySystem(const std::string& key_system,

[xhwang, 2014/05/15 15:10:29]

nit: This name is ambiguous. People may think that this is adding a key system
support. Actually it's just adding a UUID mapping.

[ycheo (away), 2014/05/16 04:53:38]

Renamed AddKeySystemUuidMapping().

+                                  const std::vector<uint8>& uuid) {
+  if (key_system_uuid_table == NULL)
+    InitializeKeySystemUuidTable();
+
+  KeySystemUuidTable::iterator it = FindKeySystem(key_system);
+  // Shouldn't overwrite the existing keysystem.

[xhwang, 2014/05/15 15:10:29]

DCHECK this condition as well?

[ycheo (away), 2014/05/16 04:53:38]

Done.

+  if (it == key_system_uuid_table->end())
+    key_system_uuid_table->push_back(make_pair(key_system, uuid));
+}
+
 // static
 bool MediaDrmBridge::IsKeySystemSupported(const std::string& key_system) {
   DCHECK(!key_system.empty());
   return IsKeySystemSupportedWithTypeImpl(key_system, "");
 }
 
 // static
 bool MediaDrmBridge::IsKeySystemSupportedWithType(
     const std::string& key_system,
     const std::string& container_mime_type) {
@@ skipping 211 matching lines @@
   JNIEnv* env = AttachCurrentThread();
   Java_MediaDrmBridge_resetDeviceCredentials(env, j_media_drm_.obj());
 }
 
 void MediaDrmBridge::OnResetDeviceCredentialsCompleted(
     JNIEnv* env, jobject, bool success) {
   base::ResetAndReturn(&reset_credentials_cb_).Run(success);
 }
 
 }  // namespace media