Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
10823299:
Add ContentDecryptionModule interface. (Closed)
|
Created:
8 years, 4 months ago by xhwang Modified:
8 years, 4 months ago CC:
chromium-reviews, feature-media-reviews_chromium.org, darin-cc_chromium.org Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionAdd ContentDecryptionModule interface.
This interface should be implemented by any CDM (ContentDecryptionModule) that needs to be hosted by the CDMWrapper. This version only supports Decrypt(). DecryptAndDecode() support will be added in the future.
BUG=none
TEST=none
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=152056
Patch Set 1 #
Total comments: 17
Patch Set 2 : Resolve comments #Patch Set 3 : Add default url in GenerateKeyRequest's output. #Patch Set 4 : Add NoKey error. #Patch Set 5 : Remove extra error code, use int for sizes and remove param in CdmCreateInstance. #Patch Set 6 : Add more comments. #
Total comments: 12
Patch Set 7 : Resolve comments. #
Total comments: 19
Patch Set 8 : Resolve comments. #
Total comments: 4
Patch Set 9 : Resolve ddorwin's comments. #
Total comments: 4
Patch Set 10 : Drop duration and fixed typo. #
Total comments: 2
Patch Set 11 : Resolve comments. #Patch Set 12 : Moved to webkit/media/crypto/ppapi/ #Messages
Total messages: 21 (0 generated)
Hello ddorwin, This CL adds the interface for ContentDecryptionModule. It mainly mirrors what the media pipeline provides/needs for decryption. DecodeAndDecrypt() is not in this version since it's not needed in short term. Comments needs to be improved (added), so don't spend too much time on it. Once the code looks good, I'll refine the comments before checking in. PTAL, xhwang
http://codereview.chromium.org/10823299/diff/1/webkit/media/crypto/content_de... File webkit/media/crypto/content_decryption_module.h (right): http://codereview.chromium.org/10823299/diff/1/webkit/media/crypto/content_de... webkit/media/crypto/content_decryption_module.h:8: #include <stdint.h> This may fail MSVC 2008. See https://groups.google.com/a/chromium.org/d/topic/chromium-dev/zFzvBFptJj4/dis.... http://codereview.chromium.org/10823299/diff/1/webkit/media/crypto/content_de... webkit/media/crypto/content_decryption_module.h:17: kCdmStatusSuccess = 0, "Though the Google C++ Style Guide says to use kConstantNaming for enums, Chrome still uses MACRO_STYLE naming for enums for consistency with our existing code." http://codereview.chromium.org/10823299/diff/1/webkit/media/crypto/content_de... webkit/media/crypto/content_decryption_module.h:26: // The Common Encryption spec supports subsample encryption, where portions The ISO... But, I think we should just define this as how Subsample Encryption is done and not lead in with comments about ISO. http://codereview.chromium.org/10823299/diff/1/webkit/media/crypto/content_de... webkit/media/crypto/content_decryption_module.h:50: // Subsamples as specified in ISO common encryption. Same. No need for a comment really since it's already a type. http://codereview.chromium.org/10823299/diff/1/webkit/media/crypto/content_de... webkit/media/crypto/content_decryption_module.h:53: int64_t timestamp; // Presentation timestamp in milliseconds. Any idea what the CDM does with this value and the next? Do we even need them in the plugin (caller of this interface) if we are already saving a request ID in the browser? http://codereview.chromium.org/10823299/diff/1/webkit/media/crypto/content_de... webkit/media/crypto/content_decryption_module.h:58: uint8_t* data; // Pointer to the beginning of the output data. This assumes the caller is allocating the output buffer. Fine for the very short term if you want to pre-allocate a large enough buffer. However, we need a TODO to fix the memory allocation. http://codereview.chromium.org/10823299/diff/1/webkit/media/crypto/content_de... webkit/media/crypto/content_decryption_module.h:75: const uint8_t* init_data, key, not init_data. EME supports init_data today, but we don't need to support it here and the next version won't. http://codereview.chromium.org/10823299/diff/1/webkit/media/crypto/content_de... webkit/media/crypto/content_decryption_module.h:84: OutputBuffer* decrypted_buffer) = 0; space after &
+scherkus for more comments. PTAL! http://codereview.chromium.org/10823299/diff/1/webkit/media/crypto/content_de... File webkit/media/crypto/content_decryption_module.h (right): http://codereview.chromium.org/10823299/diff/1/webkit/media/crypto/content_de... webkit/media/crypto/content_decryption_module.h:8: #include <stdint.h> On 2012/08/13 23:28:20, ddorwin wrote: > This may fail MSVC 2008. See > https://groups.google.com/a/chromium.org/d/topic/chromium-dev/zFzvBFptJj4/dis.... Summary of the discussion: - stdint types are available for VC10 and VC11 on Windows. - stdint types are recommended by Google's C++ style guide. - chrome is going to move towards stdint types and will probably add a stdint.h file for pre-VC9(VS2008) versions. Added typedef for Windows. http://codereview.chromium.org/10823299/diff/1/webkit/media/crypto/content_de... webkit/media/crypto/content_decryption_module.h:17: kCdmStatusSuccess = 0, On 2012/08/13 23:28:20, ddorwin wrote: > "Though the Google C++ Style Guide says to use kConstantNaming for enums, Chrome > still uses MACRO_STYLE naming for enums for consistency with our existing code." We are already using kConstantNaming at least in media code, see: http://codereview.chromium.org/10855051/diff/5002/media/base/message_loop_fac... http://codereview.chromium.org/10823299/diff/1/webkit/media/crypto/content_de... webkit/media/crypto/content_decryption_module.h:26: // The Common Encryption spec supports subsample encryption, where portions On 2012/08/13 23:28:20, ddorwin wrote: > The ISO... > > But, I think we should just define this as how Subsample Encryption is done and > not lead in with comments about ISO. Updated comment with examples and w/o mentioning ISO. http://codereview.chromium.org/10823299/diff/1/webkit/media/crypto/content_de... webkit/media/crypto/content_decryption_module.h:50: // Subsamples as specified in ISO common encryption. On 2012/08/13 23:28:20, ddorwin wrote: > Same. No need for a comment really since it's already a type. Done. http://codereview.chromium.org/10823299/diff/1/webkit/media/crypto/content_de... webkit/media/crypto/content_decryption_module.h:53: int64_t timestamp; // Presentation timestamp in milliseconds. On 2012/08/13 23:28:20, ddorwin wrote: > Any idea what the CDM does with this value and the next? > > Do we even need them in the plugin (caller of this interface) if we are already > saving a request ID in the browser? We need timestamp to track buffers since buffer can go out of decoding order. http://codereview.chromium.org/10823299/diff/1/webkit/media/crypto/content_de... webkit/media/crypto/content_decryption_module.h:58: uint8_t* data; // Pointer to the beginning of the output data. On 2012/08/13 23:28:20, ddorwin wrote: > This assumes the caller is allocating the output buffer. Fine for the very short > term if you want to pre-allocate a large enough buffer. However, we need a TODO > to fix the memory allocation. In Decrypt(), the OutputBuffer is passed in as "OutputBuffer* decrypted_buffer". So the caller allocates the OutputBuffer but leave the data as NULL, the callee (the CDM) will set the data, data_size, etc. So it's still the callee who allocates the memory. http://codereview.chromium.org/10823299/diff/1/webkit/media/crypto/content_de... webkit/media/crypto/content_decryption_module.h:75: const uint8_t* init_data, On 2012/08/13 23:28:20, ddorwin wrote: > key, not init_data. > EME supports init_data today, but we don't need to support it here and the next > version won't. Done.
Had a few updates based on offline discussion. PTAL!
LGTM http://codereview.chromium.org/10823299/diff/1/webkit/media/crypto/content_de... File webkit/media/crypto/content_decryption_module.h (right): http://codereview.chromium.org/10823299/diff/1/webkit/media/crypto/content_de... webkit/media/crypto/content_decryption_module.h:58: uint8_t* data; // Pointer to the beginning of the output data. On 2012/08/14 16:26:07, xhwang wrote: > On 2012/08/13 23:28:20, ddorwin wrote: > > This assumes the caller is allocating the output buffer. Fine for the very > short > > term if you want to pre-allocate a large enough buffer. However, we need a > TODO > > to fix the memory allocation. > > In Decrypt(), the OutputBuffer is passed in as "OutputBuffer* decrypted_buffer". > So the caller allocates the OutputBuffer but leave the data as NULL, the callee > (the CDM) will set the data, data_size, etc. So it's still the callee who > allocates the memory. Probably worth commenting this somewhere. Decrypt() I guess. I assume those are the places you'll be adding comments after the first round of reviews.
Comments added. PTAL! http://codereview.chromium.org/10823299/diff/1/webkit/media/crypto/content_de... File webkit/media/crypto/content_decryption_module.h (right): http://codereview.chromium.org/10823299/diff/1/webkit/media/crypto/content_de... webkit/media/crypto/content_decryption_module.h:58: uint8_t* data; // Pointer to the beginning of the output data. On 2012/08/14 22:52:11, ddorwin wrote: > On 2012/08/14 16:26:07, xhwang wrote: > > On 2012/08/13 23:28:20, ddorwin wrote: > > > This assumes the caller is allocating the output buffer. Fine for the very > > short > > > term if you want to pre-allocate a large enough buffer. However, we need a > > TODO > > > to fix the memory allocation. > > > > In Decrypt(), the OutputBuffer is passed in as "OutputBuffer* > decrypted_buffer". > > So the caller allocates the OutputBuffer but leave the data as NULL, the > callee > > (the CDM) will set the data, data_size, etc. So it's still the callee who > > allocates the memory. > > Probably worth commenting this somewhere. Decrypt() I guess. I assume those are > the places you'll be adding comments after the first round of reviews. Done.
http://codereview.chromium.org/10823299/diff/9001/webkit/media/crypto/content... File webkit/media/crypto/content_decryption_module.h (right): http://codereview.chromium.org/10823299/diff/9001/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:78: // The CDM may also generate a |default_url|. s/generate/extract/ http://codereview.chromium.org/10823299/diff/9001/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:80: // in which case the callee should allocates memory for the output parameters s/should/should have/. After the return, that happened in the past. http://codereview.chromium.org/10823299/diff/9001/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:81: // (e.g session_id) and takes ownership of them. But isn't ownership passed to the caller on return? http://codereview.chromium.org/10823299/diff/9001/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:102: // Returns kCdmStatusSuccess if there is no key request to be canceled or I would swap the phrases around "or" since the latter seems more common. http://codereview.chromium.org/10823299/diff/9001/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:110: // should fill the |decrypted_buffer| and take ownership of |data| in Similar to above. http://codereview.chromium.org/10823299/diff/9001/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:112: // Returns kCdmStatusErrorNoKey if the CDM does not have a key to decrypt. ... have the necessary decryption key.
http://codereview.chromium.org/10823299/diff/9001/webkit/media/crypto/content... File webkit/media/crypto/content_decryption_module.h (right): http://codereview.chromium.org/10823299/diff/9001/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:78: // The CDM may also generate a |default_url|. On 2012/08/15 02:14:57, ddorwin wrote: > s/generate/extract/ Done. http://codereview.chromium.org/10823299/diff/9001/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:80: // in which case the callee should allocates memory for the output parameters On 2012/08/15 02:14:57, ddorwin wrote: > s/should/should have/. > After the return, that happened in the past. Done. http://codereview.chromium.org/10823299/diff/9001/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:81: // (e.g session_id) and takes ownership of them. On 2012/08/15 02:14:57, ddorwin wrote: > But isn't ownership passed to the caller on return? Done. http://codereview.chromium.org/10823299/diff/9001/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:102: // Returns kCdmStatusSuccess if there is no key request to be canceled or On 2012/08/15 02:14:57, ddorwin wrote: > I would swap the phrases around "or" since the latter seems more common. Done. http://codereview.chromium.org/10823299/diff/9001/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:110: // should fill the |decrypted_buffer| and take ownership of |data| in On 2012/08/15 02:14:57, ddorwin wrote: > Similar to above. Done. http://codereview.chromium.org/10823299/diff/9001/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:112: // Returns kCdmStatusErrorNoKey if the CDM does not have a key to decrypt. On 2012/08/15 02:14:57, ddorwin wrote: > ... have the necessary decryption key. Done.
lgtm http://codereview.chromium.org/10823299/diff/3003/webkit/media/crypto/content... File webkit/media/crypto/content_decryption_module.h (right): http://codereview.chromium.org/10823299/diff/3003/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:103: // successfully canceled or there is no key request to be canceled. FYI, TBD what to do if no matching session. May be moot after switching to OO API.
I'm still awfully suspicious of the approach -- also does everything need to be in the global namespace? it seems like you only really "need" the create instance stuff as global C-extern and the rest could even be under a cdm namespace or similar http://codereview.chromium.org/10823299/diff/3003/webkit/media/crypto/content... File webkit/media/crypto/content_decryption_module.h (right): http://codereview.chromium.org/10823299/diff/3003/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:17: extern ContentDecryptionModule* CdmCreateInstance(); do these need to be extern C? http://codereview.chromium.org/10823299/diff/3003/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:17: extern ContentDecryptionModule* CdmCreateInstance(); what about deleting the instance? we can't assume the caller has the same memory allocator based on the scheme you described http://codereview.chromium.org/10823299/diff/3003/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:20: enum CdmStatus { move into CDM class and rename as Status? http://codereview.chromium.org/10823299/diff/3003/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:27: // A SubsampleEntry specifies the number of clear and cypher bytes in each s/cypher/cipher/g (all src/crypto/ code uses cipher not cypher) http://codereview.chromium.org/10823299/diff/3003/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:56: uint8_t* key_id; // Key ID to identify the decryption key. nit: add blank line between each data+size pair (it looks like a big wall of text at the moment) http://codereview.chromium.org/10823299/diff/3003/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:65: int64_t duration; // Duration in milliseconds. AFAIK we no longer care about duration http://codereview.chromium.org/10823299/diff/3003/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:79: // Returns kCdmStatusSuccess if the key request is successfully generated, s/is/was http://codereview.chromium.org/10823299/diff/3003/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:80: // in which case the callee should have allocated memory for the output I'm suspicious of this memory allocating business -- it assumes that the implementor of this CDM stuff (the .so or whatever) uses the same memory allocator as the caller i.e., the caller may be using tcmalloc but the .so is using regular Win32 malloc calls typical solutions are: 1) Caller preallocates buffer + passes in 2) Caller has to return buffers to free 3) Assume implementor uses chromium code and make this a basic chromium interface with chromium types
Comments mostly resolved, PTAL! http://codereview.chromium.org/10823299/diff/3003/webkit/media/crypto/content... File webkit/media/crypto/content_decryption_module.h (right): http://codereview.chromium.org/10823299/diff/3003/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:17: extern ContentDecryptionModule* CdmCreateInstance(); On 2012/08/15 23:26:29, scherkus wrote: > do these need to be extern C? Done. http://codereview.chromium.org/10823299/diff/3003/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:17: extern ContentDecryptionModule* CdmCreateInstance(); On 2012/08/15 23:26:29, scherkus wrote: > what about deleting the instance? > > we can't assume the caller has the same memory allocator based on the scheme you > described Done. http://codereview.chromium.org/10823299/diff/3003/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:20: enum CdmStatus { On 2012/08/15 23:26:29, scherkus wrote: > move into CDM class and rename as Status? Done. http://codereview.chromium.org/10823299/diff/3003/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:27: // A SubsampleEntry specifies the number of clear and cypher bytes in each On 2012/08/15 23:26:29, scherkus wrote: > s/cypher/cipher/g > > (all src/crypto/ code uses cipher not cypher) Done. Will fix cypher in src/media later. http://codereview.chromium.org/10823299/diff/3003/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:56: uint8_t* key_id; // Key ID to identify the decryption key. On 2012/08/15 23:26:29, scherkus wrote: > nit: add blank line between each data+size pair (it looks like a big wall of > text at the moment) Done. http://codereview.chromium.org/10823299/diff/3003/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:65: int64_t duration; // Duration in milliseconds. On 2012/08/15 23:26:29, scherkus wrote: > AFAIK we no longer care about duration Will media::Buffer drop the duration? http://codereview.chromium.org/10823299/diff/3003/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:79: // Returns kCdmStatusSuccess if the key request is successfully generated, On 2012/08/15 23:26:29, scherkus wrote: > s/is/was Done, here and below. http://codereview.chromium.org/10823299/diff/3003/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:80: // in which case the callee should have allocated memory for the output On 2012/08/15 23:26:29, scherkus wrote: > I'm suspicious of this memory allocating business -- it assumes that the > implementor of this CDM stuff (the .so or whatever) uses the same memory > allocator as the caller > > i.e., the caller may be using tcmalloc but the .so is using regular Win32 malloc > calls > > typical solutions are: > 1) Caller preallocates buffer + passes in > 2) Caller has to return buffers to free > 3) Assume implementor uses chromium code and make this a basic chromium > interface with chromium types Thank you so much for the suggestions. We were aware of this issue. ddorwin@ has a plan than can fix this, but we don't have time and test (a working CDM) to try that now. Added two TODOs to track this. About 3), even if the implementor uses chromium code, it's still possible that he/she uses a different compiler than we do, e.g. he/she has a super old checkout of chromium?
http://codereview.chromium.org/10823299/diff/3003/webkit/media/crypto/content... File webkit/media/crypto/content_decryption_module.h (right): http://codereview.chromium.org/10823299/diff/3003/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:80: // in which case the callee should have allocated memory for the output On 2012/08/16 02:37:43, xhwang wrote: > On 2012/08/15 23:26:29, scherkus wrote: > > I'm suspicious of this memory allocating business -- it assumes that the > > implementor of this CDM stuff (the .so or whatever) uses the same memory > > allocator as the caller > > > > i.e., the caller may be using tcmalloc but the .so is using regular Win32 > malloc > > calls > > > > typical solutions are: > > 1) Caller preallocates buffer + passes in > > 2) Caller has to return buffers to free > > 3) Assume implementor uses chromium code and make this a basic chromium > > interface with chromium types > > Thank you so much for the suggestions. We were aware of this issue. ddorwin@ has > a plan than can fix this, but we don't have time and test (a working CDM) to try > that now. Added two TODOs to track this. More accurately, we'll need to do some experimentation to make sure we can correctly abstract PPB_Buffer_Dev, and I'd like to have the PPAPI, sample CDM, and this landed so we can experiment on a stable base. > About 3), even if the implementor uses chromium code, it's still possible that > he/she uses a different compiler than we do, e.g. he/she has a super old > checkout of chromium? http://codereview.chromium.org/10823299/diff/11002/webkit/media/crypto/conten... File webkit/media/crypto/content_decryption_module.h (right): http://codereview.chromium.org/10823299/diff/11002/webkit/media/crypto/conten... webkit/media/crypto/content_decryption_module.h:20: extern "C" cdm::ContentDecryptionModule* CdmCreateInstance(); CreateCdmInstnace? Same below http://codereview.chromium.org/10823299/diff/11002/webkit/media/crypto/conten... webkit/media/crypto/content_decryption_module.h:100: // boundaries. Fix this later. Later is too vague.
http://codereview.chromium.org/10823299/diff/11002/webkit/media/crypto/conten... File webkit/media/crypto/content_decryption_module.h (right): http://codereview.chromium.org/10823299/diff/11002/webkit/media/crypto/conten... webkit/media/crypto/content_decryption_module.h:20: extern "C" cdm::ContentDecryptionModule* CdmCreateInstance(); On 2012/08/16 04:15:48, ddorwin wrote: > CreateCdmInstnace? Same below Done. http://codereview.chromium.org/10823299/diff/11002/webkit/media/crypto/conten... webkit/media/crypto/content_decryption_module.h:100: // boundaries. Fix this later. On 2012/08/16 04:15:48, ddorwin wrote: > Later is too vague. Done.
LGTM % typos http://codereview.chromium.org/10823299/diff/4005/webkit/media/crypto/content... File webkit/media/crypto/content_decryption_module.h (right): http://codereview.chromium.org/10823299/diff/4005/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:101: // are sample CDM are landed. s/are/and/ http://codereview.chromium.org/10823299/diff/4005/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:139: // are sample CDM are landed. same
scherkus@: Can you take a look again? FYI, this file will be moved to webkit/media/crypto/ppapi/ after Tom lands his CL: http://codereview.chromium.org/10827280/ http://codereview.chromium.org/10823299/diff/4005/webkit/media/crypto/content... File webkit/media/crypto/content_decryption_module.h (right): http://codereview.chromium.org/10823299/diff/4005/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:101: // are sample CDM are landed. On 2012/08/16 17:16:39, ddorwin wrote: > s/are/and/ Done. http://codereview.chromium.org/10823299/diff/4005/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:139: // are sample CDM are landed. On 2012/08/16 17:16:39, ddorwin wrote: > same Done.
lgtm w/ one nit -- let's get this landed and iterate http://codereview.chromium.org/10823299/diff/3003/webkit/media/crypto/content... File webkit/media/crypto/content_decryption_module.h (right): http://codereview.chromium.org/10823299/diff/3003/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:80: // in which case the callee should have allocated memory for the output On 2012/08/16 02:37:43, xhwang wrote: > On 2012/08/15 23:26:29, scherkus wrote: > > I'm suspicious of this memory allocating business -- it assumes that the > > implementor of this CDM stuff (the .so or whatever) uses the same memory > > allocator as the caller > > > > i.e., the caller may be using tcmalloc but the .so is using regular Win32 > malloc > > calls > > > > typical solutions are: > > 1) Caller preallocates buffer + passes in > > 2) Caller has to return buffers to free > > 3) Assume implementor uses chromium code and make this a basic chromium > > interface with chromium types > > Thank you so much for the suggestions. We were aware of this issue. ddorwin@ has > a plan than can fix this, but we don't have time and test (a working CDM) to try > that now. Added two TODOs to track this. > > About 3), even if the implementor uses chromium code, it's still possible that > he/she uses a different compiler than we do, e.g. he/she has a super old > checkout of chromium? For (3) I'm assuming we would build both halves of the plugin (PPAPI glue code + the implementation of this interface) using the same checkout. http://codereview.chromium.org/10823299/diff/5007/webkit/media/crypto/content... File webkit/media/crypto/content_decryption_module.h (right): http://codereview.chromium.org/10823299/diff/5007/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:20: extern "C" cdm::ContentDecryptionModule* CreateCdmInstance(); nit: extern "C" { }
Will need to move this file to whatever folder we decided before committing. http://codereview.chromium.org/10823299/diff/5007/webkit/media/crypto/content... File webkit/media/crypto/content_decryption_module.h (right): http://codereview.chromium.org/10823299/diff/5007/webkit/media/crypto/content... webkit/media/crypto/content_decryption_module.h:20: extern "C" cdm::ContentDecryptionModule* CreateCdmInstance(); On 2012/08/16 21:18:57, scherkus wrote: > nit: > > extern "C" { > > } Done.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/xhwang@chromium.org/10823299/6005
Try job failure for 10823299-6005 (retry) on linux_rel for step "interactive_ui_tests". It's a second try, previously, step "interactive_ui_tests" failed. http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/xhwang@chromium.org/10823299/6005
Change committed as 152056 |
