diff options
-rw-r--r-- | remoting/client/rectangle_update_decoder.cc | 24 | ||||
-rw-r--r-- | remoting/client/rectangle_update_decoder.h | 9 | ||||
-rw-r--r-- | remoting/codec/codec_test.cc | 135 | ||||
-rw-r--r-- | remoting/codec/video_decoder.h | 10 | ||||
-rw-r--r-- | remoting/codec/video_decoder_verbatim.cc | 157 | ||||
-rw-r--r-- | remoting/codec/video_decoder_verbatim.h | 24 | ||||
-rw-r--r-- | remoting/codec/video_encoder.h | 17 | ||||
-rw-r--r-- | remoting/codec/video_encoder_verbatim.cc | 147 | ||||
-rw-r--r-- | remoting/codec/video_encoder_verbatim.h | 30 | ||||
-rw-r--r-- | remoting/codec/video_encoder_verbatim_unittest.cc | 15 | ||||
-rw-r--r-- | remoting/codec/video_encoder_vp8.cc | 39 | ||||
-rw-r--r-- | remoting/codec/video_encoder_vp8.h | 7 | ||||
-rw-r--r-- | remoting/codec/video_encoder_vp8_unittest.cc | 32 | ||||
-rw-r--r-- | remoting/host/client_session.cc | 4 | ||||
-rw-r--r-- | remoting/host/video_scheduler.cc | 31 | ||||
-rw-r--r-- | remoting/host/video_scheduler_unittest.cc | 14 | ||||
-rw-r--r-- | remoting/proto/video.proto | 49 | ||||
-rw-r--r-- | remoting/protocol/protobuf_video_writer.cc | 5 |
18 files changed, 177 insertions, 572 deletions
diff --git a/remoting/client/rectangle_update_decoder.cc b/remoting/client/rectangle_update_decoder.cc index 741357d..106c993 100644 --- a/remoting/client/rectangle_update_decoder.cc +++ b/remoting/client/rectangle_update_decoder.cc @@ -255,36 +255,28 @@ void RectangleUpdateDecoder::ProcessVideoPacket(scoped_ptr<VideoPacket> packet, } // Measure the latency between the last packet being received and presented. - bool last_packet = (packet->flags() & VideoPacket::LAST_PACKET) != 0; - base::Time decode_start; - if (last_packet) - decode_start = base::Time::Now(); + base::Time decode_start = base::Time::Now(); - base::Closure decode_done = - base::Bind(&RectangleUpdateDecoder::OnPacketDone, this, - last_packet, decode_start, done); + base::Closure decode_done = base::Bind( + &RectangleUpdateDecoder::OnPacketDone, this, decode_start, done); decode_task_runner_->PostTask(FROM_HERE, base::Bind( &RectangleUpdateDecoder::DecodePacket, this, base::Passed(&packet), decode_done)); } -void RectangleUpdateDecoder::OnPacketDone(bool last_packet, - base::Time decode_start, +void RectangleUpdateDecoder::OnPacketDone(base::Time decode_start, const base::Closure& done) { if (!main_task_runner_->BelongsToCurrentThread()) { main_task_runner_->PostTask(FROM_HERE, base::Bind( &RectangleUpdateDecoder::OnPacketDone, this, - last_packet, decode_start, done)); + decode_start, done)); return; } - // Record the latency between the final packet being received and - // presented. - if (last_packet) { - stats_.video_decode_ms()->Record( - (base::Time::Now() - decode_start).InMilliseconds()); - } + // Record the latency between the packet being received and presented. + stats_.video_decode_ms()->Record( + (base::Time::Now() - decode_start).InMilliseconds()); done.Run(); } diff --git a/remoting/client/rectangle_update_decoder.h b/remoting/client/rectangle_update_decoder.h index 457c724..6d46a1e 100644 --- a/remoting/client/rectangle_update_decoder.h +++ b/remoting/client/rectangle_update_decoder.h @@ -79,12 +79,9 @@ class RectangleUpdateDecoder // executed. void DecodePacket(scoped_ptr<VideoPacket> packet, const base::Closure& done); - // Callback method when a VideoPacket is processed. - // If |last_packet| is true then |decode_start| contains the timestamp when - // the packet will start to be processed. - void OnPacketDone(bool last_packet, - base::Time decode_start, - const base::Closure& done); + // Callback method when a VideoPacket is processed. |decode_start| contains + // the timestamp when the packet will start to be processed. + void OnPacketDone(base::Time decode_start, const base::Closure& done); scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_; scoped_refptr<base::SingleThreadTaskRunner> decode_task_runner_; diff --git a/remoting/codec/codec_test.cc b/remoting/codec/codec_test.cc index eff143d..83d1035 100644 --- a/remoting/codec/codec_test.cc +++ b/remoting/codec/codec_test.cc @@ -50,88 +50,6 @@ std::vector<std::vector<DesktopRect> > MakeTestRectLists(DesktopSize size) { namespace remoting { -// A class to test the message output of the encoder. -class VideoEncoderMessageTester { - public: - VideoEncoderMessageTester() - : begin_rect_(0), - rect_data_(0), - end_rect_(0), - state_(kWaitingForBeginRect), - strict_(false) { - } - - ~VideoEncoderMessageTester() { - EXPECT_EQ(begin_rect_, end_rect_); - EXPECT_GT(begin_rect_, 0); - EXPECT_EQ(kWaitingForBeginRect, state_); - if (strict_) { - EXPECT_TRUE(region_.Equals(received_region_)); - } - } - - // Test that we received the correct packet. - void ReceivedPacket(VideoPacket* packet) { - if (state_ == kWaitingForBeginRect) { - EXPECT_TRUE((packet->flags() & VideoPacket::FIRST_PACKET) != 0); - state_ = kWaitingForRectData; - ++begin_rect_; - - if (strict_) { - received_region_.AddRect(webrtc::DesktopRect::MakeXYWH( - packet->format().x(), packet->format().y(), - packet->format().width(), packet->format().height())); - } - } else { - EXPECT_FALSE((packet->flags() & VideoPacket::FIRST_PACKET) != 0); - } - - if (state_ == kWaitingForRectData) { - if (packet->has_data()) { - ++rect_data_; - } - - if ((packet->flags() & VideoPacket::LAST_PACKET) != 0) { - // Expect that we have received some data. - EXPECT_GT(rect_data_, 0); - rect_data_ = 0; - state_ = kWaitingForBeginRect; - ++end_rect_; - } - - if ((packet->flags() & VideoPacket::LAST_PARTITION) != 0) { - // LAST_PARTITION must always be marked with LAST_PACKET. - EXPECT_TRUE((packet->flags() & VideoPacket::LAST_PACKET) != 0); - } - } - } - - void set_strict(bool strict) { - strict_ = strict; - } - - void AddRects(const DesktopRect* rects, int count) { - region_.AddRects(rects, count); - } - - private: - enum State { - kWaitingForBeginRect, - kWaitingForRectData, - }; - - int begin_rect_; - int rect_data_; - int end_rect_; - State state_; - bool strict_; - - DesktopRegion region_; - DesktopRegion received_region_; - - DISALLOW_COPY_AND_ASSIGN(VideoEncoderMessageTester); -}; - class VideoDecoderTester { public: VideoDecoderTester(VideoDecoder* decoder, @@ -207,18 +125,12 @@ class VideoDecoderTester { ASSERT_TRUE(frame_); // Test the content of the update region. - // - // TODO(sergeyu): Change this to use DesktopRegion when it's capable of - // merging the rectangles. - SkRegion expected_region; - for (webrtc::DesktopRegion::Iterator it(expected_region_); - !it.IsAtEnd(); it.Advance()) { - expected_region.op( - SkIRect::MakeXYWH(it.rect().top(), it.rect().left(), - it.rect().width(), it.rect().height()), - SkRegion::kUnion_Op); + webrtc::DesktopRegion update_region; + for (SkRegion::Iterator i(update_region_); !i.done(); i.next()) { + update_region.AddRect(webrtc::DesktopRect::MakeXYWH( + i.rect().x(), i.rect().y(), i.rect().width(), i.rect().height())); } - EXPECT_EQ(expected_region, update_region_); + EXPECT_TRUE(expected_region_.Equals(update_region)); for (SkRegion::Iterator i(update_region_); !i.done(); i.next()) { const int stride = view_size_.width() * kBytesPerPixel; @@ -299,9 +211,8 @@ class VideoDecoderTester { // the message to other subprograms for validaton. class VideoEncoderTester { public: - VideoEncoderTester(VideoEncoderMessageTester* message_tester) - : message_tester_(message_tester), - decoder_tester_(NULL), + VideoEncoderTester() + : decoder_tester_(NULL), data_available_(0) { } @@ -311,24 +222,17 @@ class VideoEncoderTester { void DataAvailable(scoped_ptr<VideoPacket> packet) { ++data_available_; - message_tester_->ReceivedPacket(packet.get()); - // Send the message to the VideoDecoderTester. if (decoder_tester_) { decoder_tester_->ReceivedPacket(packet.get()); } } - void AddRects(const DesktopRect* rects, int count) { - message_tester_->AddRects(rects, count); - } - void set_decoder_tester(VideoDecoderTester* decoder_tester) { decoder_tester_ = decoder_tester; } private: - VideoEncoderMessageTester* message_tester_; VideoDecoderTester* decoder_tester_; int data_available_; @@ -356,19 +260,15 @@ static void TestEncodingRects(VideoEncoder* encoder, for (int i = 0; i < count; ++i) { frame->mutable_updated_region()->AddRect(rects[i]); } - tester->AddRects(rects, count); - encoder->Encode(frame, base::Bind( - &VideoEncoderTester::DataAvailable, base::Unretained(tester))); + scoped_ptr<VideoPacket> packet = encoder->Encode(*frame); + tester->DataAvailable(packet.Pass()); } void TestVideoEncoder(VideoEncoder* encoder, bool strict) { const int kSizes[] = {320, 319, 317, 150}; - VideoEncoderMessageTester message_tester; - message_tester.set_strict(strict); - - VideoEncoderTester tester(&message_tester); + VideoEncoderTester tester; for (size_t xi = 0; xi < arraysize(kSizes); ++xi) { for (size_t yi = 0; yi < arraysize(kSizes); ++yi) { @@ -394,7 +294,6 @@ static void TestEncodeDecodeRects(VideoEncoder* encoder, for (int i = 0; i < count; ++i) { frame->mutable_updated_region()->AddRect(rects[i]); } - encoder_tester->AddRects(rects, count); decoder_tester->AddRects(rects, count); // Generate random data for the updated region. @@ -412,8 +311,8 @@ static void TestEncodeDecodeRects(VideoEncoder* encoder, } } - encoder->Encode(frame, base::Bind(&VideoEncoderTester::DataAvailable, - base::Unretained(encoder_tester))); + scoped_ptr<VideoPacket> packet = encoder->Encode(*frame); + encoder_tester->DataAvailable(packet.Pass()); decoder_tester->VerifyResults(); decoder_tester->Reset(); } @@ -422,10 +321,7 @@ void TestVideoEncoderDecoder( VideoEncoder* encoder, VideoDecoder* decoder, bool strict) { DesktopSize kSize = DesktopSize(320, 240); - VideoEncoderMessageTester message_tester; - message_tester.set_strict(strict); - - VideoEncoderTester encoder_tester(&message_tester); + VideoEncoderTester encoder_tester; scoped_ptr<webrtc::DesktopFrame> frame = PrepareFrame(kSize); @@ -475,9 +371,8 @@ void TestVideoEncoderDecoderGradient(VideoEncoder* encoder, decoder_tester.set_frame(frame.get()); decoder_tester.AddRegion(frame->updated_region()); - encoder->Encode(frame.get(), - base::Bind(&VideoDecoderTester::ReceivedScopedPacket, - base::Unretained(&decoder_tester))); + scoped_ptr<VideoPacket> packet = encoder->Encode(*frame); + decoder_tester.ReceivedScopedPacket(packet.Pass()); decoder_tester.VerifyResultsApprox(expected_result->data(), max_error_limit, mean_error_limit); diff --git a/remoting/codec/video_decoder.h b/remoting/codec/video_decoder.h index 4e1d39f..4730d3a 100644 --- a/remoting/codec/video_decoder.h +++ b/remoting/codec/video_decoder.h @@ -15,18 +15,14 @@ namespace remoting { // Interface for a decoder that takes a stream of bytes from the network and // outputs frames of data. -// -// TODO(ajwong): Beef up this documentation once the API stablizes. 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_IN_PROGRESS - // indicates that the decoder must receive more data before the frame can be - // displayed. DECODE_ERROR is returned if there was an error in the stream. + // processed, and the frame can be displayed now. DECODE_ERROR is returned if + // there was an error in the stream. enum DecodeResult { - DECODE_ERROR = -1, - DECODE_IN_PROGRESS, + DECODE_ERROR, DECODE_DONE, }; diff --git a/remoting/codec/video_decoder_verbatim.cc b/remoting/codec/video_decoder_verbatim.cc index fa36690..b6b2179 100644 --- a/remoting/codec/video_decoder_verbatim.cc +++ b/remoting/codec/video_decoder_verbatim.cc @@ -14,31 +14,13 @@ namespace { const int kBytesPerPixel = 4; } // namespace - VideoDecoderVerbatim::VideoDecoderVerbatim() - : state_(kUninitialized), - clip_(SkIRect::MakeEmpty()), - row_pos_(0), - row_y_(0), - screen_size_(SkISize::Make(0, 0)) { -} + : screen_size_(SkISize::Make(0, 0)) {} -VideoDecoderVerbatim::~VideoDecoderVerbatim() { -} +VideoDecoderVerbatim::~VideoDecoderVerbatim() {} bool VideoDecoderVerbatim::IsReadyForData() { - switch (state_) { - case kUninitialized: - case kError: - return false; - case kReady: - case kProcessing: - case kPartitionDone: - case kDone: - return true; - } - NOTREACHED(); - return false; + return true; } void VideoDecoderVerbatim::Initialize(const SkISize& screen_size) { @@ -48,120 +30,53 @@ void VideoDecoderVerbatim::Initialize(const SkISize& screen_size) { screen_size_ = screen_size; // Allocate the screen buffer, if necessary. if (!screen_size_.isEmpty()) { - screen_buffer_.reset(new uint8[ - screen_size_.width() * screen_size_.height() * kBytesPerPixel]); + screen_buffer_.reset( + new uint8 + [screen_size_.width() * screen_size_.height() * kBytesPerPixel]); } - - state_ = kReady; } VideoDecoder::DecodeResult VideoDecoderVerbatim::DecodePacket( const VideoPacket* packet) { - UpdateStateForPacket(packet); - - if (state_ == kError) { - return DECODE_ERROR; - } - - const uint8* in = reinterpret_cast<const uint8*>(packet->data().data()); - const int in_size = packet->data().size(); - const int row_size = clip_.width() * kBytesPerPixel; - - int out_stride = screen_size_.width() * kBytesPerPixel; - uint8* out = screen_buffer_.get() + out_stride * (clip_.top() + row_y_) + - kBytesPerPixel * clip_.left(); - - // Consume all the data in the message. - int used = 0; - while (used < in_size) { - if (row_y_ >= clip_.height()) { - state_ = kError; - LOG(WARNING) << "Too much data is received for the given rectangle."; - return DECODE_ERROR; - } - - int bytes_to_copy = std::min(in_size - used, row_size - row_pos_); - memcpy(out + row_pos_, in + used, bytes_to_copy); - - used += bytes_to_copy; - row_pos_ += bytes_to_copy; - - // If this row is completely filled then move onto the next row. - if (row_pos_ == row_size) { - ++row_y_; - row_pos_ = 0; - out += out_stride; - } - } - - if (state_ == kPartitionDone || state_ == kDone) { - if (row_y_ < clip_.height()) { - state_ = kError; - LOG(WARNING) << "Received LAST_PACKET, but didn't get enough data."; + SkRegion region; + + 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); + SkIRect rect = SkIRect::MakeXYWH(proto_rect.x(), + proto_rect.y(), + proto_rect.width(), + proto_rect.height()); + region.op(rect, SkRegion::kUnion_Op); + + if (!SkIRect::MakeSize(screen_size_).contains(rect)) { + LOG(ERROR) << "Invalid packet received"; return DECODE_ERROR; } - updated_region_.op(clip_, SkRegion::kUnion_Op); - } - - if (state_ == kDone) { - return DECODE_DONE; - } else { - return DECODE_IN_PROGRESS; - } -} - -void VideoDecoderVerbatim::UpdateStateForPacket(const VideoPacket* packet) { - if (state_ == kError) { - return; - } - - if (packet->flags() & VideoPacket::FIRST_PACKET) { - if (state_ != kReady && state_ != kDone && state_ != kPartitionDone) { - state_ = kError; - LOG(WARNING) << "Received unexpected FIRST_PACKET."; - return; - } - - // Reset the buffer location status variables on the first packet. - clip_.setXYWH(packet->format().x(), packet->format().y(), - packet->format().width(), packet->format().height()); - if (!SkIRect::MakeSize(screen_size_).contains(clip_)) { - state_ = kError; - LOG(WARNING) << "Invalid clipping area received."; - return; + 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()) { + LOG(ERROR) << "Invalid packet received"; + return DECODE_ERROR; + } + memcpy(out, in, rect_row_size); + in += rect_row_size; + out += stride; } - - state_ = kProcessing; - row_pos_ = 0; - row_y_ = 0; } - if (state_ != kProcessing) { - state_ = kError; - LOG(WARNING) << "Received unexpected packet."; - return; - } - - if (packet->flags() & VideoPacket::LAST_PACKET) { - if (state_ != kProcessing) { - state_ = kError; - LOG(WARNING) << "Received unexpected LAST_PACKET."; - return; - } - state_ = kPartitionDone; + if (in != packet->data().data() + packet->data().size()) { + LOG(ERROR) << "Invalid packet received"; + return DECODE_ERROR; } - if (packet->flags() & VideoPacket::LAST_PARTITION) { - if (state_ != kPartitionDone) { - state_ = kError; - LOG(WARNING) << "Received unexpected LAST_PARTITION."; - return; - } - state_ = kDone; - } + updated_region_.op(region, SkRegion::kUnion_Op); - return; + return DECODE_DONE; } VideoPacketFormat::Encoding VideoDecoderVerbatim::Encoding() { diff --git a/remoting/codec/video_decoder_verbatim.h b/remoting/codec/video_decoder_verbatim.h index f058a7e..96b75e21 100644 --- a/remoting/codec/video_decoder_verbatim.h +++ b/remoting/codec/video_decoder_verbatim.h @@ -35,30 +35,6 @@ class VideoDecoderVerbatim : public VideoDecoder { virtual const SkRegion* GetImageShape() OVERRIDE; private: - enum State { - kUninitialized, - kReady, - kProcessing, - kPartitionDone, - kDone, - kError, - }; - - // Helper method. Called from DecodePacket to updated state of the decoder. - void UpdateStateForPacket(const VideoPacket* packet); - - // The internal state of the decoder. - State state_; - - // Keeps track of the updating rect. - SkIRect clip_; - - // The position in the row that we are updating. - int row_pos_; - - // The current row in the rect that we are updaing. - int row_y_; - // The region updated that hasn't been copied to the screen yet. SkRegion updated_region_; diff --git a/remoting/codec/video_encoder.h b/remoting/codec/video_encoder.h index a89e1e7..f0f8a2f 100644 --- a/remoting/codec/video_encoder.h +++ b/remoting/codec/video_encoder.h @@ -5,10 +5,7 @@ #ifndef REMOTING_CODEC_VIDEO_ENCODER_H_ #define REMOTING_CODEC_VIDEO_ENCODER_H_ -#include "base/basictypes.h" -#include "base/callback.h" - -class SkRegion; +#include "base/memory/scoped_ptr.h" namespace webrtc { class DesktopFrame; @@ -22,18 +19,10 @@ class VideoPacket; // interface is asynchronous to enable maximum throughput. class VideoEncoder { public: - - // DataAvailableCallback is called one or more times, for each chunk the - // underlying video encoder generates. - typedef base::Callback<void(scoped_ptr<VideoPacket>)> DataAvailableCallback; - virtual ~VideoEncoder() {} - // Encode an image stored in |frame|. Doesn't take ownership of |frame|. When - // encoded data is available, partial or full |data_available_callback| is - // called. - virtual void Encode(const webrtc::DesktopFrame* frame, - const DataAvailableCallback& data_available_callback) = 0; + // Encode an image stored in |frame|. + virtual scoped_ptr<VideoPacket> Encode(const webrtc::DesktopFrame& frame) = 0; }; } // namespace remoting diff --git a/remoting/codec/video_encoder_verbatim.cc b/remoting/codec/video_encoder_verbatim.cc index 4680362..424221b 100644 --- a/remoting/codec/video_encoder_verbatim.cc +++ b/remoting/codec/video_encoder_verbatim.cc @@ -6,6 +6,7 @@ #include "base/logging.h" #include "base/stl_util.h" +#include "base/time/time.h" #include "remoting/base/util.h" #include "remoting/proto/video.pb.h" #include "third_party/webrtc/modules/desktop_capture/desktop_frame.h" @@ -14,119 +15,71 @@ namespace remoting { static const int kPacketSize = 1024 * 1024; -VideoEncoderVerbatim::VideoEncoderVerbatim() - : max_packet_size_(kPacketSize) { -} +VideoEncoderVerbatim::VideoEncoderVerbatim() {} +VideoEncoderVerbatim::~VideoEncoderVerbatim() {} -void VideoEncoderVerbatim::SetMaxPacketSize(int size) { - max_packet_size_ = size; -} +scoped_ptr<VideoPacket> VideoEncoderVerbatim::Encode( + const webrtc::DesktopFrame& frame) { + CHECK(frame.data()); -VideoEncoderVerbatim::~VideoEncoderVerbatim() { -} + base::Time encode_start_time = base::Time::Now(); + scoped_ptr<VideoPacket> packet(new VideoPacket()); -void VideoEncoderVerbatim::Encode( - const webrtc::DesktopFrame* frame, - const DataAvailableCallback& data_available_callback) { - callback_ = data_available_callback; - encode_start_time_ = base::Time::Now(); + VideoPacketFormat* format = packet->mutable_format(); + format->set_encoding(VideoPacketFormat::ENCODING_VERBATIM); + if (!frame.size().equals(screen_size_)) { + screen_size_ = frame.size(); + format->set_screen_width(screen_size_.width()); + format->set_screen_height(screen_size_.height()); + } - webrtc::DesktopRegion::Iterator iter(frame->updated_region()); - while (!iter.IsAtEnd()) { + // Calculate output size. + size_t output_size = 0; + for (webrtc::DesktopRegion::Iterator iter(frame.updated_region()); + !iter.IsAtEnd(); iter.Advance()) { const webrtc::DesktopRect& rect = iter.rect(); - iter.Advance(); - EncodeRect(frame, rect, iter.IsAtEnd()); + output_size += rect.width() * rect.height() * + webrtc::DesktopFrame::kBytesPerPixel; } - callback_.Reset(); -} - -void VideoEncoderVerbatim::EncodeRect(const webrtc::DesktopFrame* frame, - const webrtc::DesktopRect& rect, - bool last) { - CHECK(frame->data()); - const int stride = frame->stride(); - const int bytes_per_pixel = 4; - const int row_size = bytes_per_pixel * rect.width(); + uint8_t* out = GetOutputBuffer(packet.get(), output_size); + const int in_stride = frame.stride(); - scoped_ptr<VideoPacket> packet(new VideoPacket()); - PrepareUpdateStart(frame, rect, packet.get()); - const uint8* in = frame->data() + - rect.top() * stride + rect.left() * bytes_per_pixel; - // TODO(hclam): Fill in the sequence number. - uint8* out = GetOutputBuffer(packet.get(), max_packet_size_); - int filled = 0; - int row_pos = 0; // Position in the current row in bytes. - int row_y = 0; // Current row. - while (row_y < rect.height()) { - // Prepare a message for sending out. - if (!packet.get()) { - packet.reset(new VideoPacket()); - out = GetOutputBuffer(packet.get(), max_packet_size_); - filled = 0; - } - - if (row_y < rect.height()) { - int bytes_to_copy = - std::min(row_size - row_pos, max_packet_size_ - filled); - memcpy(out + filled, in + row_pos, bytes_to_copy); - row_pos += bytes_to_copy; - filled += bytes_to_copy; - - // Jump to the next row when we've reached the end of the current row. - if (row_pos == row_size) { - row_pos = 0; - in += stride; - ++row_y; - } - } - - if (row_y == rect.height()) { - DCHECK_EQ(row_pos, 0); - - packet->mutable_data()->resize(filled); - packet->set_flags(packet->flags() | VideoPacket::LAST_PACKET); - - packet->set_capture_time_ms(frame->capture_time_ms()); - packet->set_encode_time_ms( - (base::Time::Now() - encode_start_time_).InMillisecondsRoundedUp()); - if (!frame->dpi().is_zero()) { - packet->mutable_format()->set_x_dpi(frame->dpi().x()); - packet->mutable_format()->set_y_dpi(frame->dpi().y()); - } - if (last) - packet->set_flags(packet->flags() | VideoPacket::LAST_PARTITION); + // Store all changed rectangles in the packet. + for (webrtc::DesktopRegion::Iterator iter(frame.updated_region()); + !iter.IsAtEnd(); iter.Advance()) { + const webrtc::DesktopRect& rect = iter.rect(); + const int row_size = webrtc::DesktopFrame::kBytesPerPixel * rect.width(); + const uint8_t* in = frame.data() + rect.top() * in_stride + + rect.left() * webrtc::DesktopFrame::kBytesPerPixel; + for (int y = rect.top(); y < rect.top() + rect.height(); ++y) { + memcpy(out, in, row_size); + out += row_size; + in += in_stride; } - // If we have filled the current packet, then send it. - if (filled == max_packet_size_ || row_y == rect.height()) { - packet->mutable_data()->resize(filled); - callback_.Run(packet.Pass()); - } + Rect* dirty_rect = packet->add_dirty_rects(); + dirty_rect->set_x(rect.left()); + dirty_rect->set_y(rect.top()); + dirty_rect->set_width(rect.width()); + dirty_rect->set_height(rect.height()); } -} -void VideoEncoderVerbatim::PrepareUpdateStart(const webrtc::DesktopFrame* frame, - const webrtc::DesktopRect& rect, - VideoPacket* packet) { - packet->set_flags(packet->flags() | VideoPacket::FIRST_PACKET); - - VideoPacketFormat* format = packet->mutable_format(); - format->set_x(rect.left()); - format->set_y(rect.top()); - format->set_width(rect.width()); - format->set_height(rect.height()); - format->set_encoding(VideoPacketFormat::ENCODING_VERBATIM); - if (frame->size().equals(screen_size_)) { - screen_size_ = frame->size(); - format->set_screen_width(screen_size_.width()); - format->set_screen_height(screen_size_.height()); + packet->set_capture_time_ms(frame.capture_time_ms()); + packet->set_encode_time_ms( + (base::Time::Now() - encode_start_time).InMillisecondsRoundedUp()); + if (!frame.dpi().is_zero()) { + packet->mutable_format()->set_x_dpi(frame.dpi().x()); + packet->mutable_format()->set_y_dpi(frame.dpi().y()); } + + return packet.Pass(); } -uint8* VideoEncoderVerbatim::GetOutputBuffer(VideoPacket* packet, size_t size) { +uint8_t* VideoEncoderVerbatim::GetOutputBuffer(VideoPacket* packet, + size_t size) { packet->mutable_data()->resize(size); - return reinterpret_cast<uint8*>(string_as_array(packet->mutable_data())); + return reinterpret_cast<uint8_t*>(string_as_array(packet->mutable_data())); } } // namespace remoting diff --git a/remoting/codec/video_encoder_verbatim.h b/remoting/codec/video_encoder_verbatim.h index f2078fc..54a8a03 100644 --- a/remoting/codec/video_encoder_verbatim.h +++ b/remoting/codec/video_encoder_verbatim.h @@ -5,7 +5,6 @@ #ifndef REMOTING_CODEC_VIDEO_ENCODER_VERBATIM_H_ #define REMOTING_CODEC_VIDEO_ENCODER_VERBATIM_H_ -#include "base/time/time.h" #include "remoting/codec/video_encoder.h" #include "remoting/proto/video.pb.h" #include "third_party/webrtc/modules/desktop_capture/desktop_geometry.h" @@ -19,40 +18,17 @@ class VideoEncoderVerbatim : public VideoEncoder { VideoEncoderVerbatim(); virtual ~VideoEncoderVerbatim(); - // Sets maximum size of data in video packets. Used by unittests. - void SetMaxPacketSize(int size); - // VideoEncoder interface. - virtual void Encode( - const webrtc::DesktopFrame* frame, - const DataAvailableCallback& data_available_callback) OVERRIDE; + virtual scoped_ptr<VideoPacket> Encode( + const webrtc::DesktopFrame& frame) OVERRIDE; private: - // Encode a single dirty |rect|. - void EncodeRect(const webrtc::DesktopFrame* frame, - const webrtc::DesktopRect& rect, - bool last); - - // Initializes first packet in a sequence of video packets to update screen - // rectangle |rect|. - void PrepareUpdateStart(const webrtc::DesktopFrame* frame, - const webrtc::DesktopRect& rect, - VideoPacket* packet); - // Allocates a buffer of the specified |size| inside |packet| and returns the // pointer to it. uint8* GetOutputBuffer(VideoPacket* packet, size_t size); - // Submit |packet| to |callback_|. - void SubmitMessage(VideoPacket* packet, size_t rect_index); - - DataAvailableCallback callback_; - base::Time encode_start_time_; - - // The most recent screen size. + // The most recent screen size. Used to detect screen size changes. webrtc::DesktopSize screen_size_; - - int max_packet_size_; }; } // namespace remoting diff --git a/remoting/codec/video_encoder_verbatim_unittest.cc b/remoting/codec/video_encoder_verbatim_unittest.cc index d6315ee..b9e72ac 100644 --- a/remoting/codec/video_encoder_verbatim_unittest.cc +++ b/remoting/codec/video_encoder_verbatim_unittest.cc @@ -15,23 +15,10 @@ TEST(VideoEncoderVerbatimTest, TestVideoEncoder) { TestVideoEncoder(encoder.get(), true); } -TEST(VideoEncoderVerbatimTest, TestVideoEncoderSmallOutputBuffer) { - scoped_ptr<VideoEncoderVerbatim> encoder(new VideoEncoderVerbatim()); - encoder->SetMaxPacketSize(16); - TestVideoEncoder(encoder.get(), true); -} - TEST(VideoEncoderVerbatimTest, EncodeAndDecode) { scoped_ptr<VideoEncoderVerbatim> encoder(new VideoEncoderVerbatim()); scoped_ptr<VideoDecoderVerbatim> decoder(new VideoDecoderVerbatim()); - TestVideoEncoderDecoder(encoder.get(), decoder.get(), false); -} - -TEST(VideoEncoderVerbatimTest, EncodeAndDecodeSmallOutputBuffer) { - scoped_ptr<VideoEncoderVerbatim> encoder(new VideoEncoderVerbatim()); - encoder->SetMaxPacketSize(16); - scoped_ptr<VideoDecoderVerbatim> decoder(new VideoDecoderVerbatim()); - TestVideoEncoderDecoder(encoder.get(), decoder.get(), false); + TestVideoEncoderDecoder(encoder.get(), decoder.get(), true); } } // namespace remoting diff --git a/remoting/codec/video_encoder_vp8.cc b/remoting/codec/video_encoder_vp8.cc index d9c9e88..179483d 100644 --- a/remoting/codec/video_encoder_vp8.cc +++ b/remoting/codec/video_encoder_vp8.cc @@ -144,9 +144,9 @@ bool VideoEncoderVp8::Init(const webrtc::DesktopSize& size) { return true; } -void VideoEncoderVp8::PrepareImage(const webrtc::DesktopFrame* frame, +void VideoEncoderVp8::PrepareImage(const webrtc::DesktopFrame& frame, SkRegion* updated_region) { - if (frame->updated_region().is_empty()) { + if (frame.updated_region().is_empty()) { updated_region->setEmpty(); return; } @@ -155,7 +155,7 @@ void VideoEncoderVp8::PrepareImage(const webrtc::DesktopFrame* frame, // This also ensures that all rectangles have even-aligned top-left, which // is required for ConvertRGBToYUVWithRect() to work. std::vector<SkIRect> aligned_rects; - for (webrtc::DesktopRegion::Iterator r(frame->updated_region()); + for (webrtc::DesktopRegion::Iterator r(frame.updated_region()); !r.IsAtEnd(); r.Advance()) { const webrtc::DesktopRect& rect = r.rect(); aligned_rects.push_back(AlignRect( @@ -171,8 +171,8 @@ void VideoEncoderVp8::PrepareImage(const webrtc::DesktopFrame* frame, SkRegion::kIntersect_Op); // Convert the updated region to YUV ready for encoding. - const uint8* rgb_data = frame->data(); - const int rgb_stride = frame->stride(); + const uint8* rgb_data = frame.data(); + const int rgb_stride = frame.stride(); const int y_stride = image_->stride[0]; DCHECK_EQ(image_->stride[1], image_->stride[2]); const int uv_stride = image_->stride[1]; @@ -211,17 +211,16 @@ void VideoEncoderVp8::PrepareActiveMap(const SkRegion& updated_region) { } } -void VideoEncoderVp8::Encode( - const webrtc::DesktopFrame* frame, - const DataAvailableCallback& data_available_callback) { - DCHECK_LE(32, frame->size().width()); - DCHECK_LE(32, frame->size().height()); +scoped_ptr<VideoPacket> VideoEncoderVp8::Encode( + const webrtc::DesktopFrame& frame) { + DCHECK_LE(32, frame.size().width()); + DCHECK_LE(32, frame.size().height()); base::Time encode_start_time = base::Time::Now(); if (!initialized_ || - !frame->size().equals(webrtc::DesktopSize(image_->w, image_->h))) { - bool ret = Init(frame->size()); + !frame.size().equals(webrtc::DesktopSize(image_->w, image_->h))) { + bool ret = Init(frame.size()); // TODO(hclam): Handle error better. CHECK(ret) << "Initialization of encoder failed"; initialized_ = ret; @@ -282,16 +281,14 @@ void VideoEncoderVp8::Encode( // Construct the VideoPacket message. packet->mutable_format()->set_encoding(VideoPacketFormat::ENCODING_VP8); - packet->set_flags(VideoPacket::FIRST_PACKET | VideoPacket::LAST_PACKET | - VideoPacket::LAST_PARTITION); - packet->mutable_format()->set_screen_width(frame->size().width()); - packet->mutable_format()->set_screen_height(frame->size().height()); - packet->set_capture_time_ms(frame->capture_time_ms()); + packet->mutable_format()->set_screen_width(frame.size().width()); + packet->mutable_format()->set_screen_height(frame.size().height()); + packet->set_capture_time_ms(frame.capture_time_ms()); packet->set_encode_time_ms( (base::Time::Now() - encode_start_time).InMillisecondsRoundedUp()); - if (!frame->dpi().is_zero()) { - packet->mutable_format()->set_x_dpi(frame->dpi().x()); - packet->mutable_format()->set_y_dpi(frame->dpi().y()); + if (!frame.dpi().is_zero()) { + packet->mutable_format()->set_x_dpi(frame.dpi().x()); + packet->mutable_format()->set_y_dpi(frame.dpi().y()); } for (SkRegion::Iterator r(updated_region); !r.done(); r.next()) { Rect* rect = packet->add_dirty_rects(); @@ -301,7 +298,7 @@ void VideoEncoderVp8::Encode( rect->set_height(r.rect().height()); } - data_available_callback.Run(packet.Pass()); + return packet.Pass(); } } // namespace remoting diff --git a/remoting/codec/video_encoder_vp8.h b/remoting/codec/video_encoder_vp8.h index 912c845..2e1ade1 100644 --- a/remoting/codec/video_encoder_vp8.h +++ b/remoting/codec/video_encoder_vp8.h @@ -25,9 +25,8 @@ class VideoEncoderVp8 : public VideoEncoder { virtual ~VideoEncoderVp8(); // VideoEncoder interface. - virtual void Encode( - const webrtc::DesktopFrame* frame, - const DataAvailableCallback& data_available_callback) OVERRIDE; + virtual scoped_ptr<VideoPacket> Encode( + const webrtc::DesktopFrame& frame) OVERRIDE; private: FRIEND_TEST_ALL_PREFIXES(VideoEncoderVp8Test, AlignAndClipRect); @@ -42,7 +41,7 @@ class VideoEncoderVp8 : public VideoEncoder { // |updated_region|. // // TODO(sergeyu): Update this code to use webrtc::DesktopRegion. - void PrepareImage(const webrtc::DesktopFrame* frame, + void PrepareImage(const webrtc::DesktopFrame& frame, SkRegion* updated_region); // Update the active map according to |updated_region|. Active map is then diff --git a/remoting/codec/video_encoder_vp8_unittest.cc b/remoting/codec/video_encoder_vp8_unittest.cc index 684910d..a8871ba 100644 --- a/remoting/codec/video_encoder_vp8_unittest.cc +++ b/remoting/codec/video_encoder_vp8_unittest.cc @@ -7,8 +7,6 @@ #include <limits> #include <vector> -#include "base/bind.h" -#include "base/callback.h" #include "base/memory/scoped_ptr.h" #include "remoting/codec/codec_test.h" #include "remoting/proto/video.pb.h" @@ -28,12 +26,6 @@ TEST(VideoEncoderVp8Test, TestVideoEncoder) { TestVideoEncoder(&encoder, false); } -class VideoEncoderCallback { - public: - void DataAvailable(scoped_ptr<VideoPacket> packet) { - } -}; - // Test that calling Encode with a differently-sized media::ScreenCaptureData // does not leak memory. TEST(VideoEncoderVp8Test, TestSizeChangeNoLeak) { @@ -41,29 +33,20 @@ TEST(VideoEncoderVp8Test, TestSizeChangeNoLeak) { int width = 1000; VideoEncoderVp8 encoder; - VideoEncoderCallback callback; scoped_ptr<webrtc::DesktopFrame> frame(new webrtc::BasicDesktopFrame( webrtc::DesktopSize(width, height))); - encoder.Encode(frame.get(), base::Bind(&VideoEncoderCallback::DataAvailable, - base::Unretained(&callback))); + scoped_ptr<VideoPacket> packet = encoder.Encode(*frame); + EXPECT_TRUE(packet); height /= 2; frame.reset(new webrtc::BasicDesktopFrame( webrtc::DesktopSize(width, height))); - encoder.Encode(frame.get(), base::Bind(&VideoEncoderCallback::DataAvailable, - base::Unretained(&callback))); + packet = encoder.Encode(*frame); + EXPECT_TRUE(packet); } -class VideoEncoderDpiCallback { - public: - void DataAvailable(scoped_ptr<VideoPacket> packet) { - EXPECT_EQ(packet->format().x_dpi(), 96); - EXPECT_EQ(packet->format().y_dpi(), 97); - } -}; - // Test that the DPI information is correctly propagated from the // media::ScreenCaptureData to the VideoPacket. TEST(VideoEncoderVp8Test, TestDpiPropagation) { @@ -71,14 +54,13 @@ TEST(VideoEncoderVp8Test, TestDpiPropagation) { int width = 32; VideoEncoderVp8 encoder; - VideoEncoderDpiCallback callback; scoped_ptr<webrtc::DesktopFrame> frame(new webrtc::BasicDesktopFrame( webrtc::DesktopSize(width, height))); frame->set_dpi(webrtc::DesktopVector(96, 97)); - encoder.Encode(frame.get(), - base::Bind(&VideoEncoderDpiCallback::DataAvailable, - base::Unretained(&callback))); + scoped_ptr<VideoPacket> packet = encoder.Encode(*frame); + EXPECT_EQ(packet->format().x_dpi(), 96); + EXPECT_EQ(packet->format().y_dpi(), 97); } } // namespace remoting diff --git a/remoting/host/client_session.cc b/remoting/host/client_session.cc index 6d424244..67f4a95 100644 --- a/remoting/host/client_session.cc +++ b/remoting/host/client_session.cc @@ -426,9 +426,7 @@ scoped_ptr<VideoEncoder> ClientSession::CreateVideoEncoder( const protocol::SessionConfig& config) { const protocol::ChannelConfig& video_config = config.video_config(); - if (video_config.codec == protocol::ChannelConfig::CODEC_VERBATIM) { - return scoped_ptr<VideoEncoder>(new remoting::VideoEncoderVerbatim()); - } else if (video_config.codec == protocol::ChannelConfig::CODEC_VP8) { + if (video_config.codec == protocol::ChannelConfig::CODEC_VP8) { return scoped_ptr<VideoEncoder>(new remoting::VideoEncoderVp8()); } diff --git a/remoting/host/video_scheduler.cc b/remoting/host/video_scheduler.cc index 883166b..5c95c7a 100644 --- a/remoting/host/video_scheduler.cc +++ b/remoting/host/video_scheduler.cc @@ -249,11 +249,8 @@ void VideoScheduler::SendVideoPacket(scoped_ptr<VideoPacket> packet) { if (!video_stub_) return; - base::Closure callback; - if ((packet->flags() & VideoPacket::LAST_PARTITION) != 0) - callback = base::Bind(&VideoScheduler::VideoFrameSentCallback, this); - - video_stub_->ProcessVideoPacket(packet.Pass(), callback); + video_stub_->ProcessVideoPacket( + packet.Pass(), base::Bind(&VideoScheduler::VideoFrameSentCallback, this)); } void VideoScheduler::VideoFrameSentCallback() { @@ -286,7 +283,6 @@ void VideoScheduler::EncodeFrame( // If there is nothing to encode then send an empty keep-alive packet. if (!frame || frame->updated_region().is_empty()) { scoped_ptr<VideoPacket> packet(new VideoPacket()); - packet->set_flags(VideoPacket::LAST_PARTITION); packet->set_client_sequence_number(sequence_number); network_task_runner_->PostTask( FROM_HERE, base::Bind(&VideoScheduler::SendVideoPacket, this, @@ -295,25 +291,16 @@ void VideoScheduler::EncodeFrame( return; } - encoder_->Encode( - frame.get(), base::Bind(&VideoScheduler::EncodedDataAvailableCallback, - this, sequence_number)); - capture_task_runner_->DeleteSoon(FROM_HERE, frame.release()); -} - -void VideoScheduler::EncodedDataAvailableCallback( - int64 sequence_number, - scoped_ptr<VideoPacket> packet) { - DCHECK(encode_task_runner_->BelongsToCurrentThread()); - + scoped_ptr<VideoPacket> packet = encoder_->Encode(*frame); packet->set_client_sequence_number(sequence_number); - bool last = (packet->flags() & VideoPacket::LAST_PACKET) != 0; - if (last) { - scheduler_.RecordEncodeTime( - base::TimeDelta::FromMilliseconds(packet->encode_time_ms())); - } + // Destroy the frame before sending |packet| because SendVideoPacket() may + // trigger another frame to be captured, and the screen capturer expects the + // old frame to be freed by then. + frame.reset(); + scheduler_.RecordEncodeTime( + base::TimeDelta::FromMilliseconds(packet->encode_time_ms())); network_task_runner_->PostTask( FROM_HERE, base::Bind(&VideoScheduler::SendVideoPacket, this, base::Passed(&packet))); diff --git a/remoting/host/video_scheduler_unittest.cc b/remoting/host/video_scheduler_unittest.cc index 2dd7afb..25567d2 100644 --- a/remoting/host/video_scheduler_unittest.cc +++ b/remoting/host/video_scheduler_unittest.cc @@ -37,8 +37,7 @@ namespace { ACTION(FinishEncode) { scoped_ptr<VideoPacket> packet(new VideoPacket()); - packet->set_flags(VideoPacket::LAST_PACKET | VideoPacket::LAST_PARTITION); - arg1.Run(packet.Pass()); + return packet.release(); } ACTION(FinishSend) { @@ -55,9 +54,11 @@ class MockVideoEncoder : public VideoEncoder { MockVideoEncoder(); virtual ~MockVideoEncoder(); - MOCK_METHOD2(Encode, void( - const webrtc::DesktopFrame* frame, - const DataAvailableCallback& data_available_callback)); + scoped_ptr<VideoPacket> Encode( + const webrtc::DesktopFrame& frame) { + return scoped_ptr<VideoPacket>(EncodePtr(frame)); + } + MOCK_METHOD1(EncodePtr, VideoPacket*(const webrtc::DesktopFrame& frame)); private: DISALLOW_COPY_AND_ASSIGN(MockVideoEncoder); @@ -158,7 +159,6 @@ TEST_F(VideoSchedulerTest, StartAndStop) { frame_.reset(new webrtc::BasicDesktopFrame( webrtc::DesktopSize(kWidth, kHeight))); - webrtc::DesktopFrame* frame_ptr = frame_.get(); // First the capturer is called. Expectation capturer_capture = EXPECT_CALL(*capturer, Capture(_)) @@ -166,7 +166,7 @@ TEST_F(VideoSchedulerTest, StartAndStop) { .WillRepeatedly(Invoke(this, &VideoSchedulerTest::OnCaptureFrame)); // Expect the encoder be called. - EXPECT_CALL(*encoder_, Encode(frame_ptr, _)) + EXPECT_CALL(*encoder_, EncodePtr(_)) .WillRepeatedly(FinishEncode()); // By default delete the arguments when ProcessVideoPacket is received. diff --git a/remoting/proto/video.proto b/remoting/proto/video.proto index b412857..0ee5c97 100644 --- a/remoting/proto/video.proto +++ b/remoting/proto/video.proto @@ -19,17 +19,9 @@ message VideoPacketFormat { ENCODING_VP8 = 2; }; - // X,Y coordinates (in screen pixels) for origin of this update. - optional int32 x = 1; - optional int32 y = 2; - - // Width, height (in screen pixels) for this update. - optional int32 width = 3; - optional int32 height = 4; - // The encoding used for this image update. optional Encoding encoding = 5 [default = ENCODING_INVALID]; - + // Width and height of the whole screen. optional int32 screen_width = 6; optional int32 screen_height = 7; @@ -49,45 +41,14 @@ message Rect { } message VideoPacket { - // Bitmasks for use in the flags field below. - // - // The encoder may fragment one update into multiple partitions. - // Each partition may be divided into multiple packets depending on - // how the encoder outputs data. Thus, one update can logically - // consist of multiple packets. The FIRST_PACKET and LAST_PACKET - // flags are used to indicate the start and end of a partition. The - // LAST_PARTITION flag is set for the last packet in the last - // partition. Here are notable consequences: - // * Both FIRST_PACKET and LAST_PACKET may be set if an update is only - // one packet long. - // * The VideoPacketFormat is only supplied in a FIRST_PACKET. - // * LAST_PARTITION can be set only in packet that has LAST_PACKET set. - // * An local update cannot change format between a FIRST_PACKET and - // a LAST_PACKET. - // * All packets in one logical update must be processed in order, and - // packets may not be skipped. - enum Flags { - FIRST_PACKET = 1; - LAST_PACKET = 2; - LAST_PARTITION = 4; - } - optional int32 flags = 1 [default = 0]; - - // The sequence number of the partial data for updating a rectangle. - optional int32 sequence_number = 2 [default = 0]; - - optional int32 timestamp = 3 [default = 0]; - - // This is provided on the first packet of the rectangle data, when - // the flags has FIRST_PACKET set. + // Deprecated. Must be set to 7 for backward compatibility. + optional int32 deprecated_flags = 1 [default = 7]; + optional VideoPacketFormat format = 4; optional bytes data = 5; - // This field is only for VP8 to provide out-of-band information of dirty - // rects. - // TODO(hclam): Remove this field when we can obtain this information from - // libvpx. + // List of rectangles updated by this frame. repeated Rect dirty_rects = 6; // Time in milliseconds spent in capturing this video frame. diff --git a/remoting/protocol/protobuf_video_writer.cc b/remoting/protocol/protobuf_video_writer.cc index 08e937b..7fd57c7 100644 --- a/remoting/protocol/protobuf_video_writer.cc +++ b/remoting/protocol/protobuf_video_writer.cc @@ -63,6 +63,11 @@ bool ProtobufVideoWriter::is_connected() { void ProtobufVideoWriter::ProcessVideoPacket(scoped_ptr<VideoPacket> packet, const base::Closure& done) { + // Older clients may expect deprecated flags field. Always set it to 7. + // + // TODO(sergeyu): Remove this field after M31 is released. + packet->set_deprecated_flags(7); + buffered_writer_.Write(SerializeAndFrameMessage(*packet), done); } |