Add content decryptor related structs and update PP{P|B}_ContentDecryptor_Private.

ppapi/api/private/pp_decrypt_config.idl

Index: ppapi/api/private/pp_decrypt_config.idl
diff --git a/ppapi/api/private/pp_decrypt_config.idl b/ppapi/api/private/pp_decrypt_config.idl
new file mode 100644
index 0000000000000000000000000000000000000000..9fdb0d5246bf829f41fe6c25b2fa7b15439e3c85
--- /dev/null
+++ b/ppapi/api/private/pp_decrypt_config.idl
@@ -0,0 +1,107 @@
+/* Copyright (c) 2012 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.
+ */
+
+/**
+ * The data structure that can be used to associate the decrypted data with a

[dmichael (off chromium), 2012/08/16 17:36:33]

This comment is worded strangely. We usually use complete sentences, and maybe
start with the struct name...  like:
"PP_DecryptTrackingInfo is a struct that can be used..."

[xhwang, 2012/08/16 20:10:39]

Done.

+ * decrypt request and/or an input buffer.
+ */
+[assert_size(24)]
+struct PP_DecryptTrackingInfo {
+  /**
+   * Client-specified identifier for the associated decrypt request. By using
+   * this value client can associate the decrypted data with a decryption

[dmichael (off chromium), 2012/08/16 17:36:33]

"value client" -> "value, the client"

[xhwang, 2012/08/16 20:10:39]

Done.

+   * request.
+   */
+  uint64_t request_id;
+
+  /**
+   * Timestamp in milliseconds of the associated buffer. By using this value
+   * client can associate the decrypted (and decoded) data with an input buffer.

[ddorwin, 2012/08/16 04:31:52]

Why isn't the above member enough for association?

[xhwang, 2012/08/16 20:10:39]

Added more explanation: "This is needed as the decoded buffers can be out of
order."

+   */
+  int64_t timestamp;

[dmichael (off chromium), 2012/08/16 17:36:33]

We usually use PP_Time (which really just typedefs a double) for time stamps.
Would that work okay for you? It would make your time easier to associate with
things like input events, file times, or wall clock times, if those ever matter
for CDMs.

[xhwang, 2012/08/16 20:10:39]

This timestamp is really only for tracking and in theory it can be any unique
identifier. We won't use it for any input events, file times etc. So I'll just
keep it here as int64_t to avoid any unnecessary conversion.

+
+  /**
+   * Duration in milliseconds of the associated buffer.
+   */
+  int64_t duration;

[ddorwin, 2012/08/16 04:31:52]

Pending scherkus's response in the other CL.

[dmichael (off chromium), 2012/08/16 17:36:33]

64 bits seems really huge for milliseconds...  are you sure that's really the
resolution you want? Did you mean microseconds, or int32_t?

[xhwang, 2012/08/16 20:10:39]

duration is removed as per offline discussion w/ scherkus@.

+};
+
+/**
+ * The data structure for decryption subsample entry.
+ *
+ * An input buffer can be split into several continuous subsamples.
+ * A <code>PP_DecryptSubsampleEntry</code> specifies the number of clear and
+ * cipher bytes in each subsample. For example, the following buffer has three
+ * subsamples:
+ *
+ * |<----- subsample1 ----->|<----- subsample2 ----->|<----- subsample3 ----->|
+ * |   clear1   |  cipher1  |  clear2  |   cipher2   | clear3 |    cipher3    |
+ *
+ * For decryption, all of the cipher bytes in a buffer should be concatenated

[dmichael (off chromium), 2012/08/16 17:36:33]

Do you really have to concatenate the "cipher bytes"? I think you just mean that
they should be considered as a contiguous sequence of bytes during decryption,
ignoring the "clear" portion. The way you word it seems to dictate an
implementation detail, and I think the implementation would actually just want
to read the cipher text as-is from the subsamples without any intermediate step
of concatenating the encrypted data. Make sense? I'm not sure how best to word
that, though.

[xhwang, 2012/08/16 20:10:39]

Done.

+ * (in the subsample order) into a single logical stream. The clear bytes should
+ * not be considered as part of decryption.
+ *
+ * Stream to decrypt:   |  cipher1  |   cipher2   |    cipher3    |
+ * Decrypted stream:    | decrypted1|  decrypted2 |   decrypted3  |
+ *
+ * After decryption, the decrypted bytes should be copied over the position
+ * of the corresponding cipher bytes in the original buffer to form the output
+ * buffer. Following the above example, the decrypted buffer should be:
+ *
+ * |<----- subsample1 ----->|<----- subsample2 ----->|<----- subsample3 ----->|
+ * |   clear1   | decrypted1|  clear2  |  decrypted2 | clear3 |   decrypted3  |
+ */
+[assert_size(8)]
+struct PP_DecryptSubsampleEntry {

[dmichael (off chromium), 2012/08/16 17:36:33]

Maybe a different name, since this isn't really a Subsample Entry so much as a
"Subsample Description"?

[xhwang, 2012/08/16 20:10:39]

Done.

+  /**
+   * Size in bytes of clear data in a subsample entry.
+   */
+  uint32_t clear_bytes;
+
+  /**
+   * Size in bytes of encrypted data in a subsample entry.
+   */
+  uint32_t cipher_bytes;
+};
+
+/**
+ * The data structure that contains all information needed to decrypt a buffer.

[ddorwin, 2012/08/16 04:31:52]

Is "The data structure that" wording common in PPAPI? Sounds funny.

[dmichael (off chromium), 2012/08/16 17:36:33]

Not common. We'd usually say "PP_DecryptConfig contains..."

[xhwang, 2012/08/16 20:10:39]

Done.

+ */
+[assert_size(368)]
+struct PP_DecryptConfig {
+  /**
+   * Information needed by the client to track the buffer to be decrypted.
+   */
+  PP_DecryptTrackingInfo tracking_info;

[ddorwin, 2012/08/16 04:31:52]

Not really decryption configuration. Can we use a separate parameter or are we
out of parameter space in the calls?

[xhwang, 2012/08/16 20:10:39]

Added comment about this.

+
+  /**
+   * Size of data to be discarded before applying the decryption (in bytes).
+   */
+  uint32_t data_offset;
+
+  /**
+   * Key ID of the buffer to be decrypted.
+   */
+  uint8_t[68] key_id;

[xhwang, 2012/08/16 03:14:41]

As per dmichael@:
1, 8-byte data need to be on 8-byte boundary.
2, if a struct has 8byte data (e.g. timestamp in tracking_info), then the size
of the struct needs to be a multiple of 8 (through padding if necessary).

That's why I have "68" here just to avoid extra padding.

Maybe I should avoid these magic numbers and use something like "kMaxKeyIdSize"?

As per fgalligan@, the KeyId can be as large as 2048bytes in WebM. If we do need
to use that many, maybe we should move this out as a separate PP_Var parameter.

[ddorwin, 2012/08/16 04:31:52]

I still don't understand the reason for 68 here. Maybe you can explain offline.

[dmichael (off chromium), 2012/08/16 17:36:33]

Well, it's not *our* rule :-). The compiler will align it on an 8-byte boundary
for you on 64-bit platforms, and sometimes 4-byte boundaries on 32-bit (e.g.,
double aligns to 32-bit on NaCl and Windows). We try to keep consistent sizes to
make the proxy easier.
That is, the compiler will enforce that (sometimes only on 64-bit, though), so
that if you put those types in an array, all the structs and their
8-byte-aligned members remain 8-byte-aligned. To keep consistent size across
platforms, we usually pad. (Admittedly, this only really matters when we talk to
NaCl from 64-bit trusted code, and only when we do cheap memcpy serialization as
we do here).
A constant might be better, yeah. And I would just use the correct size for your
data, whatever it is. Add padding as a separate field with a nice explanatory
comment (see some of our other padding comments on structs for examples).

[xhwang, 2012/08/16 20:10:39]

There's no easy way to define and use a constant in IDL file. So keep the
numbers hard coded here for now.

And I would just use the correct size for your
Thanks for the more detailed explanation!

I double checked and adjusted the size for IV and checksum. Now it's a multiple
of 8 w/o padding :)

+  uint32_t key_id_size;
+
+  /**
+   * Initialization vector of the buffer to be decrypted.
+   */
+  uint8_t[64] iv;
+  uint32_t iv_size;
+
+  /**
+   * Checksum of the buffer to be decrypted.
+   */
+  uint8_t[64] checksum;
+  uint32_t checksum_size;
+
+  /**
+   * Subsamples of the buffer to be decrypted.

[ddorwin, 2012/08/16 04:31:52]

Subsample information/map of...

[xhwang, 2012/08/16 20:10:39]

Done.

+   */
+  PP_DecryptSubsampleEntry[16] subsamples;

[ddorwin, 2012/08/16 04:31:52]

Strange to always have 1 KB of data appended. May have similar problems if we
have to support large Key IDs.

[xhwang, 2012/08/16 20:10:39]

It's 16*8=128bytes, so I suppose it's fine here. Will keep it here for now due
to the limit on num of parameters in IPC message.

+  uint32_t num_subsamples;
+};

[dmichael (off chromium), 2012/08/16 17:36:33]

This is pretty big... you *could* consider making it a PP_Resource.
- Is it likely to change a lot (like, adding new fields, changing their size)?
- Do you have to worry about backwards compatibility, like working with older
CDM modules that were built for an older version of PPAPI?

If yes on those questions, you should probably use PP_Resource. If not, then the
way you have it is probably simpler (at least to start).

[xhwang, 2012/08/16 20:10:39]

That's a good suggestion. It may change in size and we'll need backwards
compatibility later (but not anytime soon). But currently we need something
simpler to start with and we don't have resource to do it that way. I'll keep it
as is for now and we can fix it in the future.