Cleanup VideoDecoder interface.

This removes some un-needed methods and the DecodeResult enumeration.

Review URL: https://chromiumcodereview.appspot.com/23702056

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@224762 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 38 insertions, 85 deletions

diff --git a/remoting/client/rectangle_update_decoder.cc b/remoting/client/rectangle_update_decoder.cc
index 106c993..3f21a99 100644
--- a/remoting/client/rectangle_update_decoder.cc
+++ b/remoting/client/rectangle_update_decoder.cc
@@ -92,14 +92,11 @@ void RectangleUpdateDecoder::DecodePacket(scoped_ptr<VideoPacket> packet,
   if (notify_size_or_dpi_change)
     consumer_->SetSourceSize(source_size_, source_dpi_);
 
-  if (!decoder_->IsReadyForData()) {
-    // TODO(ajwong): This whole thing should move into an invalid state.
-    LOG(ERROR) << "Decoder is unable to process data. Dropping packet.";
-    return;
-  }
-
-  if (decoder_->DecodePacket(packet.get()) == VideoDecoder::DECODE_DONE)
+  if (decoder_->DecodePacket(*packet.get())) {
     SchedulePaint();
+  } else {
+    LOG(ERROR) << "DecodePacket() failed.";
+  }
 }
 
 void RectangleUpdateDecoder::SchedulePaint() {
diff --git a/remoting/codec/codec_test.cc b/remoting/codec/codec_test.cc
index 83d1035..eebe263 100644
--- a/remoting/codec/codec_test.cc
+++ b/remoting/codec/codec_test.cc
@@ -78,13 +78,9 @@ class VideoDecoderTester {
   }
 
   void ReceivedPacket(VideoPacket* packet) {
-    VideoDecoder::DecodeResult result = decoder_->DecodePacket(packet);
+    ASSERT_TRUE(decoder_->DecodePacket(*packet));
 
-    ASSERT_NE(VideoDecoder::DECODE_ERROR, result);
-
-    if (result == VideoDecoder::DECODE_DONE) {
-      RenderFrame();
-    }
+    RenderFrame();
   }
 
   void RenderFrame() {
diff --git a/remoting/codec/video_decoder.h b/remoting/codec/video_decoder.h
index 4730d3a..61f0233 100644
--- a/remoting/codec/video_decoder.h
+++ b/remoting/codec/video_decoder.h
@@ -17,14 +17,7 @@ namespace remoting {
 // outputs frames of data.
 class VideoDecoder {
  public:
-  // DecodeResult is returned from DecodePacket() and indicates current state
-  // of the decoder. DECODE_DONE means that last packet for the frame was
-  // processed, and the frame can be displayed now. DECODE_ERROR is returned if
-  // there was an error in the stream.
-  enum DecodeResult {
-    DECODE_ERROR,
-    DECODE_DONE,
-  };
+  static const int kBytesPerPixel = 4;
 
   VideoDecoder() {}
   virtual ~VideoDecoder() {}
@@ -33,13 +26,9 @@ class VideoDecoder {
   // |screen size| must not be empty.
   virtual void Initialize(const SkISize& screen_size) = 0;
 
-  // Feeds more data into the decoder.
-  virtual DecodeResult DecodePacket(const VideoPacket* packet) = 0;
-
-  // Returns true if decoder is ready to accept data via DecodePacket.
-  virtual bool IsReadyForData() = 0;
-
-  virtual VideoPacketFormat::Encoding Encoding() = 0;
+  // Feeds more data into the decoder. Returns true if |packet| was processed
+  // and the frame can be displayed now.
+  virtual bool DecodePacket(const VideoPacket& packet) = 0;
 
   // Marks the specified |region| of the view for update the next time
   // RenderFrame() is called.  |region| is expressed in |view_size| coordinates.
diff --git a/remoting/codec/video_decoder_verbatim.cc b/remoting/codec/video_decoder_verbatim.cc
index b6b2179..0431dc2 100644
--- a/remoting/codec/video_decoder_verbatim.cc
+++ b/remoting/codec/video_decoder_verbatim.cc
@@ -9,20 +9,11 @@
 
 namespace remoting {
 
-namespace {
-// Both input and output data are assumed to be RGBA32.
-const int kBytesPerPixel = 4;
-}  // namespace
-
 VideoDecoderVerbatim::VideoDecoderVerbatim()
     : screen_size_(SkISize::Make(0, 0)) {}
 
 VideoDecoderVerbatim::~VideoDecoderVerbatim() {}
 
-bool VideoDecoderVerbatim::IsReadyForData() {
-  return true;
-}
-
 void VideoDecoderVerbatim::Initialize(const SkISize& screen_size) {
   updated_region_.setEmpty();
   screen_buffer_.reset();
@@ -31,19 +22,18 @@ void VideoDecoderVerbatim::Initialize(const SkISize& screen_size) {
   // Allocate the screen buffer, if necessary.
   if (!screen_size_.isEmpty()) {
     screen_buffer_.reset(
-        new uint8
-            [screen_size_.width() * screen_size_.height() * kBytesPerPixel]);
+        new uint8[screen_size_.width() * screen_size_.height() *
+                  kBytesPerPixel]);
   }
 }
 
-VideoDecoder::DecodeResult VideoDecoderVerbatim::DecodePacket(
-    const VideoPacket* packet) {
+bool VideoDecoderVerbatim::DecodePacket(const VideoPacket& packet) {
   SkRegion region;
 
-  const char* in = packet->data().data();
+  const char* in = packet.data().data();
   int stride = kBytesPerPixel * screen_size_.width();
-  for (int i = 0; i < packet->dirty_rects_size(); ++i) {
-    Rect proto_rect = packet->dirty_rects(i);
+  for (int i = 0; i < packet.dirty_rects_size(); ++i) {
+    Rect proto_rect = packet.dirty_rects(i);
     SkIRect rect = SkIRect::MakeXYWH(proto_rect.x(),
                                      proto_rect.y(),
                                      proto_rect.width(),
@@ -52,16 +42,16 @@ VideoDecoder::DecodeResult VideoDecoderVerbatim::DecodePacket(
 
     if (!SkIRect::MakeSize(screen_size_).contains(rect)) {
       LOG(ERROR) << "Invalid packet received";
-      return DECODE_ERROR;
+      return false;
     }
 
     int rect_row_size = kBytesPerPixel * rect.width();
     uint8_t* out = screen_buffer_.get() + rect.y() * stride +
                    rect.x() * kBytesPerPixel;
     for (int y = rect.y(); y < rect.y() + rect.height(); ++y) {
-      if (in + rect_row_size > packet->data().data() + packet->data().size()) {
+      if (in + rect_row_size > packet.data().data() + packet.data().size()) {
         LOG(ERROR) << "Invalid packet received";
-        return DECODE_ERROR;
+        return false;
       }
       memcpy(out, in, rect_row_size);
       in += rect_row_size;
@@ -69,18 +59,14 @@ VideoDecoder::DecodeResult VideoDecoderVerbatim::DecodePacket(
     }
   }
 
-  if (in != packet->data().data() + packet->data().size()) {
+  if (in != packet.data().data() + packet.data().size()) {
     LOG(ERROR) << "Invalid packet received";
-    return DECODE_ERROR;
+    return false;
   }
 
   updated_region_.op(region, SkRegion::kUnion_Op);
 
-  return DECODE_DONE;
-}
-
-VideoPacketFormat::Encoding VideoDecoderVerbatim::Encoding() {
-  return VideoPacketFormat::ENCODING_VERBATIM;
+  return true;
 }
 
 void VideoDecoderVerbatim::Invalidate(const SkISize& view_size,
diff --git a/remoting/codec/video_decoder_verbatim.h b/remoting/codec/video_decoder_verbatim.h
index 96b75e21..fa783dd 100644
--- a/remoting/codec/video_decoder_verbatim.h
+++ b/remoting/codec/video_decoder_verbatim.h
@@ -21,10 +21,8 @@ class VideoDecoderVerbatim : public VideoDecoder {
   VideoDecoderVerbatim();
 
   // VideoDecoder implementation.
-  virtual bool IsReadyForData() OVERRIDE;
   virtual void Initialize(const SkISize& screen_size) OVERRIDE;
-  virtual DecodeResult DecodePacket(const VideoPacket* packet) OVERRIDE;
-  virtual VideoPacketFormat::Encoding Encoding() OVERRIDE;
+  virtual bool DecodePacket(const VideoPacket& packet) OVERRIDE;
   virtual void Invalidate(const SkISize& view_size,
                           const SkRegion& region) OVERRIDE;
   virtual void RenderFrame(const SkISize& view_size,
diff --git a/remoting/codec/video_decoder_vp8.cc b/remoting/codec/video_decoder_vp8.cc
index 33896fb..48bf534 100644
--- a/remoting/codec/video_decoder_vp8.cc
+++ b/remoting/codec/video_decoder_vp8.cc
@@ -21,8 +21,6 @@ extern "C" {
 
 namespace remoting {
 
-enum { kBytesPerPixelRGB32 = 4 };
-
 const uint32 kTransparent = 0;
 
 VideoDecoderVp8::VideoDecoderVp8()
@@ -49,8 +47,7 @@ void VideoDecoderVp8::Initialize(const SkISize& screen_size) {
   transparent_region_.setRect(SkIRect::MakeSize(screen_size_));
 }
 
-VideoDecoder::DecodeResult VideoDecoderVp8::DecodePacket(
-    const VideoPacket* packet) {
+bool VideoDecoderVp8::DecodePacket(const VideoPacket& packet) {
   DCHECK_EQ(kReady, state_);
 
   // Initialize the codec as needed.
@@ -71,19 +68,19 @@ VideoDecoder::DecodeResult VideoDecoderVp8::DecodePacket(
       delete codec_;
       codec_ = NULL;
       state_ = kError;
-      return DECODE_ERROR;
+      return false;
     }
   }
 
   // Do the actual decoding.
   vpx_codec_err_t ret = vpx_codec_decode(
-      codec_, reinterpret_cast<const uint8*>(packet->data().data()),
-      packet->data().size(), NULL, 0);
+      codec_, reinterpret_cast<const uint8*>(packet.data().data()),
+      packet.data().size(), NULL, 0);
   if (ret != VPX_CODEC_OK) {
     LOG(INFO) << "Decoding failed:" << vpx_codec_err_to_string(ret) << "\n"
               << "Details: " << vpx_codec_error(codec_) << "\n"
               << vpx_codec_error_detail(codec_);
-    return DECODE_ERROR;
+    return false;
   }
 
   // Gets the decoded data.
@@ -91,13 +88,13 @@ VideoDecoder::DecodeResult VideoDecoderVp8::DecodePacket(
   vpx_image_t* image = vpx_codec_get_frame(codec_, &iter);
   if (!image) {
     LOG(INFO) << "No video frame decoded";
-    return DECODE_ERROR;
+    return false;
   }
   last_image_ = image;
 
   SkRegion region;
-  for (int i = 0; i < packet->dirty_rects_size(); ++i) {
-    Rect remoting_rect = packet->dirty_rects(i);
+  for (int i = 0; i < packet.dirty_rects_size(); ++i) {
+    Rect remoting_rect = packet.dirty_rects(i);
     SkIRect rect = SkIRect::MakeXYWH(remoting_rect.x(),
                                      remoting_rect.y(),
                                      remoting_rect.width(),
@@ -109,9 +106,9 @@ VideoDecoder::DecodeResult VideoDecoderVp8::DecodePacket(
 
   // Update the desktop shape region.
   SkRegion desktop_shape_region;
-  if (packet->has_use_desktop_shape()) {
-    for (int i = 0; i < packet->desktop_shape_rects_size(); ++i) {
-      Rect remoting_rect = packet->desktop_shape_rects(i);
+  if (packet.has_use_desktop_shape()) {
+    for (int i = 0; i < packet.desktop_shape_rects_size(); ++i) {
+      Rect remoting_rect = packet.desktop_shape_rects(i);
       SkIRect rect = SkIRect::MakeXYWH(remoting_rect.x(),
                                        remoting_rect.y(),
                                        remoting_rect.width(),
@@ -126,15 +123,7 @@ VideoDecoder::DecodeResult VideoDecoderVp8::DecodePacket(
 
   UpdateImageShapeRegion(&desktop_shape_region);
 
-  return DECODE_DONE;
-}
-
-bool VideoDecoderVp8::IsReadyForData() {
-  return state_ == kReady;
-}
-
-VideoPacketFormat::Encoding VideoDecoderVp8::Encoding() {
-  return VideoPacketFormat::ENCODING_VP8;
+  return true;
 }
 
 void VideoDecoderVp8::Invalidate(const SkISize& view_size,
@@ -272,11 +261,11 @@ void VideoDecoderVp8::FillRect(uint8* buffer,
                                const SkIRect& rect,
                                uint32 color) {
   uint32* ptr = reinterpret_cast<uint32*>(buffer + (rect.top() * stride) +
-      (rect.left() * kBytesPerPixelRGB32));
+      (rect.left() * kBytesPerPixel));
   int width = rect.width();
   for (int height = rect.height(); height > 0; --height) {
     std::fill(ptr, ptr + width, color);
-    ptr += stride / kBytesPerPixelRGB32;
+    ptr += stride / kBytesPerPixel;
   }
 }
 
diff --git a/remoting/codec/video_decoder_vp8.h b/remoting/codec/video_decoder_vp8.h
index 2efbd22..6912ab0 100644
--- a/remoting/codec/video_decoder_vp8.h
+++ b/remoting/codec/video_decoder_vp8.h
@@ -20,9 +20,7 @@ class VideoDecoderVp8 : public VideoDecoder {
 
   // VideoDecoder implementations.
   virtual void Initialize(const SkISize& screen_size) OVERRIDE;
-  virtual DecodeResult DecodePacket(const VideoPacket* packet) OVERRIDE;
-  virtual bool IsReadyForData() OVERRIDE;
-  virtual VideoPacketFormat::Encoding Encoding() OVERRIDE;
+  virtual bool DecodePacket(const VideoPacket& packet) OVERRIDE;
   virtual void Invalidate(const SkISize& view_size,
                           const SkRegion& region) OVERRIDE;
   virtual void RenderFrame(const SkISize& view_size,