summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorwez <wez@chromium.org>2015-07-10 14:43:33 -0700
committerCommit bot <commit-bot@chromium.org>2015-07-10 21:45:29 +0000
commit9242f6c195c4b6ed37c1e565362f8324f9edc879 (patch)
tree1cbb3fd05eba7468c4878ac6ca649502da47cf1f
parentb3da857eaa84821c47719a9ebe9c464aedbe02c7 (diff)
downloadchromium_src-9242f6c195c4b6ed37c1e565362f8324f9edc879.zip
chromium_src-9242f6c195c4b6ed37c1e565362f8324f9edc879.tar.gz
chromium_src-9242f6c195c4b6ed37c1e565362f8324f9edc879.tar.bz2
Supply empty frames until codec is done with top-off.
Previously the VideoEncoderVpx would pass up to two empty/unchanged frames to the codec when in lossy-VP9 mode, to allow some quality top-off even if the desktop is otherwise idle. VideoEncoderVpx now calls the codec until it reports that no changes were encoded. This CL also uses a DesktopSize to hold the active-map dimensions. BUG=134202 Review URL: https://codereview.chromium.org/1213323003 Cr-Commit-Position: refs/heads/master@{#338371}
Diffstat
-rw-r--r--remoting/codec/codec_test.cc16
-rw-r--r--remoting/codec/codec_test.h4
-rw-r--r--remoting/codec/video_encoder_vpx.cc55
-rw-r--r--remoting/codec/video_encoder_vpx.h8
-rw-r--r--remoting/codec/video_encoder_vpx_unittest.cc10
5 files changed, 48 insertions, 45 deletions
diff --git a/remoting/codec/codec_test.cc b/remoting/codec/codec_test.cc
index 51b52f0..c5e1dae 100644
--- a/remoting/codec/codec_test.cc
+++ b/remoting/codec/codec_test.cc
@@ -284,7 +284,8 @@ void TestVideoEncoder(VideoEncoder* encoder, bool strict) {
}
}
-void TestVideoEncoderEmptyFrames(VideoEncoder* encoder, int topoff_frames) {
+void TestVideoEncoderEmptyFrames(VideoEncoder* encoder,
+ int max_topoff_frames) {
const DesktopSize kSize(640, 480);
scoped_ptr<DesktopFrame> frame(PrepareFrame(kSize));
@@ -292,12 +293,19 @@ void TestVideoEncoderEmptyFrames(VideoEncoder* encoder, int topoff_frames) {
webrtc::DesktopRect::MakeSize(kSize));
EXPECT_TRUE(encoder->Encode(*frame));
+ int topoff_frames = 0;
frame->mutable_updated_region()->Clear();
- for (int i=0; i < topoff_frames; ++i) {
- EXPECT_TRUE(encoder->Encode(*frame));
+ for (int i = 0; i < max_topoff_frames + 1; ++i) {
+ if (!encoder->Encode(*frame))
+ break;
+ topoff_frames++;
}
- EXPECT_FALSE(encoder->Encode(*frame));
+ // If top-off is enabled then our random frame contents should always
+ // trigger it, so expect at least one top-off frame - strictly, though,
+ // an encoder may not always need to top-off.
+ EXPECT_GE(topoff_frames, max_topoff_frames ? 1 : 0);
+ EXPECT_LE(topoff_frames, max_topoff_frames);
}
static void TestEncodeDecodeRects(VideoEncoder* encoder,
diff --git a/remoting/codec/codec_test.h b/remoting/codec/codec_test.h
index 0c143b7..e5d5bb6 100644
--- a/remoting/codec/codec_test.h
+++ b/remoting/codec/codec_test.h
@@ -28,9 +28,9 @@ void TestVideoEncoder(VideoEncoder* encoder, bool strict);
// Generate test data and test the encoder for a sequence of one "changed"
// frame followed by one or more "unchanged" frames, and verify that the
-// encoder sends exactly |topoff_frames| of non-empty data for unchanged
+// encoder sends up to |max_topoff_frames| of non-empty data for unchanged
// frames, after which it returns null frames.
-void TestVideoEncoderEmptyFrames(VideoEncoder* encoder, int topoff_frames);
+void TestVideoEncoderEmptyFrames(VideoEncoder* encoder, int max_topoff_frames);
// Generate test data and test the encoder and decoder pair.
//
diff --git a/remoting/codec/video_encoder_vpx.cc b/remoting/codec/video_encoder_vpx.cc
index adc6bf9..3c54652 100644
--- a/remoting/codec/video_encoder_vpx.cc
+++ b/remoting/codec/video_encoder_vpx.cc
@@ -266,21 +266,9 @@ scoped_ptr<VideoPacket> VideoEncoderVpx::Encode(
DCHECK_LE(32, frame.size().width());
DCHECK_LE(32, frame.size().height());
- if (!use_vp9_ || lossless_encode_) {
- // Neither VP8 nor VP9-lossless support top-off, so ignore unchanged frames.
- if (frame.updated_region().is_empty())
- return nullptr;
- } else {
- // Let VP9-lossy mode top-off, by continuing to pass it unchanged frames
- // for a short while.
- if (frame.updated_region().is_empty()) {
- if (topoff_frame_count_ == 0)
- return nullptr;
- topoff_frame_count_--;
- } else {
- topoff_frame_count_ = 2;
- }
- }
+ // If there is nothing to encode, and nothing to top-off, then return nothing.
+ if (frame.updated_region().is_empty() && !encode_unchanged_frame_)
+ return nullptr;
base::TimeTicks encode_start_time = base::TimeTicks::Now();
@@ -300,8 +288,8 @@ scoped_ptr<VideoPacket> VideoEncoderVpx::Encode(
// Apply active map to the encoder.
vpx_active_map_t act_map;
- act_map.rows = active_map_height_;
- act_map.cols = active_map_width_;
+ act_map.rows = active_map_size_.height();
+ act_map.cols = active_map_size_.width();
act_map.active_map = active_map_.get();
if (vpx_codec_control(codec_.get(), VP8E_SET_ACTIVEMAP, &act_map)) {
LOG(ERROR) << "Unable to apply active map";
@@ -322,6 +310,9 @@ scoped_ptr<VideoPacket> VideoEncoderVpx::Encode(
<< "Failed to fetch active map: "
<< vpx_codec_err_to_string(ret) << "\n";
UpdateRegionFromActiveMap(&updated_region);
+
+ // If the encoder output no changes then there's nothing left to top-off.
+ encode_unchanged_frame_ = !updated_region.is_empty();
}
// Read the encoded data.
@@ -357,7 +348,8 @@ scoped_ptr<VideoPacket> VideoEncoderVpx::Encode(
return packet.Pass();
}
-VideoEncoderVpx::VideoEncoderVpx(bool use_vp9) : use_vp9_(use_vp9) {
+VideoEncoderVpx::VideoEncoderVpx(bool use_vp9)
+ : use_vp9_(use_vp9), encode_unchanged_frame_(false) {
}
void VideoEncoderVpx::Configure(const webrtc::DesktopSize& size) {
@@ -370,9 +362,11 @@ void VideoEncoderVpx::Configure(const webrtc::DesktopSize& size) {
FreeImageIfMismatched(lossless_color_, size, &image_, &image_buffer_);
// Initialize active map.
- active_map_width_ = (size.width() + kMacroBlockSize - 1) / kMacroBlockSize;
- active_map_height_ = (size.height() + kMacroBlockSize - 1) / kMacroBlockSize;
- active_map_.reset(new uint8[active_map_width_ * active_map_height_]);
+ active_map_size_ = webrtc::DesktopSize(
+ (size.width() + kMacroBlockSize - 1) / kMacroBlockSize,
+ (size.height() + kMacroBlockSize - 1) / kMacroBlockSize);
+ active_map_.reset(
+ new uint8[active_map_size_.width() * active_map_size_.height()]);
// TODO(wez): Remove this hack once VPX can handle frame size reconfiguration.
// See https://code.google.com/p/webm/issues/detail?id=912.
@@ -507,7 +501,8 @@ void VideoEncoderVpx::PrepareImage(const webrtc::DesktopFrame& frame,
void VideoEncoderVpx::SetActiveMapFromRegion(
const webrtc::DesktopRegion& updated_region) {
// Clear active map first.
- memset(active_map_.get(), 0, active_map_width_ * active_map_height_);
+ memset(active_map_.get(), 0,
+ active_map_size_.width() * active_map_size_.height());
// Mark updated areas active.
for (webrtc::DesktopRegion::Iterator r(updated_region); !r.IsAtEnd();
@@ -517,14 +512,14 @@ void VideoEncoderVpx::SetActiveMapFromRegion(
int right = (rect.right() - 1) / kMacroBlockSize;
int top = rect.top() / kMacroBlockSize;
int bottom = (rect.bottom() - 1) / kMacroBlockSize;
- DCHECK_LT(right, active_map_width_);
- DCHECK_LT(bottom, active_map_height_);
+ DCHECK_LT(right, active_map_size_.width());
+ DCHECK_LT(bottom, active_map_size_.height());
- uint8* map = active_map_.get() + top * active_map_width_;
+ uint8* map = active_map_.get() + top * active_map_size_.width();
for (int y = top; y <= bottom; ++y) {
for (int x = left; x <= right; ++x)
map[x] = 1;
- map += active_map_width_;
+ map += active_map_size_.width();
}
}
}
@@ -532,11 +527,11 @@ void VideoEncoderVpx::SetActiveMapFromRegion(
void VideoEncoderVpx::UpdateRegionFromActiveMap(
webrtc::DesktopRegion* updated_region) {
const uint8* map = active_map_.get();
- for (int y = 0; y < active_map_height_; ++y) {
- for (int x0 = 0; x0 < active_map_width_;) {
+ for (int y = 0; y < active_map_size_.height(); ++y) {
+ for (int x0 = 0; x0 < active_map_size_.width();) {
int x1 = x0;
- for (; x1 < active_map_width_; ++x1) {
- if (map[y * active_map_width_ + x1] == 0)
+ for (; x1 < active_map_size_.width(); ++x1) {
+ if (map[y * active_map_size_.width() + x1] == 0)
break;
}
if (x1 > x0) {
diff --git a/remoting/codec/video_encoder_vpx.h b/remoting/codec/video_encoder_vpx.h
index dbfefeb..7d80592 100644
--- a/remoting/codec/video_encoder_vpx.h
+++ b/remoting/codec/video_encoder_vpx.h
@@ -73,12 +73,10 @@ class VideoEncoderVpx : public VideoEncoder {
// Active map used to optimize out processing of un-changed macroblocks.
scoped_ptr<uint8[]> active_map_;
- int active_map_width_ = 0;
- int active_map_height_ = 0;
+ webrtc::DesktopSize active_map_size_;
- // Number of "top-off" frames we've encoded since we were last passed a
- // frame containing an actual change. Used only when encoding with top-off.
- int topoff_frame_count_ = 0;
+ // True if the codec wants unchanged frames to finish topping-off with.
+ bool encode_unchanged_frame_;
// Used to help initialize VideoPackets from DesktopFrames.
VideoEncoderHelper helper_;
diff --git a/remoting/codec/video_encoder_vpx_unittest.cc b/remoting/codec/video_encoder_vpx_unittest.cc
index efe07d2..dfb3627 100644
--- a/remoting/codec/video_encoder_vpx_unittest.cc
+++ b/remoting/codec/video_encoder_vpx_unittest.cc
@@ -162,21 +162,23 @@ TEST(VideoEncoderVpxTest, DpiPropagation) {
EXPECT_EQ(packet->format().y_dpi(), 97);
}
-TEST(VideoEncoderVerbatimTest, Vp8EncodeUnchangedFrame) {
+TEST(VideoEncoderVpxTest, Vp8EncodeUnchangedFrame) {
scoped_ptr<VideoEncoderVpx> encoder(VideoEncoderVpx::CreateForVP8());
TestVideoEncoderEmptyFrames(encoder.get(), 0);
}
-TEST(VideoEncoderVerbatimTest, Vp9LosslessUnchangedFrame) {
+TEST(VideoEncoderVpxTest, Vp9LosslessUnchangedFrame) {
scoped_ptr<VideoEncoderVpx> encoder(VideoEncoderVpx::CreateForVP9());
encoder->SetLosslessEncode(true);
TestVideoEncoderEmptyFrames(encoder.get(), 0);
}
-TEST(VideoEncoderVerbatimTest, Vp9LossyUnchangedFrame) {
+TEST(VideoEncoderVpxTest, Vp9LossyUnchangedFrame) {
scoped_ptr<VideoEncoderVpx> encoder(VideoEncoderVpx::CreateForVP9());
encoder->SetLosslessEncode(false);
- TestVideoEncoderEmptyFrames(encoder.get(), 2);
+ // Expect that VP9+CR should generate no more than 10 top-off frames
+ // per cycle, and take no more than 2 cycles to top-off.
+ TestVideoEncoderEmptyFrames(encoder.get(), 20);
}
} // namespace remoting
generated by cgit v1.1 at 2025-03-18 05:07:36 +0000