From b047307ab561878d4357bdf7e617785e4dbd4f44 Mon Sep 17 00:00:00 2001 From: sergeyu Date: Tue, 18 Oct 2016 10:19:29 -0700 Subject: [PATCH] Don't use barcodes in ProtocolPerfTests Previously the perf tests were using a unique barcode to identify video frames when measuring latency. This is not longer necessary. Now the tests will use fake input event timestamps and video stats messages to measure latency. Committed: https://crrev.com/69d9c4d4f5a66e0f7bcf02c40058d7d2e122300b Review-Url: https://codereview.chromium.org/2420183002 Cr-Original-Commit-Position: refs/heads/master@{#425895} Cr-Commit-Position: refs/heads/master@{#425998} --- remoting/host/chromoting_host.cc | 39 +++---- remoting/host/chromoting_host.h | 9 +- remoting/host/chromoting_host_unittest.cc | 18 ++-- remoting/host/client_session.cc | 13 ++- remoting/host/client_session.h | 7 ++ remoting/test/cyclic_frame_generator.cc | 15 +-- remoting/test/cyclic_frame_generator.h | 31 +++--- remoting/test/frame_generator_util.cc | 50 --------- remoting/test/frame_generator_util.h | 7 -- remoting/test/protocol_perftest.cc | 123 ++++++++++++++-------- remoting/test/scroll_frame_generator.cc | 13 +-- remoting/test/scroll_frame_generator.h | 13 +-- 12 files changed, 157 insertions(+), 181 deletions(-) diff --git a/remoting/host/chromoting_host.cc b/remoting/host/chromoting_host.cc index c137d55d2ba337..eb9e182c548aaa 100644 --- a/remoting/host/chromoting_host.cc +++ b/remoting/host/chromoting_host.cc @@ -177,30 +177,24 @@ void ChromotingHost::OnSessionAuthenticated(ClientSession* client) { login_backoff_.Reset(); - // Disconnect all other clients. |it| should be advanced before Disconnect() - // is called to avoid it becoming invalid when the client is removed from - // the list. - ClientList::iterator it = clients_.begin(); + // Disconnect all clients, except |client|. base::WeakPtr self = weak_factory_.GetWeakPtr(); - while (it != clients_.end()) { - ClientSession* other_client = *it++; - if (other_client != client) { - other_client->DisconnectSession(protocol::OK); - - // Quit if the host was destroyed. - if (!self) - return; - } + while (clients_.size() > 1) { + clients_[(clients_.front().get() == client) ? 1 : 0]->DisconnectSession( + protocol::OK); + + // Quit if the host was destroyed. + if (!self) + return; } // Disconnects above must have destroyed all other clients. DCHECK_EQ(clients_.size(), 1U); + DCHECK(clients_.front().get() == client); // Notify observers that there is at least one authenticated client. - const std::string& jid = client->client_jid(); - for (auto& observer : status_observers_) - observer.OnClientAuthenticated(jid); + observer.OnClientAuthenticated(client->client_jid()); } void ChromotingHost::OnSessionChannelsConnected(ClientSession* client) { @@ -222,13 +216,16 @@ void ChromotingHost::OnSessionAuthenticationFailed(ClientSession* client) { void ChromotingHost::OnSessionClosed(ClientSession* client) { DCHECK(CalledOnValidThread()); - ClientList::iterator it = std::find(clients_.begin(), clients_.end(), client); + ClientSessions::iterator it = + std::find_if(clients_.begin(), clients_.end(), + [client](const std::unique_ptr& item) { + return item.get() == client; + }); CHECK(it != clients_.end()); bool was_authenticated = client->is_authenticated(); std::string jid = client->client_jid(); clients_.erase(it); - delete client; if (was_authenticated) { for (auto& observer : status_observers_) @@ -277,11 +274,9 @@ void ChromotingHost::OnIncomingSession( } // Create a ClientSession object. - ClientSession* client = new ClientSession( + clients_.push_back(base::MakeUnique( this, std::move(connection), desktop_environment_factory_, - max_session_duration_, pairing_registry_, extensions_.get()); - - clients_.push_back(client); + max_session_duration_, pairing_registry_, extensions_.get())); } void ChromotingHost::DisconnectAllClients() { diff --git a/remoting/host/chromoting_host.h b/remoting/host/chromoting_host.h index 5186988f0b9eda..01a6d1715d8b0d 100644 --- a/remoting/host/chromoting_host.h +++ b/remoting/host/chromoting_host.h @@ -5,9 +5,9 @@ #ifndef REMOTING_HOST_CHROMOTING_HOST_H_ #define REMOTING_HOST_CHROMOTING_HOST_H_ -#include #include #include +#include #include "base/macros.h" #include "base/memory/ref_counted.h" @@ -66,6 +66,8 @@ class ChromotingHost : public base::NonThreadSafe, public ClientSession::EventHandler, public HostStatusMonitor { public: + typedef std::vector> ClientSessions; + // |desktop_environment_factory| must outlive this object. ChromotingHost( DesktopEnvironmentFactory* desktop_environment_factory, @@ -133,6 +135,8 @@ class ChromotingHost : public base::NonThreadSafe, pairing_registry_ = pairing_registry; } + const ClientSessions& client_sessions_for_tests() { return clients_; } + base::WeakPtr AsWeakPtr() { return weak_factory_.GetWeakPtr(); } @@ -140,7 +144,6 @@ class ChromotingHost : public base::NonThreadSafe, private: friend class ChromotingHostTest; - typedef std::list ClientList; typedef ScopedVector HostExtensionList; // Immediately disconnects all active clients. Host-internal components may @@ -162,7 +165,7 @@ class ChromotingHost : public base::NonThreadSafe, base::ObserverList status_observers_; // The connections to remote clients. - ClientList clients_; + ClientSessions clients_; // True if the host has been started. bool started_; diff --git a/remoting/host/chromoting_host_unittest.cc b/remoting/host/chromoting_host_unittest.cc index f01539bf7886da..5c631f052d317b 100644 --- a/remoting/host/chromoting_host_unittest.cc +++ b/remoting/host/chromoting_host_unittest.cc @@ -136,7 +136,7 @@ class ChromotingHostTest : public testing::Test { get_client(connection_index) = client_ptr; // |host| is responsible for deleting |client| from now on. - host_->clients_.push_back(client.release()); + host_->clients_.push_back(std::move(client)); if (authenticate) { client_ptr->OnConnectionAuthenticated(); @@ -244,11 +244,6 @@ class ChromotingHostTest : public testing::Test { return (connection_index == 0) ? client1_ : client2_; } - // Returns the list of clients of the host_. - std::list& get_clients_from_host() { - return host_->clients_; - } - const std::string& get_session_jid(int connection_index) { return (connection_index == 0) ? session_jid1_ : session_jid2_; } @@ -338,7 +333,8 @@ TEST_F(ChromotingHostTest, LoginBackOffUponConnection) { host_->OnIncomingSession(session_unowned1_.release(), &response); EXPECT_EQ(protocol::SessionManager::ACCEPT, response); - host_->OnSessionAuthenticating(get_clients_from_host().front()); + host_->OnSessionAuthenticating( + host_->client_sessions_for_tests().front().get()); host_->OnIncomingSession(session_unowned2_.get(), &response); EXPECT_EQ(protocol::SessionManager::OVERLOAD, response); } @@ -362,13 +358,15 @@ TEST_F(ChromotingHostTest, LoginBackOffUponAuthenticating) { EXPECT_EQ(protocol::SessionManager::ACCEPT, response); // This will set the backoff. - host_->OnSessionAuthenticating(get_clients_from_host().front()); + host_->OnSessionAuthenticating( + host_->client_sessions_for_tests().front().get()); // This should disconnect client2. - host_->OnSessionAuthenticating(get_clients_from_host().back()); + host_->OnSessionAuthenticating( + host_->client_sessions_for_tests().back().get()); // Verify that the host only has 1 client at this point. - EXPECT_EQ(get_clients_from_host().size(), 1U); + EXPECT_EQ(host_->client_sessions_for_tests().size(), 1U); } TEST_F(ChromotingHostTest, OnSessionRouteChange) { diff --git a/remoting/host/client_session.cc b/remoting/host/client_session.cc index dcb87044efc8a0..f3fcbc31758969 100644 --- a/remoting/host/client_session.cc +++ b/remoting/host/client_session.cc @@ -295,6 +295,9 @@ void ClientSession::CreateMediaStreams() { // Pause capturing if necessary. video_stream_->Pause(pause_video_); + + if (event_timestamp_source_for_tests_) + video_stream_->SetEventTimestampsSource(event_timestamp_source_for_tests_); } void ClientSession::OnConnectionChannelsConnected() { @@ -404,9 +407,17 @@ ClientSessionControl* ClientSession::session_control() { return this; } -std::unique_ptr ClientSession::CreateClipboardProxy() { +void ClientSession::SetEventTimestampsSourceForTests( + scoped_refptr + event_timestamp_source) { DCHECK(CalledOnValidThread()); + event_timestamp_source_for_tests_ = event_timestamp_source; + if (video_stream_) + video_stream_->SetEventTimestampsSource(event_timestamp_source_for_tests_); +} +std::unique_ptr ClientSession::CreateClipboardProxy() { + DCHECK(CalledOnValidThread()); return base::MakeUnique( client_clipboard_factory_.GetWeakPtr(), base::ThreadTaskRunnerHandle::Get()); diff --git a/remoting/host/client_session.h b/remoting/host/client_session.h index 96e177fca209b0..8f10358b016f6b 100644 --- a/remoting/host/client_session.h +++ b/remoting/host/client_session.h @@ -140,6 +140,10 @@ class ClientSession : public base::NonThreadSafe, return client_capabilities_.get(); } + void SetEventTimestampsSourceForTests( + scoped_refptr + event_timestamp_source); + private: // Creates a proxy for sending clipboard events to the client. std::unique_ptr CreateClipboardProxy(); @@ -237,6 +241,9 @@ class ClientSession : public base::NonThreadSafe, // then it's stored in |pending_video_layout_message_|. std::unique_ptr pending_video_layout_message_; + scoped_refptr + event_timestamp_source_for_tests_; + // Used to disable callbacks to |this| once DisconnectSession() has been // called. base::WeakPtrFactory weak_factory_; diff --git a/remoting/test/cyclic_frame_generator.cc b/remoting/test/cyclic_frame_generator.cc index 11c8bc57a2d8eb..ef59810efc3f36 100644 --- a/remoting/test/cyclic_frame_generator.cc +++ b/remoting/test/cyclic_frame_generator.cc @@ -48,6 +48,7 @@ void CyclicFrameGenerator::SetTickClock(base::TickClock* tick_clock) { std::unique_ptr CyclicFrameGenerator::GenerateFrame( webrtc::SharedMemoryFactory* shared_memory_factory) { base::TimeTicks now = clock_->NowTicks(); + int frame_id = (now - started_time_) / cursor_blink_period_; int reference_frame = ((now - started_time_) / frame_cycle_period_) % reference_frames_.size(); @@ -80,20 +81,15 @@ std::unique_ptr CyclicFrameGenerator::GenerateFrame( last_frame_type_ = ChangeType::NO_CHANGES; } - if (draw_barcode_) - DrawBarcode(frame_id, frame_id != last_frame_id_, frame.get()); - last_reference_frame_ = reference_frame; last_cursor_state_ = cursor_state; - last_frame_id_ = frame_id; return frame; } CyclicFrameGenerator::ChangeInfoList CyclicFrameGenerator::GetChangeList( - webrtc::DesktopFrame* frame) { - CHECK(draw_barcode_); - int frame_id = ReadBarcode(*frame); + base::TimeTicks timestamp) { + int frame_id = (timestamp - started_time_) / cursor_blink_period_; CHECK_GE(frame_id, last_identifier_frame_); ChangeInfoList result; @@ -110,5 +106,10 @@ CyclicFrameGenerator::ChangeInfoList CyclicFrameGenerator::GetChangeList( return result; } +protocol::InputEventTimestamps CyclicFrameGenerator::TakeLastEventTimestamps() { + base::TimeTicks now = clock_->NowTicks(); + return protocol::InputEventTimestamps{now, now}; +} + } // namespace test } // namespace remoting diff --git a/remoting/test/cyclic_frame_generator.h b/remoting/test/cyclic_frame_generator.h index d03118a718921b..4c2fa4581117e2 100644 --- a/remoting/test/cyclic_frame_generator.h +++ b/remoting/test/cyclic_frame_generator.h @@ -9,8 +9,10 @@ #include #include +#include "base/callback.h" #include "base/memory/ref_counted.h" #include "base/time/default_tick_clock.h" +#include "remoting/protocol/input_event_timestamps.h" #include "third_party/webrtc/modules/desktop_capture/desktop_capturer.h" namespace remoting { @@ -21,8 +23,7 @@ namespace test { // loads a sequence of reference frames and switches between them with the // specified frequency (every 2 seconds by default). Between reference frames it // also generate frames with small changes which simulate a blinking cursor. -class CyclicFrameGenerator - : public base::RefCountedThreadSafe { +class CyclicFrameGenerator : public protocol::InputEventTimestampsSource { public: enum class ChangeType { // No changes. @@ -60,25 +61,22 @@ class CyclicFrameGenerator void SetTickClock(base::TickClock* tick_clock); - // When |draw_barcode| is set to true a barcode is drawn on each generated - // frame. This makes it possible to call GetChangeList() to identify the frame - // by its content. - void set_draw_barcode(bool draw_barcode) { draw_barcode_ = draw_barcode; } - std::unique_ptr GenerateFrame( webrtc::SharedMemoryFactory* shared_memory_factory); ChangeType last_frame_type() { return last_frame_type_; } - // Identifies |frame| by its content and returns list of ChangeInfo for all - // changes between the frame passed to the previous GetChangeList() call and - // this one. GetChangeList() must be called for the frames in the order in - // which they were received, which is expected to match the order in which - // they are generated. - ChangeInfoList GetChangeList(webrtc::DesktopFrame* frame); + // Returns list of changes for the frame with the specified |timestamp|, which + // should be an event timestamp generated by InputEventTimestampsSource + // implemented in this class. Must be called in the same order in which frames + // were generated. + ChangeInfoList GetChangeList(base::TimeTicks timestamp); + + // InputEventTimestampsSource interface. + protocol::InputEventTimestamps TakeLastEventTimestamps() override; private: - ~CyclicFrameGenerator(); + ~CyclicFrameGenerator() override; friend class base::RefCountedThreadSafe; std::vector> reference_frames_; @@ -92,9 +90,6 @@ class CyclicFrameGenerator // By default blink the cursor 4 times per seconds. base::TimeDelta cursor_blink_period_ = base::TimeDelta::FromMilliseconds(250); - // Id of the last frame encoded on the barcode. - int last_frame_id_ = -1; - // Index of the reference frame used to render the last generated frame. int last_reference_frame_ = -1; @@ -103,8 +98,6 @@ class CyclicFrameGenerator ChangeType last_frame_type_ = ChangeType::NO_CHANGES; - bool draw_barcode_ = false; - base::TimeTicks started_time_; // frame_id of the frame passed to the last GetChangeList() call. diff --git a/remoting/test/frame_generator_util.cc b/remoting/test/frame_generator_util.cc index c84115a482a8a2..bf78b0f14ae415 100644 --- a/remoting/test/frame_generator_util.cc +++ b/remoting/test/frame_generator_util.cc @@ -15,14 +15,6 @@ namespace remoting { namespace test { -namespace { -const int kBarcodeCellWidth = 8; -const int kBarcodeCellHeight = 8; -const int kBarcodeBits = 10; -const int kBarcodeBlackThreshold = 85; -const int kBarcodeWhiteThreshold = 170; -} // namespace - std::unique_ptr LoadDesktopFrameFromPng( const char* name) { base::FilePath file_path; @@ -59,47 +51,5 @@ void DrawRect(webrtc::DesktopFrame* frame, } } -void DrawBarcode(int value, bool changed, webrtc::DesktopFrame* frame) { - CHECK(value < (1 << kBarcodeBits)); - for (int i = 0; i < kBarcodeBits; ++i) { - DrawRect(frame, webrtc::DesktopRect::MakeXYWH(i * kBarcodeCellWidth, 0, - kBarcodeCellWidth, - kBarcodeCellHeight), - (value & 1) ? 0xffffffff : 0xff000000); - value >>= 1; - } - if (changed) { - frame->mutable_updated_region()->AddRect(webrtc::DesktopRect::MakeXYWH( - 0, 0, kBarcodeCellWidth * kBarcodeBits, kBarcodeCellHeight)); - } -} - -int ReadBarcode(const webrtc::DesktopFrame& frame) { - int result = 0; - for (int i = kBarcodeBits - 1; i >= 0; --i) { - // Sample barcode in the center of the cell for each bit. - int x = i * kBarcodeCellWidth + kBarcodeCellWidth / 2; - int y = kBarcodeCellHeight / 2; - uint8_t* data = frame.GetFrameDataAtPos(webrtc::DesktopVector(x, y)); - int b = data[0]; - int g = data[1]; - int r = data[2]; - bool bit = 0; - if (b > kBarcodeWhiteThreshold && g > kBarcodeWhiteThreshold && - r > kBarcodeWhiteThreshold) { - bit = 1; - } else if (b < kBarcodeBlackThreshold && g < kBarcodeBlackThreshold && - r < kBarcodeBlackThreshold) { - bit = 0; - } else { - LOG(FATAL) << "Invalid barcode."; - } - result <<= 1; - if (bit) - result |= 1; - } - return result; -} - } // namespace test } // namespace remoting diff --git a/remoting/test/frame_generator_util.h b/remoting/test/frame_generator_util.h index e0f7c53422520e..719ce33e23fc07 100644 --- a/remoting/test/frame_generator_util.h +++ b/remoting/test/frame_generator_util.h @@ -24,13 +24,6 @@ void DrawRect(webrtc::DesktopFrame* frame, webrtc::DesktopRect rect, uint32_t color); -// Draws barcode that encodes |value| on the |frame|. If |changed| is true then -// frame->updated_region() will be updated as well. -void DrawBarcode(int value, bool changed, webrtc::DesktopFrame* frame); - -// Reads barcode from the frame. -int ReadBarcode(const webrtc::DesktopFrame& frame); - } // namespace test } // namespace remoting diff --git a/remoting/test/protocol_perftest.cc b/remoting/test/protocol_perftest.cc index f243cfb4be9904..f7d0907ac90e2b 100644 --- a/remoting/test/protocol_perftest.cc +++ b/remoting/test/protocol_perftest.cc @@ -163,6 +163,10 @@ class ProtocolPerfTest // FrameStatsConsumer interface. void OnVideoFrameStats(const protocol::FrameStats& frame_stats) override { + // Ignore store stats for empty frames. + if (!frame_stats.host_stats.frame_size) + return; + frame_stats_.push_back(frame_stats); if (waiting_frame_stats_loop_ && @@ -172,6 +176,14 @@ class ProtocolPerfTest } // HostStatusObserver interface. + void OnClientAuthenticated(const std::string& jid) override { + if (event_timestamp_source_) { + auto& session = host_->client_sessions_for_tests().front(); + session->SetEventTimestampsSourceForTests( + std::move(event_timestamp_source_)); + } + } + void OnClientConnected(const std::string& jid) override { message_loop_.task_runner()->PostTask( FROM_HERE, base::Bind(&ProtocolPerfTest::OnHostConnectedMainThread, @@ -360,6 +372,8 @@ class ProtocolPerfTest base::Thread decode_thread_; std::unique_ptr desktop_environment_factory_; + scoped_refptr event_timestamp_source_; + FakeCursorShapeStub cursor_shape_stub_; std::unique_ptr protocol_config_; @@ -436,58 +450,76 @@ INSTANTIATE_TEST_CASE_P( void ProtocolPerfTest::MeasureTotalLatency(bool use_webrtc) { scoped_refptr frame_generator = test::CyclicFrameGenerator::Create(); - frame_generator->set_draw_barcode(true); - desktop_environment_factory_->set_frame_generator( base::Bind(&test::CyclicFrameGenerator::GenerateFrame, frame_generator)); + event_timestamp_source_ = frame_generator; StartHostAndClient(use_webrtc); ASSERT_NO_FATAL_FAILURE(WaitConnected()); - int skipped_frames = 0; - while (skipped_frames < 10) { - std::unique_ptr frame = ReceiveFrame(); - test::CyclicFrameGenerator::ChangeInfoList changes = - frame_generator->GetChangeList(frame.get()); - skipped_frames += changes.size(); + int total_frames = 0; + + const base::TimeDelta kWarmUpTime = base::TimeDelta::FromSeconds(2); + const base::TimeDelta kTestTime = base::TimeDelta::FromSeconds(5); + + base::TimeTicks start_time = base::TimeTicks::Now(); + while ((base::TimeTicks::Now() - start_time) < (kWarmUpTime + kTestTime)) { + ReceiveFrame(); + ++total_frames; } - base::TimeDelta total_latency_big_frames; - int big_frame_count = 0; - base::TimeDelta total_latency_small_frames; - int small_frame_count = 0; + WaitFrameStats(total_frames); + + int warm_up_frames = 0; + + int big_update_count = 0; + base::TimeDelta total_latency_big_updates; + int small_update_count = 0; + base::TimeDelta total_latency_small_updates; + for (int i = 0; i < total_frames; ++i) { + const protocol::FrameStats& stats = frame_stats_[i]; + + // CyclicFrameGenerator::TakeLastEventTimestamps() always returns non-null + // timestamps. + CHECK(!stats.host_stats.latest_event_timestamp.is_null()); - while (big_frame_count + small_frame_count < 30) { - std::unique_ptr frame = ReceiveFrame(); - base::TimeTicks frame_received_time = base::TimeTicks::Now(); test::CyclicFrameGenerator::ChangeInfoList changes = - frame_generator->GetChangeList(frame.get()); + frame_generator->GetChangeList(stats.host_stats.latest_event_timestamp); + + // Allow 2 seconds for the connection to warm-up, e.g. to get bandwidth + // estimate, etc. These frames are ignored when calculating stats below. + if (stats.client_stats.time_rendered < (start_time + kWarmUpTime)) { + ++warm_up_frames; + continue; + } + for (auto& change_info : changes) { - base::TimeDelta latency = frame_received_time - change_info.timestamp; + base::TimeDelta latency = + stats.client_stats.time_rendered - change_info.timestamp; switch (change_info.type) { case test::CyclicFrameGenerator::ChangeType::NO_CHANGES: NOTREACHED(); break; case test::CyclicFrameGenerator::ChangeType::FULL: - total_latency_big_frames += latency; - ++big_frame_count; + total_latency_big_updates += latency; + ++big_update_count; break; case test::CyclicFrameGenerator::ChangeType::CURSOR: - total_latency_small_frames += latency; - ++small_frame_count; + total_latency_small_updates += latency; + ++small_update_count; break; } } } - CHECK(big_frame_count); - VLOG(0) << "Average latency for big frames: " - << (total_latency_big_frames / big_frame_count).InMillisecondsF(); + CHECK(big_update_count); + VLOG(0) << "Average latency for big updates: " + << (total_latency_big_updates / big_update_count).InMillisecondsF(); - if (small_frame_count) { + if (small_update_count) { VLOG(0) - << "Average latency for small frames: " - << (total_latency_small_frames / small_frame_count).InMillisecondsF(); + << "Average latency for small updates: " + << (total_latency_small_updates / small_update_count).InMillisecondsF(); } } @@ -504,34 +536,29 @@ TEST_P(ProtocolPerfTest, TotalLatencyWebrtc) { void ProtocolPerfTest::MeasureScrollPerformance(bool use_webrtc) { scoped_refptr frame_generator = new test::ScrollFrameGenerator(); - desktop_environment_factory_->set_frame_generator( base::Bind(&test::ScrollFrameGenerator::GenerateFrame, frame_generator)); + event_timestamp_source_ = frame_generator; StartHostAndClient(use_webrtc); ASSERT_NO_FATAL_FAILURE(WaitConnected()); - int warm_up_frames = 0; - - base::TimeTicks start_time = base::TimeTicks::Now(); const base::TimeDelta kWarmUpTime = base::TimeDelta::FromSeconds(2); - while ((base::TimeTicks::Now() - start_time) < kWarmUpTime) { - ReceiveFrame(); - ++warm_up_frames; - } - - client_socket_factory_->ResetStats(); - - // Run the test for 2 seconds. const base::TimeDelta kTestTime = base::TimeDelta::FromSeconds(2); int num_frames = 0; - base::TimeDelta latency_sum; - start_time = base::TimeTicks::Now(); - while ((base::TimeTicks::Now() - start_time) < kTestTime) { - std::unique_ptr frame = ReceiveFrame(); + int warm_up_frames = 0; + base::TimeTicks start_time = base::TimeTicks::Now(); + while ((base::TimeTicks::Now() - start_time) < (kTestTime + kWarmUpTime)) { + ReceiveFrame(); ++num_frames; - latency_sum += frame_generator->GetFrameLatency(*frame); + + // Allow 2 seconds for the connection to warm-up, e.g. to get bandwidth + // estimate, etc. These frames are ignored when calculating stats below. + if ((base::TimeTicks::Now() - start_time) < kWarmUpTime) { + ++warm_up_frames; + client_socket_factory_->ResetStats(); + } } base::TimeDelta total_time = (base::TimeTicks::Now() - start_time); @@ -545,6 +572,14 @@ void ProtocolPerfTest::MeasureScrollPerformance(bool use_webrtc) { return sum + stats.host_stats.frame_size; }); + base::TimeDelta latency_sum = std::accumulate( + frame_stats_.begin() + warm_up_frames, + frame_stats_.begin() + warm_up_frames + num_frames, base::TimeDelta(), + [](base::TimeDelta sum, const protocol::FrameStats& stats) { + return sum + (stats.client_stats.time_rendered - + stats.host_stats.latest_event_timestamp); + }); + VLOG(0) << "FPS: " << num_frames / total_time.InSecondsF(); VLOG(0) << "Average latency: " << latency_sum.InMillisecondsF() / num_frames << " ms"; diff --git a/remoting/test/scroll_frame_generator.cc b/remoting/test/scroll_frame_generator.cc index 1a6a1ec0b6834d..01e0af239da995 100644 --- a/remoting/test/scroll_frame_generator.cc +++ b/remoting/test/scroll_frame_generator.cc @@ -41,19 +41,12 @@ std::unique_ptr ScrollFrameGenerator::GenerateFrame( result->mutable_updated_region()->SetRect( webrtc::DesktopRect::MakeSize(result->size())); - ++last_frame_id_; - frame_timestamp_[last_frame_id_] = now; - DrawBarcode(last_frame_id_, true, result.get()); - return result; } -base::TimeDelta ScrollFrameGenerator::GetFrameLatency( - const webrtc::DesktopFrame& frame) { - int frame_id = ReadBarcode(frame); - if (!frame_timestamp_.count(frame_id)) - LOG(FATAL) << "Unknown frame_id."; - return base::TimeTicks::Now() - frame_timestamp_[frame_id]; +protocol::InputEventTimestamps ScrollFrameGenerator::TakeLastEventTimestamps() { + base::TimeTicks now = base::TimeTicks::Now(); + return protocol::InputEventTimestamps{now, now}; } } // namespace test diff --git a/remoting/test/scroll_frame_generator.h b/remoting/test/scroll_frame_generator.h index 762ad7bd0c19dc..e76059713c3b1a 100644 --- a/remoting/test/scroll_frame_generator.h +++ b/remoting/test/scroll_frame_generator.h @@ -10,33 +10,30 @@ #include "base/memory/ref_counted.h" #include "base/time/time.h" +#include "remoting/protocol/input_event_timestamps.h" #include "third_party/webrtc/modules/desktop_capture/desktop_capturer.h" namespace remoting { namespace test { -class ScrollFrameGenerator - : public base::RefCountedThreadSafe { +class ScrollFrameGenerator : public protocol::InputEventTimestampsSource { public: ScrollFrameGenerator(); std::unique_ptr GenerateFrame( webrtc::SharedMemoryFactory* shared_memory_factory); - base::TimeDelta GetFrameLatency(const webrtc::DesktopFrame& frame); + // InputEventTimestampsSource interface. + protocol::InputEventTimestamps TakeLastEventTimestamps() override; private: - ~ScrollFrameGenerator(); - friend class base::RefCountedThreadSafe; + ~ScrollFrameGenerator() override; std::unique_ptr base_frame_; base::TimeTicks start_time_; std::unordered_map frame_timestamp_; - // Id of the last frame encoded on the barcode. - int last_frame_id_ = -1; - DISALLOW_COPY_AND_ASSIGN(ScrollFrameGenerator); };