content/public/common/cdm_info.h - Issue 1637083002: Add CdmService to keep track of CDMs

Side by Side Diff: content/public/common/cdm_info.h

Issue 1637083002: Add CdmService to keep track of CDMs (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: rename + simplification Created 4 years, 9 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

OLD	NEW
(Empty)
	1 // Copyright 2016 The Chromium Authors. All rights reserved.

	2 // Use of this source code is governed by a BSD-style license that can be

	3 // found in the LICENSE file.

	4

	5 #ifndef CONTENT_PUBLIC_COMMON_CDM_INFO_H_

	6 #define CONTENT_PUBLIC_COMMON_CDM_INFO_H_

	7

	8 #include <string>

	9 #include <vector>

	10

	11 #include "base/files/file_path.h"

	12 #include "base/strings/string16.h"
	ddorwin 2016/03/18 19:24:18 Don't use string16. Don't use string16. jrummell 2016/03/23 21:38:10 Done. Show quoted text On 2016/03/18 19:24:18, ddorwin wrote: > Don't use string16. Done.
	13 #include "content/common/content_export.h"

	14

	15 namespace content {

	16

	17 // Collects information about Content Decryption Modules.
	ddorwin 2016/03/18 19:24:18 An object does not perform actions. Contains? Repr An object does not perform actions. Contains? Represents? Also, it only represents a single CDM. For example, "Represents a Content Decryption Module implementation and its capabilities." Note that it is important that this represents details about a concrete CDM that is actually present - not an abstract one like Key System info. jrummell 2016/03/23 21:38:10 Done. Show quoted text On 2016/03/18 19:24:18, ddorwin wrote: > An object does not perform actions. Contains? Represents? > Also, it only represents a single CDM. > > For example, "Represents a Content Decryption Module implementation and its > capabilities." > > Note that it is important that this represents details about a concrete CDM that > is actually present - not an abstract one like Key System info. Done.
	18 struct CONTENT_EXPORT CdmInfo {

	19 // Identifier for the CDM.

	20 std::string cdm_id;
	ddorwin 2016/03/18 19:24:18 "CDM ID" is already overloaded. For example, in Cd "CDM ID" is already overloaded. For example, in CdmContext. We should try to come up with another term. xhwang 2016/03/18 19:35:38 What is this for? If we need this, we are already What is this for? If we need this, we are already using cdm_id in some other context (see cdm_context). Maybe we should find another name. Also, since we are already in Cdm* class, "cdm" prefix seems redundant. jrummell 2016/03/23 21:38:09 Renamed. Show quoted text On 2016/03/18 19:35:38, xhwang wrote: > What is this for? > > If we need this, we are already using cdm_id in some other context (see > cdm_context). Maybe we should find another name. > > Also, since we are already in Cdm* class, "cdm" prefix seems redundant. Renamed.
	21

	22 // Version of the CDM.
	ddorwin 2016/03/18 19:24:18 May be empty. (Pretty much everywhere except desk May be empty. (Pretty much everywhere except desktop.) jrummell 2016/03/23 21:38:09 Done. Show quoted text On 2016/03/18 19:24:18, ddorwin wrote: > May be empty. > > (Pretty much everywhere except desktop.) Done.
	23 std::string version;
	xhwang 2016/03/18 19:35:38 use base::Version? use base::Version? jrummell 2016/03/23 21:38:09 Done. Show quoted text On 2016/03/18 19:35:38, xhwang wrote: > use base::Version? Done.
	24

	25 // Path to the library implementing the CDM. May be empty() if the
	ddorwin 2016/03/18 19:24:18 Remove "()". This should probably be ifdef'd to t Remove "()". This should probably be ifdef'd to the equivalent of ENABLE_PEPPER_CDMS. I would also move it below the more common and required values below. jrummell 2016/03/23 21:38:09 I'd like to avoid #ifdefs. Show quoted text On 2016/03/18 19:24:18, ddorwin wrote: > Remove "()". > > This should probably be ifdef'd to the equivalent of ENABLE_PEPPER_CDMS. > > I would also move it below the more common and required values below. I'd like to avoid #ifdefs.
	26 // CDM is already included as part of the system.
	ddorwin 2016/03/18 19:24:18 If we don't ifdef this, we should clean up this li If we don't ifdef this, we should clean up this line. jrummell 2016/03/23 21:38:09 Done. Show quoted text On 2016/03/18 19:24:18, ddorwin wrote: > If we don't ifdef this, we should clean up this line. Done.
	27 base::FilePath module_library_path;
	xhwang 2016/03/18 19:35:38 nit: s/module_library_path/path? See: https://co nit: s/module_library_path/path? See: https://code.google.com/p/chromium/codesearch#chromium/src/content/public/com... jrummell 2016/03/23 21:38:09 Done. Show quoted text On 2016/03/18 19:35:38, xhwang wrote: > nit: s/module_library_path/path? > > See: > > https://code.google.com/p/chromium/codesearch#chromium/src/content/public/com... Done.
	28

	29 // List of codecs supported by the CDM.
	ddorwin 2016/03/18 19:24:18 How we use this information varies depending on th How we use this information varies depending on the platform. We should fix that sometime. jrummell 2016/03/23 21:38:09 Acknowledged. Show quoted text On 2016/03/18 19:24:18, ddorwin wrote: > How we use this information varies depending on the platform. We should fix that > sometime. Acknowledged.
	30 std::vector<base::string16> supported_codecs;
	xhwang 2016/03/18 19:35:38 why these need to be string16 instead of string? why these need to be string16 instead of string? jrummell 2016/03/23 21:38:09 Copied from what the Pepper code for WV did. Now s Show quoted text On 2016/03/18 19:35:38, xhwang wrote: > why these need to be string16 instead of string? Copied from what the Pepper code for WV did. Now switched to std::string.
	31

	32 // List of init_data_types supported by the CDM.
	ddorwin 2016/03/18 19:24:18 s/init_data_types/Initialization Data Types/ s/init_data_types/Initialization Data Types/ jrummell 2016/03/23 21:38:09 Removed since we aren't using this yet. Show quoted text On 2016/03/18 19:24:18, ddorwin wrote: > s/init_data_types/Initialization Data Types/ Removed since we aren't using this yet.
	33 std::vector<base::string16> supported_init_data_types;
	xhwang 2016/03/18 19:35:38 Are we using these init_data_types in this CL? If Are we using these init_data_types in this CL? If not shall we leave it in a future CL, where we may also need session types? jrummell 2016/03/23 21:38:10 Done. Show quoted text On 2016/03/18 19:35:38, xhwang wrote: > Are we using these init_data_types in this CL? If not shall we leave it in a > future CL, where we may also need session types? Done.
	34

	35 CdmInfo(const std::string& cdm_id,

	36 const std::string& version,

	37 const base::FilePath& module_library_path,
	ddorwin 2016/03/18 19:24:18 The reordering suggestion above would complicate t The reordering suggestion above would complicate the suggested ifdef here, though. jrummell 2016/03/23 21:38:10 Acknowledged. Show quoted text On 2016/03/18 19:24:18, ddorwin wrote: > The reordering suggestion above would complicate the suggested ifdef here, > though. Acknowledged.
	38 const std::vector<base::string16>& supported_codecs,

	39 const std::vector<base::string16>& supported_init_data_types);

	40 ~CdmInfo();
	xhwang 2016/03/18 19:35:38 Declare ctor/dtor before member variables Declare ctor/dtor before member variables jrummell 2016/03/23 21:38:10 Done. Show quoted text On 2016/03/18 19:35:38, xhwang wrote: > Declare ctor/dtor before member variables Done.
	41 };

	42

	43 } // namespace content

	44

	45 #endif // CONTENT_PUBLIC_COMMON_CDM_INFO_H_

OLD	NEW

« content/common/media/cdm_service.cc ('K') | « content/content_tests.gypi ('k') | content/public/common/cdm_service.h » ('j') | content/public/common/cdm_service.h » ('J')