summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorwez@chromium.org <wez@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-06-04 20:09:51 +0000
committerwez@chromium.org <wez@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-06-04 20:09:51 +0000
commitafa501d729384ede2b1d71d81e08f58ea842fd11 (patch)
tree22042c683104e3b89e857b27626840897e1260d4
parentd7f099601ba9868fde701526b9a8b79d0adfb506 (diff)
downloadchromium_src-afa501d729384ede2b1d71d81e08f58ea842fd11.zip
chromium_src-afa501d729384ede2b1d71d81e08f58ea842fd11.tar.gz
chromium_src-afa501d729384ede2b1d71d81e08f58ea842fd11.tar.bz2
Refactor EncoderVp8 to use SkRegion in place of RectVector.
This simplifies the EncoderVp8 capture preparation logic. The CL also changes the EncoderVp8 to CHECK if the capture pixel format is not supported, rather than silently failing. BUG=105401 TEST=Unit-tests and manual verification of Chromoting session video playback. Review URL: https://chromiumcodereview.appspot.com/10502003 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@140354 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--remoting/base/encoder_vp8.cc98
-rw-r--r--remoting/base/encoder_vp8.h26
-rw-r--r--remoting/base/encoder_vp8_unittest.cc23
3 files changed, 57 insertions, 90 deletions
diff --git a/remoting/base/encoder_vp8.cc b/remoting/base/encoder_vp8.cc
index fae1c27..2d8ae77 100644
--- a/remoting/base/encoder_vp8.cc
+++ b/remoting/base/encoder_vp8.cc
@@ -10,7 +10,6 @@
#include "remoting/base/capture_data.h"
#include "remoting/base/util.h"
#include "remoting/proto/video.pb.h"
-#include "third_party/skia/include/core/SkRegion.h"
extern "C" {
#define VPX_CODEC_DISABLE_COMPAT 1
@@ -149,26 +148,38 @@ bool EncoderVp8::Init(const SkISize& size) {
return true;
}
-// static
-SkIRect EncoderVp8::AlignAndClipRect(const SkIRect& rect,
- int width, int height) {
- SkIRect screen(SkIRect::MakeWH(RoundToTwosMultiple(width),
- RoundToTwosMultiple(height)));
- if (!screen.intersect(AlignRect(rect))) {
- screen = SkIRect::MakeWH(0, 0);
+void EncoderVp8::PrepareImage(scoped_refptr<CaptureData> capture_data,
+ SkRegion* updated_region) {
+ // Perform RGB->YUV conversion.
+ CHECK_EQ(capture_data->pixel_format(), media::VideoFrame::RGB32)
+ << "Only RGB32 is supported";
+
+ const SkRegion& region = capture_data->dirty_region();
+ if (region.isEmpty()) {
+ updated_region->setEmpty();
+ return;
}
- return screen;
-}
-bool EncoderVp8::PrepareImage(scoped_refptr<CaptureData> capture_data,
- RectVector* updated_rects) {
- // Perform RGB->YUV conversion.
- if (capture_data->pixel_format() != media::VideoFrame::RGB32) {
- LOG(ERROR) << "Only RGB32 is supported";
- return false;
+ // Align rectangles in the region, to avoid encoding artefacts.
+ // Note that this also results in rectangles with even-aligned positions,
+ // which is a requirement for some of the conversion routines to work.
+ std::vector<SkIRect> updated_rects;
+ for (SkRegion::Iterator r(region); !r.done(); r.next()) {
+ updated_rects.push_back(AlignRect(r.rect()));
+ }
+ if (!updated_rects.empty()) {
+ updated_region->setRects(&updated_rects[0], updated_rects.size());
}
- const SkRegion& region = capture_data->dirty_region();
+ // Clip to the screen again, in case it has non-aligned size.
+ // Note that we round the screen down to even dimensions to satisfy the
+ // limitations of some of the conversion routines.
+ int even_width = RoundToTwosMultiple(image_->w);
+ int even_height = RoundToTwosMultiple(image_->h);
+ updated_region->op(SkRegion(SkIRect::MakeWH(even_width, even_height)),
+ SkRegion::kIntersect_Op);
+
+ // Convert the updated region to YUV ready for encoding.
const uint8* in = capture_data->data_planes().data[0];
const int in_stride = capture_data->data_planes().strides[0];
const int plane_size =
@@ -179,35 +190,26 @@ bool EncoderVp8::PrepareImage(scoped_refptr<CaptureData> capture_data,
const int y_stride = image_->stride[0];
const int uv_stride = image_->stride[1];
- DCHECK(updated_rects->empty());
- for (SkRegion::Iterator r(region); !r.done(); r.next()) {
- // Align the rectangle, report it as updated.
- SkIRect rect = r.rect();
- rect = AlignAndClipRect(rect, image_->w, image_->h);
- if (!rect.isEmpty())
- updated_rects->push_back(rect);
-
+ for (SkRegion::Iterator r(*updated_region); !r.done(); r.next()) {
+ const SkIRect& rect = r.rect();
ConvertRGB32ToYUVWithRect(
in, y_out, u_out, v_out,
- rect.fLeft, rect.fTop, rect.width(), rect.height(),
+ rect.x(), rect.y(), rect.width(), rect.height(),
in_stride, y_stride, uv_stride);
}
- return true;
}
-void EncoderVp8::PrepareActiveMap(const RectVector& updated_rects) {
+void EncoderVp8::PrepareActiveMap(const SkRegion& updated_region) {
// Clear active map first.
memset(active_map_.get(), 0, active_map_width_ * active_map_height_);
- // Mark blocks at active.
- for (size_t i = 0; i < updated_rects.size(); ++i) {
- const SkIRect& r = updated_rects[i];
- CHECK(r.width() && r.height());
-
- int left = r.fLeft / kMacroBlockSize;
- int right = (r.fRight - 1) / kMacroBlockSize;
- int top = r.fTop / kMacroBlockSize;
- int bottom = (r.fBottom - 1) / kMacroBlockSize;
+ // Mark updated areas active.
+ for (SkRegion::Iterator r(updated_region); !r.done(); r.next()) {
+ const SkIRect& rect = r.rect();
+ int left = rect.left() / kMacroBlockSize;
+ int right = (rect.right() - 1) / kMacroBlockSize;
+ int top = rect.top() / kMacroBlockSize;
+ int bottom = (rect.bottom() - 1) / kMacroBlockSize;
CHECK(right < active_map_width_);
CHECK(bottom < active_map_height_);
@@ -230,13 +232,12 @@ void EncoderVp8::Encode(scoped_refptr<CaptureData> capture_data,
initialized_ = ret;
}
- RectVector updated_rects;
- if (!PrepareImage(capture_data, &updated_rects)) {
- NOTREACHED() << "Can't image data for encoding";
- }
+ // Convert the updated capture data ready for encode.
+ SkRegion updated_region;
+ PrepareImage(capture_data, &updated_region);
- // Update active map based on updated rectangles.
- PrepareActiveMap(updated_rects);
+ // Update active map based on updated region.
+ PrepareActiveMap(updated_region);
// Apply active map to the encoder.
vpx_active_map_t act_map;
@@ -284,6 +285,7 @@ void EncoderVp8::Encode(scoped_refptr<CaptureData> capture_data,
}
}
+ // Construct the VideoPacket message.
packet->mutable_format()->set_encoding(VideoPacketFormat::ENCODING_VP8);
packet->set_flags(VideoPacket::FIRST_PACKET | VideoPacket::LAST_PACKET |
VideoPacket::LAST_PARTITION);
@@ -291,12 +293,12 @@ void EncoderVp8::Encode(scoped_refptr<CaptureData> capture_data,
packet->mutable_format()->set_screen_height(capture_data->size().height());
packet->set_capture_time_ms(capture_data->capture_time_ms());
packet->set_client_sequence_number(capture_data->client_sequence_number());
- for (size_t i = 0; i < updated_rects.size(); ++i) {
+ for (SkRegion::Iterator r(updated_region); !r.done(); r.next()) {
Rect* rect = packet->add_dirty_rects();
- rect->set_x(updated_rects[i].fLeft);
- rect->set_y(updated_rects[i].fTop);
- rect->set_width(updated_rects[i].width());
- rect->set_height(updated_rects[i].height());
+ rect->set_x(r.rect().x());
+ rect->set_y(r.rect().y());
+ rect->set_width(r.rect().width());
+ rect->set_height(r.rect().height());
}
data_available_callback.Run(packet.Pass());
diff --git a/remoting/base/encoder_vp8.h b/remoting/base/encoder_vp8.h
index 81cfff0..42194c0 100644
--- a/remoting/base/encoder_vp8.h
+++ b/remoting/base/encoder_vp8.h
@@ -1,15 +1,13 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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 REMOTING_BASE_ENCODER_VP8_H_
#define REMOTING_BASE_ENCODER_VP8_H_
-#include <vector>
-
#include "base/gtest_prod_util.h"
#include "remoting/base/encoder.h"
-#include "third_party/skia/include/core/SkRect.h"
+#include "third_party/skia/include/core/SkRegion.h"
typedef struct vpx_codec_ctx vpx_codec_ctx_t;
typedef struct vpx_image vpx_image_t;
@@ -28,8 +26,6 @@ class EncoderVp8 : public Encoder {
const DataAvailableCallback& data_available_callback) OVERRIDE;
private:
- typedef std::vector<SkIRect> RectVector;
-
FRIEND_TEST_ALL_PREFIXES(EncoderVp8Test, AlignAndClipRect);
// Initialize the encoder. Returns true if successful.
@@ -39,21 +35,13 @@ class EncoderVp8 : public Encoder {
void Destroy();
// Prepare |image_| for encoding. Write updated rectangles into
- // |updated_rects|. Returns true if successful.
- bool PrepareImage(scoped_refptr<CaptureData> capture_data,
- RectVector* updated_rects);
+ // |updated_region|.
+ void PrepareImage(scoped_refptr<CaptureData> capture_data,
+ SkRegion* updated_region);
- // Update the active map according to |updated_rects|. Active map is then
+ // Update the active map according to |updated_region|. Active map is then
// given to the encoder to speed up encoding.
- void PrepareActiveMap(const RectVector& updated_rects);
-
- // Align the sides of the rectangle to multiples of 2 (expanding outwards),
- // but ensuring the result stays within the screen area (width, height).
- // If |rect| falls outside the screen area, return an empty rect.
- //
- // TODO(lambroslambrou): Pull this out if it's useful for other things than
- // VP8-encoding?
- static SkIRect AlignAndClipRect(const SkIRect& rect, int width, int height);
+ void PrepareActiveMap(const SkRegion& updated_region);
// True if the encoder is initialized.
bool initialized_;
diff --git a/remoting/base/encoder_vp8_unittest.cc b/remoting/base/encoder_vp8_unittest.cc
index dd8c3c6..0060869 100644
--- a/remoting/base/encoder_vp8_unittest.cc
+++ b/remoting/base/encoder_vp8_unittest.cc
@@ -62,27 +62,4 @@ TEST(EncoderVp8Test, TestSizeChangeNoLeak) {
base::Unretained(&callback)));
}
-TEST(EncoderVp8Test, AlignAndClipRect) {
- // Simple test case (no clipping).
- SkIRect r1(SkIRect::MakeXYWH(100, 200, 300, 400));
- EXPECT_EQ(EncoderVp8::AlignAndClipRect(r1, kIntMax, kIntMax), r1);
-
- // Should expand outward to r1.
- SkIRect r2(SkIRect::MakeXYWH(101, 201, 298, 398));
- EXPECT_EQ(EncoderVp8::AlignAndClipRect(r2, kIntMax, kIntMax), r1);
-
- // Test clipping to screen size.
- EXPECT_EQ(EncoderVp8::AlignAndClipRect(r1, 110, 220),
- SkIRect::MakeXYWH(100, 200, 10, 20));
-
- // Rectangle completely off-screen.
- EXPECT_TRUE(EncoderVp8::AlignAndClipRect(r1, 50, 50).isEmpty());
-
- // Clipping to odd-sized screen. An unlikely case, and we might not deal
- // with it cleanly in the encoder (we possibly lose 1px at right & bottom
- // of screen).
- EXPECT_EQ(EncoderVp8::AlignAndClipRect(r1, 199, 299),
- SkIRect::MakeXYWH(100, 200, 98, 98));
-}
-
} // namespace remoting