Add video decoding methods in Decryptor and add DecryptingVideoDecoder.

media/crypto/decrypting_video_decoder.h

Index: media/crypto/decrypting_video_decoder.h
diff --git a/media/crypto/decrypting_video_decoder.h b/media/crypto/decrypting_video_decoder.h
new file mode 100644
index 0000000000000000000000000000000000000000..c8d0ceb4f767db07966d2a5e2c401a9c113f3bf1
--- /dev/null
+++ b/media/crypto/decrypting_video_decoder.h
@@ -0,0 +1,98 @@
+// 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.
+
+#ifndef MEDIA_CRYPTO_DECRYPTING_VIDEO_DECODER_H_
+#define MEDIA_CRYPTO_DECRYPTING_VIDEO_DECODER_H_
+
+#include "base/callback.h"
+#include "base/memory/ref_counted.h"
+#include "media/base/decryptor.h"
+#include "media/base/demuxer_stream.h"
+#include "media/base/video_decoder.h"

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

And decoder_buffer.h to IWYU.

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

DecoderBuffer is forward declared on line 20. The header is included in
decrypting_video_decoder.cc.

+
+namespace base {
+class MessageLoopProxy;
+}
+
+namespace media {
+
+class DecoderBuffer;
+class Decryptor;
+
+// Decryptor powered VideoDecoder implementation that can decrypt and decode

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

Decryptor powered ==>
Decryptor-based? Decryptor-backed?

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

Done.

+// encrypted buffers and return unencrypted and uncompressed video frames.

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

s/unencrypted and uncompressed/decrypted decompressed/

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

Done.

+// TODO(xhwang): For now, DecryptingVideoDecoder relies on the decryptor to do
+// both decryption and video decoding. Add the path to use the decryptor for
+// decryption only and use other VideoDecoder implementations within
+// DecryptingVideoDecoder for video decoding.
+class MEDIA_EXPORT DecryptingVideoDecoder : public VideoDecoder {
+ public:
+  typedef base::Callback<
+      scoped_refptr<base::MessageLoopProxy>()> MessageLoopFactoryCB;
+  DecryptingVideoDecoder(const MessageLoopFactoryCB& message_loop_factory_cb,
+                         Decryptor* decryptor);
+
+  // VideoDecoder implementation.
+  virtual void Initialize(const scoped_refptr<DemuxerStream>& stream,
+                          const PipelineStatusCB& status_cb,
+                          const StatisticsCB& statistics_cb) OVERRIDE;
+  virtual void Read(const ReadCB& read_cb) OVERRIDE;
+  virtual void Reset(const base::Closure& closure) OVERRIDE;
+  virtual void Stop(const base::Closure& closure) OVERRIDE;
+
+ protected:
+  virtual ~DecryptingVideoDecoder();
+
+ private:
+  enum DecoderState {
+    kUninitialized,
+    kNormal,
+    kDecodeFinished
+  };
+
+  void OnDecoderInitDone(bool success);

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

Would OnDecoderInitialized not be accurate?

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

Done.

+  void DoRead(const ReadCB& read_cb);
+
+  // Reads from the demuxer stream with corresponding callback method.

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

I don't understand "corresponding callback method"

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

"corresponding callback method" is whatever callback the DVD passed into
DemuxerStream::Read(). I agree it's too much implementation detail and doesn't
need to be here. Remove the comment since the function name is pretty
self-explanatory.

+  void ReadFromDemuxerStream();
+  void DecryptAndDecodeBuffer(DemuxerStream::Status status,

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

Since there is no white space, I would assume 57 also applies to this, but I
don't think it does. So, add a comment or white space. Why are these grouped bug
DeliverFrame is not?

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

Done.

+                              const scoped_refptr<DecoderBuffer>& buffer);
+  // Carries out the buffer processing operation scheduled by
+  // DecryptOrDecodeBuffer().

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

DecryptAndDecodeBuffer

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

Done.

+  void DoDecryptAndDecodeBuffer(DemuxerStream::Status status,
+                                const scoped_refptr<DecoderBuffer>& buffer);
+
+  void DeliverFrame(int buffer_size,

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

Should this be a Do function as well since it is presumably called by another
callback?

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

I think we use "Do" for posted tasks, not on normal callbacks. 

+                    Decryptor::Status status,
+                    const scoped_refptr<VideoFrame>& frame);
+
+  // Reset decoder and call |reset_cb_|.
+  void DoReset();
+
+  // Free decoder resources and call |stop_cb_|.
+  void DoStop();
+
+  // This is !is_null() iff Initialize() hasn't been called.
+  MessageLoopFactoryCB message_loop_factory_cb_;
+
+  scoped_refptr<base::MessageLoopProxy> message_loop_;
+  DecoderState state_;
+  PipelineStatusCB status_cb_;
+  StatisticsCB statistics_cb_;
+  ReadCB read_cb_;
+  base::Closure reset_cb_;
+  base::Closure stop_cb_;
+
+  // Pointer to the demuxer stream that will feed us compressed buffers.
+  scoped_refptr<DemuxerStream> demuxer_stream_;
+
+  // Pointer to a Decryptor that can DecryptAndDecodeVideo() or Decrypt().
+  Decryptor* decryptor_;

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

*const

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

Done.

+
+  DISALLOW_COPY_AND_ASSIGN(DecryptingVideoDecoder);
+};
+
+}  // namespace media
+
+#endif  // MEDIA_CRYPTO_DECRYPTING_VIDEO_DECODER_H_