Commit 52c31b74 authored by Thomas Anderson's avatar Thomas Anderson Committed by Commit Bot

Revert "FrameSinkVideoCapturer: Fix refresh logic for damaged sources."

This reverts commit 93f7aa87.

Reason for revert: Causes failure in WebContentsVideoCaptureDeviceBrowserTest on Linux Tests (dbg)(1)(32):
https://ci.chromium.org/buildbot/chromium.linux/Linux%20Tests%20%28dbg%29%281%29%2832%29/47651

Original change's description:
> FrameSinkVideoCapturer: Fix refresh logic for damaged sources.
> 
> Fixes a couple of issues where frame capture becomes frozen or choppy,
> with the following changes:
> 
> 1. Move the "source dirty" logic out of VideoCaptureOracle and into
> FrameSinkVideoCapturerImpl.
> 
> 2. More-agressive response to frame refresh requests (needed for
> improved mouse cursor update response).
> 
> 3. Remove the distinction between passive and active refreshes from
> VideoCaptureOracle. FrameSinkVideoCapturer now tracks source damage, and
> can execute any refresh passively when there is no damage.
> 
> 4. Add logic to ensure that FrameSinkVideoCapturerImpl provides
> consumers with a refresh frame whenever key events occur; such as params
> changes, capture target changes, etc.; that are known to require a full
> update.
> 
> 5. Re-enable the content_browsertests that were disabled due to
> flakiness.
> 
> Bug: 785072, 754872
> Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel
> Change-Id: I402f3f928de61e6754e04014327421e4f6917d5f
> Reviewed-on: https://chromium-review.googlesource.com/884996
> Reviewed-by: Yuri Wiitala <miu@chromium.org>
> Reviewed-by: Xiangjun Zhang <xjz@chromium.org>
> Reviewed-by: kylechar <kylechar@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#532161}

TBR=miu@chromium.org,kylechar@chromium.org,xjz@chromium.org

Change-Id: I32fd2fb0bf1e1dc32c026198809d12ff2ecc69f1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 785072, 754872
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/890380Reviewed-by: default avatarThomas Anderson <thomasanderson@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532247}
parent fb85136d
...@@ -409,15 +409,12 @@ void CompositorFrameSinkSupport::DetachCaptureClient( ...@@ -409,15 +409,12 @@ void CompositorFrameSinkSupport::DetachCaptureClient(
capture_clients_.erase(it); capture_clients_.erase(it);
} }
gfx::Size CompositorFrameSinkSupport::GetActiveFrameSize() { gfx::Size CompositorFrameSinkSupport::GetSurfaceSize() {
if (current_surface_id_.is_valid()) { if (current_surface_id_.is_valid()) {
Surface* current_surface = Surface* current_surface =
surface_manager_->GetSurfaceForId(current_surface_id_); surface_manager_->GetSurfaceForId(current_surface_id_);
if (current_surface->HasActiveFrame()) { if (current_surface)
DCHECK(current_surface->GetActiveFrame().size_in_pixels() ==
current_surface->size_in_pixels());
return current_surface->size_in_pixels(); return current_surface->size_in_pixels();
}
} }
return gfx::Size(); return gfx::Size();
} }
......
...@@ -101,7 +101,7 @@ class VIZ_SERVICE_EXPORT CompositorFrameSinkSupport ...@@ -101,7 +101,7 @@ class VIZ_SERVICE_EXPORT CompositorFrameSinkSupport
// CapturableFrameSink implementation. // CapturableFrameSink implementation.
void AttachCaptureClient(CapturableFrameSink::Client* client) override; void AttachCaptureClient(CapturableFrameSink::Client* client) override;
void DetachCaptureClient(CapturableFrameSink::Client* client) override; void DetachCaptureClient(CapturableFrameSink::Client* client) override;
gfx::Size GetActiveFrameSize() override; gfx::Size GetSurfaceSize() override;
void RequestCopyOfSurface( void RequestCopyOfSurface(
std::unique_ptr<CopyOutputRequest> request) override; std::unique_ptr<CopyOutputRequest> request) override;
......
...@@ -49,9 +49,8 @@ class CapturableFrameSink { ...@@ -49,9 +49,8 @@ class CapturableFrameSink {
virtual void AttachCaptureClient(Client* client) = 0; virtual void AttachCaptureClient(Client* client) = 0;
virtual void DetachCaptureClient(Client* client) = 0; virtual void DetachCaptureClient(Client* client) = 0;
// Returns the currently-active frame size, or an empty size if there is no // Returns the current surface size.
// active frame. virtual gfx::Size GetSurfaceSize() = 0;
virtual gfx::Size GetActiveFrameSize() = 0;
// Issues a request for a copy of the next composited frame. // Issues a request for a copy of the next composited frame.
virtual void RequestCopyOfSurface( virtual void RequestCopyOfSurface(
......
...@@ -125,6 +125,10 @@ class VIZ_SERVICE_EXPORT FrameSinkVideoCapturerImpl final ...@@ -125,6 +125,10 @@ class VIZ_SERVICE_EXPORT FrameSinkVideoCapturerImpl final
// exceeding 60% of the design limit is considered "red line" operation. // exceeding 60% of the design limit is considered "red line" operation.
static constexpr float kTargetPipelineUtilization = 0.6f; static constexpr float kTargetPipelineUtilization = 0.6f;
// The amount of time to wait before retrying a refresh frame request.
static constexpr base::TimeDelta kRefreshFrameRetryInterval =
base::TimeDelta::FromMicroseconds(base::Time::kMicrosecondsPerSecond / 4);
private: private:
friend class FrameSinkVideoCapturerTest; friend class FrameSinkVideoCapturerTest;
...@@ -136,24 +140,12 @@ class VIZ_SERVICE_EXPORT FrameSinkVideoCapturerImpl final ...@@ -136,24 +140,12 @@ class VIZ_SERVICE_EXPORT FrameSinkVideoCapturerImpl final
// up-to-date content will be sent to the consumer in the near future. This // up-to-date content will be sent to the consumer in the near future. This
// refresh operation will be canceled if a compositing event triggers a frame // refresh operation will be canceled if a compositing event triggers a frame
// capture in the meantime. // capture in the meantime.
void ScheduleRefreshFrame(); void ScheduleRefreshFrame(media::VideoCaptureOracle::Event event);
// Returns the delay that should be used when setting the refresh timer. This
// is based on the current oracle prediction for frame duration.
base::TimeDelta GetDelayBeforeNextRefreshAttempt() const;
// Called whenever a major damage event, such as a capture parameter change, a // Executes the refresh capture, if conditions permit. Otherwise, schedules a
// resolved target change, etc., occurs. This marks the entire source as dirty
// and ensures the consumer will receive a refresh frame with up-to-date
// content.
void RefreshEntireSourceSoon();
// Executes a refresh capture, if conditions permit. Otherwise, schedules a
// later retry. Note that the retry "polling" should be a short-term state, // later retry. Note that the retry "polling" should be a short-term state,
// since it only occurs until the oracle allows the next frame capture to take // since it only occurs when the capture target hasn't yet been resolved.
// place. If a refresh was already pending, it is canceled in favor of this void RefreshOrReschedule(media::VideoCaptureOracle::Event event);
// new refresh.
void RefreshSoon();
// CapturableFrameSink::Client implementation: // CapturableFrameSink::Client implementation:
void OnBeginFrame(const BeginFrameArgs& args) final; void OnBeginFrame(const BeginFrameArgs& args) final;
...@@ -235,20 +227,15 @@ class VIZ_SERVICE_EXPORT FrameSinkVideoCapturerImpl final ...@@ -235,20 +227,15 @@ class VIZ_SERVICE_EXPORT FrameSinkVideoCapturerImpl final
using TimeRingBuffer = std::array<base::TimeTicks, kDesignLimitMaxFrames>; using TimeRingBuffer = std::array<base::TimeTicks, kDesignLimitMaxFrames>;
base::flat_map<BeginFrameSourceId, TimeRingBuffer> frame_display_times_; base::flat_map<BeginFrameSourceId, TimeRingBuffer> frame_display_times_;
// The portion of the source content that has changed, but has not yet been
// captured.
gfx::Rect dirty_rect_;
// These are sequence counters used to ensure that the frames are being // These are sequence counters used to ensure that the frames are being
// delivered in the same order they are captured. // delivered in the same order they are captured.
int64_t next_capture_frame_number_ = 0; int64_t next_capture_frame_number_ = 0;
int64_t next_delivery_frame_number_ = 0; int64_t next_delivery_frame_number_ = 0;
// This timer is started whenever the consumer needs another frame delivered. // When the oracle rejects a "refresh frame" request, or a target is not yet
// This might be because: 1) the consumer was just started and needs an // resolved, this timer is set to auto-retry the refresh at a later point.
// initial frame; 2) the capture target changed; 3) the oracle rejected // This ensures refresh frame requests eventually result in a frame being
// an event for timing reasons; 4) to satisfy explicit requests for a refresh // delivered to the consumer.
// frame, when RequestRefreshFrame() has been called.
// //
// Note: This is always set, but the instance is overridden for unit testing. // Note: This is always set, but the instance is overridden for unit testing.
base::Optional<base::OneShotTimer> refresh_frame_retry_timer_; base::Optional<base::OneShotTimer> refresh_frame_retry_timer_;
......
...@@ -228,7 +228,7 @@ void AuraWindowCaptureMachine::Capture(base::TimeTicks event_time) { ...@@ -228,7 +228,7 @@ void AuraWindowCaptureMachine::Capture(base::TimeTicks event_time) {
const base::TimeTicks start_time = base::TimeTicks::Now(); const base::TimeTicks start_time = base::TimeTicks::Now();
media::VideoCaptureOracle::Event event; media::VideoCaptureOracle::Event event;
if (event_time.is_null()) { if (event_time.is_null()) {
event = media::VideoCaptureOracle::kRefreshRequest; event = media::VideoCaptureOracle::kActiveRefreshRequest;
event_time = start_time; event_time = start_time;
} else { } else {
event = media::VideoCaptureOracle::kCompositorUpdate; event = media::VideoCaptureOracle::kCompositorUpdate;
......
...@@ -216,26 +216,31 @@ class WebContentsVideoCaptureDeviceBrowserTest : public ContentBrowserTest { ...@@ -216,26 +216,31 @@ class WebContentsVideoCaptureDeviceBrowserTest : public ContentBrowserTest {
// Creates and starts the device for frame capture, and checks that the // Creates and starts the device for frame capture, and checks that the
// initial refresh frame is delivered. // initial refresh frame is delivered.
void AllocateAndStartAndWaitForFirstFrame() { void AllocateAndStartAndWaitForFirstFrame() {
frames_.clear();
last_frame_timestamp_ = base::TimeDelta::Min();
capture_stack()->SetFrameCallback(
base::BindRepeating(&WebContentsVideoCaptureDeviceBrowserTest::OnFrame,
base::Unretained(this)));
auto* const main_frame = shell()->web_contents()->GetMainFrame(); auto* const main_frame = shell()->web_contents()->GetMainFrame();
device_ = std::make_unique<WebContentsVideoCaptureDevice>( device_ = std::make_unique<WebContentsVideoCaptureDevice>(
main_frame->GetProcess()->GetID(), main_frame->GetRoutingID()); main_frame->GetProcess()->GetID(), main_frame->GetRoutingID());
base::RunLoop run_loop;
capture_stack()->SetFrameCallback(base::BindRepeating(
[](base::RunLoop* loop, base::TimeDelta* first_frame_timestamp,
scoped_refptr<media::VideoFrame> frame) {
*first_frame_timestamp = frame->timestamp();
loop->Quit();
},
&run_loop, &last_frame_timestamp_));
device_->AllocateAndStartWithReceiver( device_->AllocateAndStartWithReceiver(
SnapshotCaptureParams(), capture_stack()->CreateFrameReceiver()); SnapshotCaptureParams(), capture_stack()->CreateFrameReceiver());
RunAllPendingInMessageLoop(BrowserThread::UI); run_loop.Run();
EXPECT_TRUE(capture_stack()->started()); EXPECT_TRUE(capture_stack()->started());
EXPECT_FALSE(capture_stack()->error_occurred()); EXPECT_FALSE(capture_stack()->error_occurred());
capture_stack()->ExpectNoLogMessages(); capture_stack()->ExpectNoLogMessages();
frames_.clear();
capture_stack()->SetFrameCallback(
base::BindRepeating(&WebContentsVideoCaptureDeviceBrowserTest::OnFrame,
base::Unretained(this)));
min_capture_period_ = base::TimeDelta::FromMicroseconds( min_capture_period_ = base::TimeDelta::FromMicroseconds(
base::Time::kMicrosecondsPerSecond / base::Time::kMicrosecondsPerSecond /
device_->capture_params().requested_format.frame_rate); device_->capture_params().requested_format.frame_rate);
WaitForFrameWithColor(SK_ColorBLACK);
} }
// Stops and destroys the device. // Stops and destroys the device.
...@@ -307,7 +312,6 @@ class WebContentsVideoCaptureDeviceBrowserTest : public ContentBrowserTest { ...@@ -307,7 +312,6 @@ class WebContentsVideoCaptureDeviceBrowserTest : public ContentBrowserTest {
} }
if (IsApproximatelySameColor(color, average_content_rgb)) { if (IsApproximatelySameColor(color, average_content_rgb)) {
VLOG(1) << "Observed desired frame.";
return; return;
} else { } else {
VLOG(3) << "PNG dump of undesired frame: " VLOG(3) << "PNG dump of undesired frame: "
...@@ -468,8 +472,10 @@ class WebContentsVideoCaptureDeviceBrowserTest : public ContentBrowserTest { ...@@ -468,8 +472,10 @@ class WebContentsVideoCaptureDeviceBrowserTest : public ContentBrowserTest {
// Tests that the device refuses to start if the WebContents target was // Tests that the device refuses to start if the WebContents target was
// destroyed before the device could start. // destroyed before the device could start.
IN_PROC_BROWSER_TEST_F(WebContentsVideoCaptureDeviceBrowserTest, // TODO(crbug/754872): Temporarily disabling due to flakiness.
ErrorsOutIfWebContentsHasGoneBeforeDeviceStart) { IN_PROC_BROWSER_TEST_F(
WebContentsVideoCaptureDeviceBrowserTest,
DISABLED_ErrorsOutIfWebContentsHasGoneBeforeDeviceStart) {
auto* const main_frame = shell()->web_contents()->GetMainFrame(); auto* const main_frame = shell()->web_contents()->GetMainFrame();
const auto render_process_id = main_frame->GetProcess()->GetID(); const auto render_process_id = main_frame->GetProcess()->GetID();
const auto render_frame_id = main_frame->GetRoutingID(); const auto render_frame_id = main_frame->GetRoutingID();
...@@ -501,8 +507,9 @@ IN_PROC_BROWSER_TEST_F(WebContentsVideoCaptureDeviceBrowserTest, ...@@ -501,8 +507,9 @@ IN_PROC_BROWSER_TEST_F(WebContentsVideoCaptureDeviceBrowserTest,
// Tests that the device starts, captures a frame, and then gracefully // Tests that the device starts, captures a frame, and then gracefully
// errors-out because the WebContents is destroyed before the device is stopped. // errors-out because the WebContents is destroyed before the device is stopped.
// TODO(crbug/754872): Temporarily disabling due to flakiness.
IN_PROC_BROWSER_TEST_F(WebContentsVideoCaptureDeviceBrowserTest, IN_PROC_BROWSER_TEST_F(WebContentsVideoCaptureDeviceBrowserTest,
ErrorsOutWhenWebContentsIsDestroyed) { DISABLED_ErrorsOutWhenWebContentsIsDestroyed) {
AllocateAndStartAndWaitForFirstFrame(); AllocateAndStartAndWaitForFirstFrame();
// Initially, the device captures any content changes normally. // Initially, the device captures any content changes normally.
...@@ -522,8 +529,9 @@ IN_PROC_BROWSER_TEST_F(WebContentsVideoCaptureDeviceBrowserTest, ...@@ -522,8 +529,9 @@ IN_PROC_BROWSER_TEST_F(WebContentsVideoCaptureDeviceBrowserTest,
// Tests that the device stops delivering frames while suspended. When resumed, // Tests that the device stops delivering frames while suspended. When resumed,
// any content changes that occurred during the suspend should cause a new frame // any content changes that occurred during the suspend should cause a new frame
// to be delivered, to ensure the client is up-to-date. // to be delivered, to ensure the client is up-to-date.
// TODO(crbug/754872): Temporarily disabling due to flakiness.
IN_PROC_BROWSER_TEST_F(WebContentsVideoCaptureDeviceBrowserTest, IN_PROC_BROWSER_TEST_F(WebContentsVideoCaptureDeviceBrowserTest,
SuspendsAndResumes) { DISABLED_SuspendsAndResumes) {
AllocateAndStartAndWaitForFirstFrame(); AllocateAndStartAndWaitForFirstFrame();
// Initially, the device captures any content changes normally. // Initially, the device captures any content changes normally.
...@@ -555,8 +563,9 @@ IN_PROC_BROWSER_TEST_F(WebContentsVideoCaptureDeviceBrowserTest, ...@@ -555,8 +563,9 @@ IN_PROC_BROWSER_TEST_F(WebContentsVideoCaptureDeviceBrowserTest,
// Tests that the device delivers refresh frames when asked, while the source // Tests that the device delivers refresh frames when asked, while the source
// content is not changing. // content is not changing.
// TODO(crbug/754872): Temporarily disabling due to flakiness.
IN_PROC_BROWSER_TEST_F(WebContentsVideoCaptureDeviceBrowserTest, IN_PROC_BROWSER_TEST_F(WebContentsVideoCaptureDeviceBrowserTest,
DeliversRefreshFramesUponRequest) { DISABLED_DeliversRefreshFramesUponRequest) {
AllocateAndStartAndWaitForFirstFrame(); AllocateAndStartAndWaitForFirstFrame();
// Set the page content to a known color. // Set the page content to a known color.
...@@ -586,16 +595,8 @@ class WebContentsVideoCaptureDeviceBrowserTestP ...@@ -586,16 +595,8 @@ class WebContentsVideoCaptureDeviceBrowserTestP
} }
}; };
#if defined(OS_CHROMEOS) // TODO(crbug/754872): Determine why these tests time out on CrOS only.
INSTANTIATE_TEST_CASE_P( #if !defined(OS_CHROMEOS)
,
WebContentsVideoCaptureDeviceBrowserTestP,
testing::Combine(
// On ChromeOS, software compositing is not an option.
testing::Values(false),
// Force video frame resolutions to have a fixed aspect ratio?
testing::Values(false, true)));
#else
INSTANTIATE_TEST_CASE_P( INSTANTIATE_TEST_CASE_P(
, ,
WebContentsVideoCaptureDeviceBrowserTestP, WebContentsVideoCaptureDeviceBrowserTestP,
...@@ -604,14 +605,15 @@ INSTANTIATE_TEST_CASE_P( ...@@ -604,14 +605,15 @@ INSTANTIATE_TEST_CASE_P(
testing::Values(false, true), testing::Values(false, true),
// Force video frame resolutions to have a fixed aspect ratio? // Force video frame resolutions to have a fixed aspect ratio?
testing::Values(false, true))); testing::Values(false, true)));
#endif // defined(OS_CHROMEOS) #endif // !defined(OS_CHROMEOS)
// Tests that the device successfully captures a series of content changes, // Tests that the device successfully captures a series of content changes,
// whether the browser is running with software compositing or GPU-accelerated // whether the browser is running with software compositing or GPU-accelerated
// compositing, and whether the WebContents is visible/hidden or // compositing, and whether the WebContents is visible/hidden or
// occluded/unoccluded. // occluded/unoccluded.
// TODO(crbug/754872): Temporarily disabling due to flakiness.
IN_PROC_BROWSER_TEST_P(WebContentsVideoCaptureDeviceBrowserTestP, IN_PROC_BROWSER_TEST_P(WebContentsVideoCaptureDeviceBrowserTestP,
CapturesContentChanges) { DISABLED_CapturesContentChanges) {
SCOPED_TRACE(testing::Message() SCOPED_TRACE(testing::Message()
<< "Test parameters: " << "Test parameters: "
<< (use_software_compositing() ? "Software Compositing" << (use_software_compositing() ? "Software Compositing"
......
...@@ -278,7 +278,8 @@ void ScreenCaptureMachineAndroid::MaybeCaptureForRefresh() { ...@@ -278,7 +278,8 @@ void ScreenCaptureMachineAndroid::MaybeCaptureForRefresh() {
if (lastFrame_.get() == nullptr) if (lastFrame_.get() == nullptr)
return; return;
const VideoCaptureOracle::Event event = VideoCaptureOracle::kRefreshRequest; const VideoCaptureOracle::Event event =
VideoCaptureOracle::kActiveRefreshRequest;
const base::TimeTicks start_time = base::TimeTicks::Now(); const base::TimeTicks start_time = base::TimeTicks::Now();
scoped_refptr<VideoFrame> frame; scoped_refptr<VideoFrame> frame;
ThreadSafeCaptureOracle::CaptureFrameCallback capture_frame_cb; ThreadSafeCaptureOracle::CaptureFrameCallback capture_frame_cb;
......
...@@ -70,6 +70,18 @@ void ScreenCaptureDeviceCore::RequestRefreshFrame() { ...@@ -70,6 +70,18 @@ void ScreenCaptureDeviceCore::RequestRefreshFrame() {
if (state_ != kCapturing) if (state_ != kCapturing)
return; return;
// Try to use the less resource-intensive "passive" refresh mechanism, unless
// this is the first refresh following a Resume().
if (force_active_refresh_once_) {
capture_machine_->MaybeCaptureForRefresh();
force_active_refresh_once_ = false;
return;
}
// Make a best-effort attempt at a passive refresh, but fall-back to an active
// refresh if that fails.
if (oracle_proxy_->AttemptPassiveRefresh())
return;
capture_machine_->MaybeCaptureForRefresh(); capture_machine_->MaybeCaptureForRefresh();
} }
...@@ -90,6 +102,7 @@ void ScreenCaptureDeviceCore::Resume() { ...@@ -90,6 +102,7 @@ void ScreenCaptureDeviceCore::Resume() {
if (state_ != kSuspended) if (state_ != kSuspended)
return; return;
force_active_refresh_once_ = true;
TransitionStateTo(kCapturing); TransitionStateTo(kCapturing);
capture_machine_->Resume(); capture_machine_->Resume();
...@@ -127,7 +140,9 @@ void ScreenCaptureDeviceCore::CaptureStarted(bool success) { ...@@ -127,7 +140,9 @@ void ScreenCaptureDeviceCore::CaptureStarted(bool success) {
ScreenCaptureDeviceCore::ScreenCaptureDeviceCore( ScreenCaptureDeviceCore::ScreenCaptureDeviceCore(
std::unique_ptr<VideoCaptureMachine> capture_machine) std::unique_ptr<VideoCaptureMachine> capture_machine)
: state_(kIdle), capture_machine_(std::move(capture_machine)) { : state_(kIdle),
capture_machine_(std::move(capture_machine)),
force_active_refresh_once_(false) {
DCHECK(capture_machine_.get()); DCHECK(capture_machine_.get());
} }
......
...@@ -50,8 +50,15 @@ class CAPTURE_EXPORT VideoCaptureMachine { ...@@ -50,8 +50,15 @@ class CAPTURE_EXPORT VideoCaptureMachine {
// overloading or under-utilization. // overloading or under-utilization.
virtual bool IsAutoThrottlingEnabled() const; virtual bool IsAutoThrottlingEnabled() const;
// Called by ScreenCaptureDeviceCore when it failed to satisfy a "refresh
// frame" request by attempting to resurrect the last video frame from the
// buffer pool (this is referred to as the "passive" refresh approach). The
// failure can happen for a number of reasons (e.g., the oracle decided to
// change resolution, or consumers of the last video frame are not yet
// finished with it).
//
// The implementation of this method should consult the oracle, using the // The implementation of this method should consult the oracle, using the
// kRefreshRequest event type, to decide whether to initiate a new frame // kActiveRefreshRequest event type, to decide whether to initiate a new frame
// capture, and then do so if the oracle agrees. // capture, and then do so if the oracle agrees.
virtual void MaybeCaptureForRefresh() = 0; virtual void MaybeCaptureForRefresh() = 0;
...@@ -114,6 +121,12 @@ class CAPTURE_EXPORT ScreenCaptureDeviceCore ...@@ -114,6 +121,12 @@ class CAPTURE_EXPORT ScreenCaptureDeviceCore
// component of the system with direct access to |client_|. // component of the system with direct access to |client_|.
scoped_refptr<ThreadSafeCaptureOracle> oracle_proxy_; scoped_refptr<ThreadSafeCaptureOracle> oracle_proxy_;
// After Resume(), some unknown amount of time has passed, and the content of
// the capture source may have changed. This flag is used to ensure that the
// passive refresh mechanism is not used for the first refresh frame following
// a Resume().
bool force_active_refresh_once_;
DISALLOW_COPY_AND_ASSIGN(ScreenCaptureDeviceCore); DISALLOW_COPY_AND_ASSIGN(ScreenCaptureDeviceCore);
}; };
......
...@@ -100,9 +100,20 @@ bool ThreadSafeCaptureOracle::ObserveEventAndDecideCapture( ...@@ -100,9 +100,20 @@ bool ThreadSafeCaptureOracle::ObserveEventAndDecideCapture(
coded_size.SetSize(base::bits::Align(visible_size.width(), 16), coded_size.SetSize(base::bits::Align(visible_size.width(), 16),
base::bits::Align(visible_size.height(), 16)); base::bits::Align(visible_size.height(), 16));
output_buffer = client_->ReserveOutputBuffer( if (event == VideoCaptureOracle::kPassiveRefreshRequest) {
coded_size, params_.requested_format.pixel_format, output_buffer = client_->ResurrectLastOutputBuffer(
params_.requested_format.pixel_storage, frame_number); coded_size, params_.requested_format.pixel_format,
params_.requested_format.pixel_storage, frame_number);
if (!output_buffer.is_valid()) {
TRACE_EVENT_INSTANT0("gpu.capture", "ResurrectionFailed",
TRACE_EVENT_SCOPE_THREAD);
return false;
}
} else {
output_buffer = client_->ReserveOutputBuffer(
coded_size, params_.requested_format.pixel_format,
params_.requested_format.pixel_storage, frame_number);
}
// Get the current buffer pool utilization and attenuate it: The utilization // Get the current buffer pool utilization and attenuate it: The utilization
// reported to the oracle is in terms of a maximum sustainable amount (not // reported to the oracle is in terms of a maximum sustainable amount (not
...@@ -177,6 +188,21 @@ bool ThreadSafeCaptureOracle::ObserveEventAndDecideCapture( ...@@ -177,6 +188,21 @@ bool ThreadSafeCaptureOracle::ObserveEventAndDecideCapture(
return true; return true;
} }
bool ThreadSafeCaptureOracle::AttemptPassiveRefresh() {
const base::TimeTicks refresh_time = base::TimeTicks::Now();
scoped_refptr<VideoFrame> frame;
CaptureFrameCallback capture_callback;
if (!ObserveEventAndDecideCapture(VideoCaptureOracle::kPassiveRefreshRequest,
gfx::Rect(), refresh_time, &frame,
&capture_callback)) {
return false;
}
capture_callback.Run(std::move(frame), refresh_time, true);
return true;
}
gfx::Size ThreadSafeCaptureOracle::GetCaptureSize() const { gfx::Size ThreadSafeCaptureOracle::GetCaptureSize() const {
base::AutoLock guard(lock_); base::AutoLock guard(lock_);
return oracle_.capture_size(); return oracle_.capture_size();
......
...@@ -60,6 +60,12 @@ class CAPTURE_EXPORT ThreadSafeCaptureOracle ...@@ -60,6 +60,12 @@ class CAPTURE_EXPORT ThreadSafeCaptureOracle
scoped_refptr<VideoFrame>* storage, scoped_refptr<VideoFrame>* storage,
CaptureFrameCallback* callback); CaptureFrameCallback* callback);
// Attempt to re-send the last frame to the VideoCaptureDevice::Client.
// Returns true if successful. This can fail if the last frame is no longer
// available in the buffer pool, or if the VideoCaptureOracle decides to
// reject the "passive" refresh.
bool AttemptPassiveRefresh();
base::TimeDelta min_capture_period() const { base::TimeDelta min_capture_period() const {
return oracle_.min_capture_period(); return oracle_.min_capture_period();
} }
......
...@@ -101,6 +101,7 @@ constexpr base::TimeDelta VideoCaptureOracle::kDefaultMinCapturePeriod; ...@@ -101,6 +101,7 @@ constexpr base::TimeDelta VideoCaptureOracle::kDefaultMinCapturePeriod;
VideoCaptureOracle::VideoCaptureOracle(bool enable_auto_throttling) VideoCaptureOracle::VideoCaptureOracle(bool enable_auto_throttling)
: auto_throttling_enabled_(enable_auto_throttling), : auto_throttling_enabled_(enable_auto_throttling),
next_frame_number_(0), next_frame_number_(0),
source_is_dirty_(true),
last_successfully_delivered_frame_number_(-1), last_successfully_delivered_frame_number_(-1),
num_frames_pending_(0), num_frames_pending_(0),
smoothing_sampler_(kDefaultMinCapturePeriod), smoothing_sampler_(kDefaultMinCapturePeriod),
...@@ -150,6 +151,11 @@ bool VideoCaptureOracle::ObserveEventAndDecideCapture( ...@@ -150,6 +151,11 @@ bool VideoCaptureOracle::ObserveEventAndDecideCapture(
} }
last_event_time_[event] = event_time; last_event_time_[event] = event_time;
// If the event indicates a change to the source content, set a flag that will
// prevent passive refresh requests until a capture is made.
if (event != kActiveRefreshRequest && event != kPassiveRefreshRequest)
source_is_dirty_ = true;
bool should_sample = false; bool should_sample = false;
duration_of_next_frame_ = base::TimeDelta(); duration_of_next_frame_ = base::TimeDelta();
switch (event) { switch (event) {
...@@ -172,7 +178,12 @@ bool VideoCaptureOracle::ObserveEventAndDecideCapture( ...@@ -172,7 +178,12 @@ bool VideoCaptureOracle::ObserveEventAndDecideCapture(
break; break;
} }
case kRefreshRequest: case kPassiveRefreshRequest:
if (source_is_dirty_)
break;
// Intentional flow-through to next case here!
case kActiveRefreshRequest:
case kMouseCursorUpdate:
// Only allow non-compositor samplings when content has not recently been // Only allow non-compositor samplings when content has not recently been
// animating, and only if there are no samplings currently in progress. // animating, and only if there are no samplings currently in progress.
if (num_frames_pending_ == 0) { if (num_frames_pending_ == 0) {
...@@ -226,6 +237,8 @@ bool VideoCaptureOracle::ObserveEventAndDecideCapture( ...@@ -226,6 +237,8 @@ bool VideoCaptureOracle::ObserveEventAndDecideCapture(
void VideoCaptureOracle::RecordCapture(double pool_utilization) { void VideoCaptureOracle::RecordCapture(double pool_utilization) {
DCHECK(std::isfinite(pool_utilization) && pool_utilization >= 0.0); DCHECK(std::isfinite(pool_utilization) && pool_utilization >= 0.0);
source_is_dirty_ = false;
smoothing_sampler_.RecordSample(); smoothing_sampler_.RecordSample();
const base::TimeTicks timestamp = GetFrameTimestamp(next_frame_number_); const base::TimeTicks timestamp = GetFrameTimestamp(next_frame_number_);
content_sampler_.RecordSample(timestamp); content_sampler_.RecordSample(timestamp);
...@@ -276,6 +289,9 @@ bool VideoCaptureOracle::CompleteCapture(int frame_number, ...@@ -276,6 +289,9 @@ bool VideoCaptureOracle::CompleteCapture(int frame_number,
if (!capture_was_successful) { if (!capture_was_successful) {
VLOG(2) << "Capture of frame #" << frame_number << " was not successful."; VLOG(2) << "Capture of frame #" << frame_number << " was not successful.";
// Since capture of this frame might have been required for capturing an
// update to the source content, set the dirty flag.
source_is_dirty_ = true;
return false; return false;
} }
...@@ -327,6 +343,7 @@ void VideoCaptureOracle::CancelAllCaptures() { ...@@ -327,6 +343,7 @@ void VideoCaptureOracle::CancelAllCaptures() {
// //
// ...which simplifies to: // ...which simplifies to:
num_frames_pending_ = 0; num_frames_pending_ = 0;
source_is_dirty_ = true;
} }
void VideoCaptureOracle::RecordConsumerFeedback(int frame_number, void VideoCaptureOracle::RecordConsumerFeedback(int frame_number,
...@@ -363,8 +380,12 @@ const char* VideoCaptureOracle::EventAsString(Event event) { ...@@ -363,8 +380,12 @@ const char* VideoCaptureOracle::EventAsString(Event event) {
switch (event) { switch (event) {
case kCompositorUpdate: case kCompositorUpdate:
return "compositor"; return "compositor";
case kRefreshRequest: case kActiveRefreshRequest:
return "refresh"; return "active_refresh";
case kPassiveRefreshRequest:
return "passive_refresh";
case kMouseCursorUpdate:
return "mouse";
case kNumEvents: case kNumEvents:
break; break;
} }
......
...@@ -24,7 +24,9 @@ class CAPTURE_EXPORT VideoCaptureOracle { ...@@ -24,7 +24,9 @@ class CAPTURE_EXPORT VideoCaptureOracle {
public: public:
enum Event { enum Event {
kCompositorUpdate, kCompositorUpdate,
kRefreshRequest, kActiveRefreshRequest,
kPassiveRefreshRequest,
kMouseCursorUpdate,
kNumEvents, kNumEvents,
}; };
...@@ -167,6 +169,11 @@ class CAPTURE_EXPORT VideoCaptureOracle { ...@@ -167,6 +169,11 @@ class CAPTURE_EXPORT VideoCaptureOracle {
// sanity-check that event times are monotonically non-decreasing. // sanity-check that event times are monotonically non-decreasing.
base::TimeTicks last_event_time_[kNumEvents]; base::TimeTicks last_event_time_[kNumEvents];
// Set to true if there have been updates to the source content that were not
// sampled. This will prevent passive refresh requests from being satisfied
// when an active refresh should be used instead.
bool source_is_dirty_;
// Updated by the last call to ObserveEventAndDecideCapture() with the // Updated by the last call to ObserveEventAndDecideCapture() with the
// estimated duration of the next frame to sample. This is zero if the method // estimated duration of the next frame to sample. This is zero if the method
// returned false. // returned false.
......
...@@ -257,7 +257,7 @@ TEST(VideoCaptureOracleTest, SamplesAtCorrectTimesAroundRefreshRequests) { ...@@ -257,7 +257,7 @@ TEST(VideoCaptureOracleTest, SamplesAtCorrectTimesAroundRefreshRequests) {
for (int i = 0; i < 10; ++i) { for (int i = 0; i < 10; ++i) {
t += refresh_interval; t += refresh_interval;
ASSERT_FALSE(oracle.ObserveEventAndDecideCapture( ASSERT_FALSE(oracle.ObserveEventAndDecideCapture(
VideoCaptureOracle::kRefreshRequest, gfx::Rect(), t)); VideoCaptureOracle::kPassiveRefreshRequest, gfx::Rect(), t));
} }
// Now, complete the oustanding compositor-based capture and continue // Now, complete the oustanding compositor-based capture and continue
...@@ -267,8 +267,8 @@ TEST(VideoCaptureOracleTest, SamplesAtCorrectTimesAroundRefreshRequests) { ...@@ -267,8 +267,8 @@ TEST(VideoCaptureOracleTest, SamplesAtCorrectTimesAroundRefreshRequests) {
did_complete_a_capture = false; did_complete_a_capture = false;
for (int i = 0; i < 10; ++i) { for (int i = 0; i < 10; ++i) {
t += refresh_interval; t += refresh_interval;
if (oracle.ObserveEventAndDecideCapture(VideoCaptureOracle::kRefreshRequest, if (oracle.ObserveEventAndDecideCapture(
gfx::Rect(), t)) { VideoCaptureOracle::kPassiveRefreshRequest, gfx::Rect(), t)) {
const int frame_number = oracle.next_frame_number(); const int frame_number = oracle.next_frame_number();
oracle.RecordCapture(0.0); oracle.RecordCapture(0.0);
ASSERT_TRUE(oracle.CompleteCapture(frame_number, true, &ignored)); ASSERT_TRUE(oracle.CompleteCapture(frame_number, true, &ignored));
...@@ -281,8 +281,8 @@ TEST(VideoCaptureOracleTest, SamplesAtCorrectTimesAroundRefreshRequests) { ...@@ -281,8 +281,8 @@ TEST(VideoCaptureOracleTest, SamplesAtCorrectTimesAroundRefreshRequests) {
for (int i = 0; i <= 10; ++i) { for (int i = 0; i <= 10; ++i) {
ASSERT_GT(10, i) << "BUG: Seems like it'll never happen!"; ASSERT_GT(10, i) << "BUG: Seems like it'll never happen!";
t += refresh_interval; t += refresh_interval;
if (oracle.ObserveEventAndDecideCapture(VideoCaptureOracle::kRefreshRequest, if (oracle.ObserveEventAndDecideCapture(
gfx::Rect(), t)) { VideoCaptureOracle::kPassiveRefreshRequest, gfx::Rect(), t)) {
break; break;
} }
} }
...@@ -294,14 +294,14 @@ TEST(VideoCaptureOracleTest, SamplesAtCorrectTimesAroundRefreshRequests) { ...@@ -294,14 +294,14 @@ TEST(VideoCaptureOracleTest, SamplesAtCorrectTimesAroundRefreshRequests) {
for (int i = 0; i < 10; ++i) { for (int i = 0; i < 10; ++i) {
t += refresh_interval; t += refresh_interval;
ASSERT_FALSE(oracle.ObserveEventAndDecideCapture( ASSERT_FALSE(oracle.ObserveEventAndDecideCapture(
VideoCaptureOracle::kRefreshRequest, gfx::Rect(), t)); VideoCaptureOracle::kPassiveRefreshRequest, gfx::Rect(), t));
} }
ASSERT_TRUE(oracle.CompleteCapture(frame_number, true, &ignored)); ASSERT_TRUE(oracle.CompleteCapture(frame_number, true, &ignored));
for (int i = 0; i <= 10; ++i) { for (int i = 0; i <= 10; ++i) {
ASSERT_GT(10, i) << "BUG: Seems like it'll never happen!"; ASSERT_GT(10, i) << "BUG: Seems like it'll never happen!";
t += refresh_interval; t += refresh_interval;
if (oracle.ObserveEventAndDecideCapture(VideoCaptureOracle::kRefreshRequest, if (oracle.ObserveEventAndDecideCapture(
gfx::Rect(), t)) { VideoCaptureOracle::kPassiveRefreshRequest, gfx::Rect(), t)) {
break; break;
} }
} }
...@@ -361,6 +361,43 @@ TEST(VideoCaptureOracleTest, DoesNotRapidlyChangeCaptureSize) { ...@@ -361,6 +361,43 @@ TEST(VideoCaptureOracleTest, DoesNotRapidlyChangeCaptureSize) {
} }
} }
// Tests that un-sampled compositor update event will fail the next passive
// refresh request, forcing an active refresh.
TEST(VideoCaptureOracleTest, EnforceActiveRefreshForUnsampledCompositorUpdate) {
const gfx::Rect damage_rect(Get720pSize());
const base::TimeDelta event_increment = Get30HzPeriod() * 2;
const base::TimeDelta short_event_increment = Get30HzPeriod() / 4;
VideoCaptureOracle oracle(false);
oracle.SetMinCapturePeriod(Get30HzPeriod());
oracle.SetCaptureSizeConstraints(Get720pSize(), Get720pSize(), false);
base::TimeTicks t = InitialTestTimeTicks();
int last_frame_number;
base::TimeTicks ignored;
// CompositorUpdate is sampled normally.
t += event_increment;
ASSERT_TRUE(oracle.ObserveEventAndDecideCapture(
VideoCaptureOracle::kCompositorUpdate, damage_rect, t));
last_frame_number = oracle.next_frame_number();
oracle.RecordCapture(0.0);
ASSERT_TRUE(oracle.CompleteCapture(last_frame_number, true, &ignored));
// Next CompositorUpdate comes too soon and won't be sampled.
t += short_event_increment;
ASSERT_FALSE(oracle.ObserveEventAndDecideCapture(
VideoCaptureOracle::kCompositorUpdate, damage_rect, t));
// Then the next valid PassiveRefreshRequest will fail to enforce an
// ActiveRefreshRequest to capture the updated content.
t += event_increment;
ASSERT_FALSE(oracle.ObserveEventAndDecideCapture(
VideoCaptureOracle::kPassiveRefreshRequest, damage_rect, t));
ASSERT_TRUE(oracle.ObserveEventAndDecideCapture(
VideoCaptureOracle::kActiveRefreshRequest, damage_rect, t));
}
namespace { namespace {
// Tests that VideoCaptureOracle can auto-throttle by stepping the capture size // Tests that VideoCaptureOracle can auto-throttle by stepping the capture size
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment