Ensure there is a unique FrameQueue for each DxgiOutputDuplicator

DxgiOutputDuplicator objects hold a reference to the last frame that
they succesfully captured by maintaining a reference to the
SharedDesktopFrame that was passed as their target. This is done because
the DirectX capture APIs may fail to provide an update if there has been
no (or no substantial) change since the last capture call was made.
However, the higher levels of this capture stack
(DxgiDuplicatorController and ScreenCapturerWinDirectX), were unaware of
this, and assumed that the caller of CaptureFrame is the only one who
may have held a reference to the frame. Thus, when CaptureFrame is
called, the DirectX screen capturer assumes that the oldest frame in its
queue can be safely reused.

In the steady state, where capture is not being switched between
monitors, this is fine as there are no competing DxgiOutputDuplicators
being run and this assumption mostly holds true (or the frame is being
overwritten only when the DxgiOutputDuplicator is also done holding it).
However, when capture is being rapidly switched between multiple targets
(e.g. to show a preview of each of the available monitors), this can
result in a frame being held by one DxgiOutputDuplicator being passed to
another as a valid target and overwritten. In the common case of only a
single monitor this is essentially the same as steady state capture,
where there are no competing DxgiOutputDuplicator. In the other common
case of two monitors being captured, the fact that the
ScreenCaptureFrameQueue has two frames ends up masking this issue. Since
each monitor is captured in the same order, the same frame ends up
getting passed to each DxgiOutputDuplicator, so no data actually ends
up getting overwritten. In the case of 3 monitors, the 1st and 3rd
monitor end up sharing a frame, which when capture fails on one of them
surfaces as the other monitor being duplicately shown.

This change addresses the issue by ensuring that each screen that the
ScreenCapturerWinDirectX *actually attempts* to capture, gets it's own
FrameQueue, and thus essentially brings us back to the "steady state"
case for each monitor. Note that this does increase memory usage of
capturers that are switched between multiple targets by 2 frames/target
used (and actually attempted to be captured).

Alternatives considered:
DxgiOutputDuplicator makes a copy of the frame, rather than holding
a reference
  This was rejected because adding an additional copy for every
  capture upon getting a new frame, would expensive and could degrade
  performance.

Allow the DxgiOutputDuplicators to "fail" when there has been no update
  This would result in either a breaking change to the API for consumers
  or would require the ScreenCapturerWinDirectX to track these last
  captured frames; which would result in essentially the same approach,
  but with less abstraction for re-using the frames.

Bug: chromium:1296228
Change-Id: I5442ec40e9f234046010b562b258db63693ccc6b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/256043
Reviewed-by: Mark Foltz <mfoltz@chromium.org>
Commit-Queue: Alexander Cooper <alcooper@chromium.org>
Cr-Commit-Position: refs/heads/main@{#36295}
This commit is contained in:
Alex Cooper 2022-03-21 14:47:05 -07:00 committed by WebRTC LUCI CQ
parent d7f9550647
commit 56b836d958
4 changed files with 34 additions and 9 deletions

View File

@ -41,6 +41,10 @@ namespace webrtc {
// but a later Duplicate() returns false, this usually means the display mode is
// changing. Consumers should retry after a while. (Typically 50 milliseconds,
// but according to hardware performance, this time may vary.)
// The underyling DxgiOutputDuplicators may take an additional reference on the
// frame passed in to the Duplicate methods so that they can guarantee delivery
// of new frames when requested; since if there have been no updates to the
// surface, they may be unable to capture a frame.
class RTC_EXPORT DxgiDuplicatorController {
public:
using Context = DxgiFrameContext;
@ -89,7 +93,8 @@ class RTC_EXPORT DxgiDuplicatorController {
// function returns false, the information in `info` may not accurate.
bool RetrieveD3dInfo(D3dInfo* info);
// Captures current screen and writes into `frame`.
// Captures current screen and writes into `frame`. May retain a reference to
// `frame`'s underlying |SharedDesktopFrame|.
// TODO(zijiehe): Windows cannot guarantee the frames returned by each
// IDXGIOutputDuplication are synchronized. But we are using a totally
// different threading model than the way Windows suggested, it's hard to
@ -98,7 +103,8 @@ class RTC_EXPORT DxgiDuplicatorController {
// Captures one monitor and writes into target. `monitor_id` should >= 0. If
// `monitor_id` is greater than the total screen count of all the Duplicators,
// this function returns false.
// this function returns false. May retain a reference to `frame`'s underlying
// |SharedDesktopFrame|.
Result DuplicateMonitor(DxgiFrame* frame, int monitor_id);
// Returns dpi of current system. Returns an empty DesktopVector if system

View File

@ -61,6 +61,11 @@ class DxgiOutputDuplicator {
// function copies the content to the rectangle of (offset.x(), offset.y()) to
// (offset.x() + desktop_rect_.width(), offset.y() + desktop_rect_.height()).
// Returns false in case of a failure.
// May retain a reference to `target` so that a "captured" frame can be
// returned in the event that a new frame is not ready to be captured yet.
// (Or in other words, if the call to IDXGIOutputDuplication::AcquireNextFrame
// indicates that there is not yet a new frame, this is usually because no
// updates have occurred to the frame).
bool Duplicate(Context* context,
DesktopVector offset,
SharedDesktopFrame* target);

View File

@ -125,17 +125,23 @@ void ScreenCapturerWinDirectx::CaptureFrame() {
int64_t capture_start_time_nanos = rtc::TimeNanos();
frames_.MoveToNextFrame();
if (!frames_.current_frame()) {
frames_.ReplaceCurrentFrame(
// Note that the [] operator will create the ScreenCaptureFrameQueue if it
// doesn't exist, so this is safe.
ScreenCaptureFrameQueue<DxgiFrame>& frames =
frame_queue_map_[current_screen_id_];
frames.MoveToNextFrame();
if (!frames.current_frame()) {
frames.ReplaceCurrentFrame(
std::make_unique<DxgiFrame>(shared_memory_factory_.get()));
}
DxgiDuplicatorController::Result result;
if (current_screen_id_ == kFullDesktopScreenId) {
result = controller_->Duplicate(frames_.current_frame());
result = controller_->Duplicate(frames.current_frame());
} else {
result = controller_->DuplicateMonitor(frames_.current_frame(),
result = controller_->DuplicateMonitor(frames.current_frame(),
current_screen_id_);
}
@ -172,7 +178,7 @@ void ScreenCapturerWinDirectx::CaptureFrame() {
}
case DuplicateResult::SUCCEEDED: {
std::unique_ptr<DesktopFrame> frame =
frames_.current_frame()->frame()->Share();
frames.current_frame()->frame()->Share();
int capture_time_ms = (rtc::TimeNanos() - capture_start_time_nanos) /
rtc::kNumNanosecsPerMillisec;

View File

@ -14,6 +14,7 @@
#include <d3dcommon.h>
#include <memory>
#include <unordered_map>
#include <vector>
#include "api/scoped_refptr.h"
@ -86,7 +87,14 @@ class RTC_EXPORT ScreenCapturerWinDirectx : public DesktopCapturer {
private:
const rtc::scoped_refptr<DxgiDuplicatorController> controller_;
ScreenCaptureFrameQueue<DxgiFrame> frames_;
// The underlying DxgiDuplicators may retain a reference to the frames that
// we ask them to duplicate so that they can continue returning valid frames
// in the event that the target has not been updated. Thus, we need to ensure
// that we have a separate frame queue for each source id, so that these held
// frames don't get overwritten with the data from another Duplicator/monitor.
std::unordered_map<SourceId, ScreenCaptureFrameQueue<DxgiFrame>>
frame_queue_map_;
std::unique_ptr<SharedMemoryFactory> shared_memory_factory_;
Callback* callback_ = nullptr;
SourceId current_screen_id_ = kFullDesktopScreenId;