Add video decoding methods in Decryptor and add DecryptingVideoDecoder.

media/base/decryptor.h

Index: media/base/decryptor.h
diff --git a/media/base/decryptor.h b/media/base/decryptor.h
index f014406d50155519d8725bc0887d168bc0087197..b79a3367893d44dc0515cadd925c27e84706f785 100644
--- a/media/base/decryptor.h
+++ b/media/base/decryptor.h
@@ -15,12 +15,19 @@
 namespace media {
 
 class DecoderBuffer;
-
-// Performs key operations and decrypts encrypted buffer.
-// All public methods other than Decrypt() will be called on the renderer
-// thread. Therefore, these calls should be fast and nonblocking, with key
-// events fired asynchronously. Decrypt() will be called on the (video/audio)
-// decoder thread.
+class VideoDecoderConfig;
+class VideoFrame;
+
+// Performs key operations and decrypts (and decodes) encrypted buffer.
+//
+// Key operations (GenerateKeyRequest(), AddKey() and CancelKeyRequest())
+// are called on the renderer thread. Therefore, these calls should be fast
+// and nonblocking (key events should be fired asynchronously).

[ddorwin, 2012/09/21 00:36:08]

Parenthesis implies this is an interpretation of the previous statement. I think
it is in addition (the actions must be async too), so maybe it should be after a
';'.

[xhwang, 2012/09/25 23:52:32]

Done.

+// All other methods are called on the (video/audio) decoder thread.
+// Decryptor implementations must be thread safe when methods are called
+// following the above model.
+//
+// Note that the all callbacks can be fired synchronously or asynchronously.

[ddorwin, 2012/09/21 00:36:08]

Why? This can lead to confusion.

[scherkus (not reviewing), 2012/09/21 18:57:56]

Would be really, really nice to mandate one or the other.

[xhwang, 2012/09/24 16:13:19]

Well, this isn't new. I just moved it from the old code (line 71) here.

The reason for this is that
a) decryptors don't have their own threads, and
b) we need both cases: in AesDecryptor, we call those callbacks synchronously
(and since (a), we can't make it asyn now). In PpapiDecryptor, callbacks are
fired on the renderer's thread asynchronously.

By specifying this clearly here, the caller must ensure that it supports both
(pretty much by force posting the callbacks).

 class MEDIA_EXPORT Decryptor {
  public:
   enum KeyError {
@@ -35,6 +42,7 @@ class MEDIA_EXPORT Decryptor {
   enum Status {
     kSuccess,  // Decryption successfully completed. Decrypted buffer ready.
     kNoKey,  // No key is available to decrypt.
+    kNeedMoreData,  // Decoder needs more data to produce a frame.
     kError  // Key is available but an error occurred during decryption.
   };
 
@@ -64,27 +72,77 @@ class MEDIA_EXPORT Decryptor {
   virtual void CancelKeyRequest(const std::string& key_system,
                                 const std::string& session_id) = 0;
 
-  // Decrypts the |encrypted| buffer. The decrypt status and decrypted buffer
-  // are returned via the provided callback |decrypt_cb|. The |encrypted| buffer
-  // must not be NULL.
+  // Indicates completion of a decryption operation.
   //
-  // Note that the callback maybe called synchronously or asynchronously.
-  //
-  // If the returned status is kSuccess, the |encrypted| buffer is successfully
-  // decrypted and the decrypted buffer is ready to be read.
-  // If the returned status is kNoKey, no decryption key is available to decrypt
-  // |encrypted| buffer. In this case the decrypted buffer must be NULL.
-  // If the returned status is kError, unexpected error has occurred. In this
-  // case the decrypted buffer must be NULL.
+  // First parameter: The status of the decryption operation.
+  // - Set to kSuccess if the encrypted buffer is successfully decrypted and
+  //   the decrypted buffer is ready to be read.
+  // - Set to kNoKey if no decryption key is available to decrypt the encrypted
+  //   buffer. In this case the decrypted buffer must be NULL.
+  // - Set to kError if unexpected error has occurred. In this case the
+  //   decrypted buffer must be NULL.
+  // - This parameter should not be set to kNeedMoreData.
+  // Second parameter: The decrypted buffer.
   typedef base::Callback<void(Status,
                               const scoped_refptr<DecoderBuffer>&)> DecryptCB;
+
+  // Decrypts the |encrypted| buffer. The decrypt status and decrypted buffer
+  // are returned via the provided callback |decrypt_cb|. The |encrypted| buffer
+  // must not be NULL.
   virtual void Decrypt(const scoped_refptr<DecoderBuffer>& encrypted,
                        const DecryptCB& decrypt_cb) = 0;
 
   // Stops the decryptor and fires any pending DecryptCB immediately with kError
   // and NULL buffer.
+  // TODO(xhwang): Rename this to StopDecrypting().
   virtual void Stop() = 0;
 
+  // Indicates completion of decoder initialization.
+  //
+  // First Parameter: Indicates initialization success.
+  // - Set to true if initialization was successful. False if an error occurred.
+  typedef base::Callback<void(bool)> InitializeCB;

[ddorwin, 2012/09/21 00:36:08]

I think "Initialize" is too generic here. InitializeDecoderCB is long but much
more meaningful.

[xhwang, 2012/09/25 23:52:32]

Renamed to DecoderInitCB.

+
+  // Initializes a video decoder with the given |config|, executing the
+  // |init_cb| upon completion.
+  // Note: DecryptAndDecodeVideo(), ResetVideoDecoder() and StopVideoDecoder()
+  // should only be called after InitializeVideoDecoder() succeeded.

[ddorwin, 2012/09/21 00:36:08]

Hopefully we have a DCHECK for this too.

[xhwang, 2012/09/25 23:52:32]

Will do in implementations in the next CL.

[ddorwin, 2012/09/26 02:34:27]

Add a TODO?

+  virtual void InitializeVideoDecoder(const VideoDecoderConfig& config,
+                                      const InitializeCB& init_cb) = 0;
+
+  // Indicates completion of video decrypting and decoding operation.
+  //
+  // First parameter: The status of the decrypting and decoding operation.
+  // - Set to kSuccess if the encrypted buffer is successfully decrypted and
+  //   decoded. In this case, the decoded video frame is not NULL. It either
+  //   contains the decoded video frame or indicates the end of the stream.

[ddorwin, 2012/09/21 00:36:08]

How does it indicate the end of the stream? If it's a function on VideFrame,
maybe we can imply that more directly than "indicates".

[xhwang, 2012/09/25 23:52:32]

Done.

+  // - Set to kNoKey if no decryption key is available to decrypt the encrypted
+  //   buffer. In this case the decoded video frame must be NULL.
+  // - Set to kNeedMoreData if more data is needed to produce a video frame. In
+  //   this case the decoded video frame must be NULL.
+  // - Set to kError if unexpected error has occurred. In this case the
+  //   decoded video frame must be NULL.
+  // Second parameter: The decoded video frame.
+  typedef base::Callback<void(Status,
+                              const scoped_refptr<VideoFrame>&)> VideoDecodeCB;
+
+  // Decrypts and decodes the |encrypted| buffer. The status and the decrypted
+  // buffer are returned via the provided callback |video_decode_cb|.
+  // The |encrypted| buffer must not be NULL.
+  // At end-of-stream, this method should be called repeatedly with
+  // end-of-stream DecoderBuffer until no video frame can be produced.
+  virtual void DecryptAndDecodeVideo(
+      const scoped_refptr<DecoderBuffer>& encrypted,
+      const VideoDecodeCB& video_decode_cb) = 0;
+
+  // Resets decoder state, fulfilling all pending VideoDecodeCB and dropping
+  // extra queued decoded data. After this call, the decoder is back to an
+  // initialized clean state.
+  virtual void ResetVideoDecoder() = 0;
+
+  // Stops decoder and sets it to an uninitialized state.
+  virtual void StopVideoDecoder() = 0;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(Decryptor);
 };