Add DesktopFrame::MoveFrameInfoFrom() and DesktopFrame::CopyFrameInfoFrom()
The original change https://chromium-review.googlesource.com/c/575315 and
https://chromium-review.googlesource.com/c/590508 have not been well-considered.
So this change reverts part of two changes and adds a
DesktopFrame::set_top_left() function
A DesktopFrame usually contains a very large chunk of memory, which should be
reused as much as possible to reduce the memory allocations. The size of the
memory usually controls by the DesktopFrame::size(). So it's reasonable to const
DesktopFrame::size_: changing it is wrong if the underly buffer is not large
enough.
But DesktopFrame::top_left() is a different story, same as capturer_id,
capture_time_ms and other information in the DesktopFrame, it can be changed to
any value without needing to reconstruct a DesktopFrame instance. So instead of
adding it to the constructor, a DesktopFrame::set_top_left() is added to adjust
the top-left of the DesktopFrame in the entire display coordinate.
After adding DesktopFrame::set_top_left(), we have five variables in a
DesktopFrame which is not initialized in the constructor. For any kind of
wrapper DesktopFrame, say, SharedDesktopFrame and CroppedDesktopFrame, they
needs to copy these five variables after constructing themselves. This is not
convenient and easily to be broken if an implementation forgot to copy them.
So DesktopFrame::MoveFrameInfoFrom() and DesktopFrame::CopyFrameInfoFrom() are
added to the DesktopFrame to help derived classes to copy or move these
variables in one function call.
The difference between MoveFrameInfoFrom() and CopyFrameInfoFrom() is that the
former one uses DesktopRegion::Swap() to move the DesktopRegion from the source
DesktopFrame to this instance, while the later one uses copy-operator to copy
the DesktopRegion from the source DesktopFrame.
So CopyFrameInfoFrom() is usually used when sharing a source DesktopFrame with
several clients. I.e. the source DesktopFrame should be kept unchanged. For
example, BasicDesktopFrame::CopyOf() and SharedDesktopFrame::Share().
On the other side, MoveFrameInfoFrom() is usually used when wrapping a
DesktopFrame. E.g. CroppedDesktopFrame and DesktopFrameWithCursor.
Bug: webrtc:7950
Change-Id: I8b23418960fb681d2ea1f012d1b453f514da2272
Reviewed-on: https://chromium-review.googlesource.com/622453
Commit-Queue: Zijie He <zijiehe@chromium.org>
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#19504}
Cr-Mirrored-From: https://chromium.googlesource.com/external/webrtc
Cr-Mirrored-Commit: 5acd9d0393260b63e80b435fed9e5024ea6f4498diff --git a/modules/desktop_capture/cropped_desktop_frame.cc b/modules/desktop_capture/cropped_desktop_frame.cc
index ac3cd1c..67f03e7 100644
--- a/modules/desktop_capture/cropped_desktop_frame.cc
+++ b/modules/desktop_capture/cropped_desktop_frame.cc
@@ -17,17 +17,6 @@
namespace webrtc {
-namespace {
-
-DesktopRect TranslateRect(const DesktopRect& rect,
- const DesktopVector& top_left) {
- DesktopRect result = rect;
- result.Translate(top_left);
- return result;
-}
-
-} // namespace
-
// A DesktopFrame that is a sub-rect of another DesktopFrame.
class CroppedDesktopFrame : public DesktopFrame {
public:
@@ -35,7 +24,7 @@
const DesktopRect& rect);
private:
- std::unique_ptr<DesktopFrame> frame_;
+ const std::unique_ptr<DesktopFrame> frame_;
RTC_DISALLOW_COPY_AND_ASSIGN(CroppedDesktopFrame);
};
@@ -59,11 +48,15 @@
CroppedDesktopFrame::CroppedDesktopFrame(std::unique_ptr<DesktopFrame> frame,
const DesktopRect& rect)
- : DesktopFrame(TranslateRect(rect, frame->top_left()),
+ : DesktopFrame(frame->size(),
frame->stride(),
frame->GetFrameDataAtPos(rect.top_left()),
- frame->shared_memory()) {
- frame_ = std::move(frame);
+ frame->shared_memory()),
+ frame_(std::move(frame)) {
+ MoveFrameInfoFrom(frame_.get());
+ set_top_left(frame_->top_left().add(rect.top_left()));
+ mutable_updated_region()->IntersectWith(rect);
+ mutable_updated_region()->Translate(-rect.left(), -rect.top());
}
} // namespace webrtc
diff --git a/modules/desktop_capture/desktop_and_cursor_composer.cc b/modules/desktop_capture/desktop_and_cursor_composer.cc
index cc7c522..87f5726 100644
--- a/modules/desktop_capture/desktop_and_cursor_composer.cc
+++ b/modules/desktop_capture/desktop_and_cursor_composer.cc
@@ -67,7 +67,7 @@
~DesktopFrameWithCursor() override;
private:
- std::unique_ptr<DesktopFrame> original_frame_;
+ const std::unique_ptr<DesktopFrame> original_frame_;
DesktopVector restore_position_;
std::unique_ptr<DesktopFrame> restore_frame_;
@@ -79,15 +79,12 @@
std::unique_ptr<DesktopFrame> frame,
const MouseCursor& cursor,
const DesktopVector& position)
- : DesktopFrame(frame->rect(),
+ : DesktopFrame(frame->size(),
frame->stride(),
frame->data(),
- frame->shared_memory()) {
- set_dpi(frame->dpi());
- set_capture_time_ms(frame->capture_time_ms());
- set_capturer_id(frame->capturer_id());
- mutable_updated_region()->Swap(frame->mutable_updated_region());
- original_frame_ = std::move(frame);
+ frame->shared_memory()),
+ original_frame_(std::move(frame)) {
+ MoveFrameInfoFrom(original_frame_.get());
DesktopVector image_pos = position.subtract(cursor.hotspot());
DesktopRect target_rect = DesktopRect::MakeSize(cursor.image()->size());
diff --git a/modules/desktop_capture/desktop_capturer_differ_wrapper.cc b/modules/desktop_capture/desktop_capturer_differ_wrapper.cc
index f3a846c..8a93bc1 100644
--- a/modules/desktop_capture/desktop_capturer_differ_wrapper.cc
+++ b/modules/desktop_capture/desktop_capturer_differ_wrapper.cc
@@ -201,7 +201,7 @@
if (last_frame_) {
DesktopRegion hints;
- hints.Swap(frame->GetUnderlyingFrame()->mutable_updated_region());
+ hints.Swap(frame->mutable_updated_region());
for (DesktopRegion::Iterator it(hints); !it.IsAtEnd(); it.Advance()) {
CompareFrames(*last_frame_, *frame, it.rect(),
frame->mutable_updated_region());
@@ -212,10 +212,9 @@
}
last_frame_ = frame->Share();
- frame->set_capture_time_ms(frame->GetUnderlyingFrame()->capture_time_ms() +
+ frame->set_capture_time_ms(frame->capture_time_ms() +
(rtc::TimeNanos() - start_time_nanos) /
rtc::kNumNanosecsPerMillisec);
- frame->set_capturer_id(frame->GetUnderlyingFrame()->capturer_id());
callback_->OnCaptureResult(result, std::move(frame));
}
diff --git a/modules/desktop_capture/desktop_frame.cc b/modules/desktop_capture/desktop_frame.cc
index dbc3e3e..75317ce 100644
--- a/modules/desktop_capture/desktop_frame.cc
+++ b/modules/desktop_capture/desktop_frame.cc
@@ -24,23 +24,14 @@
int stride,
uint8_t* data,
SharedMemory* shared_memory)
- : DesktopFrame(DesktopRect::MakeSize(size),
- stride,
- data,
- shared_memory) {}
-
-DesktopFrame::DesktopFrame(DesktopRect rect,
- int stride,
- uint8_t* data,
- SharedMemory* shared_memory)
: data_(data),
shared_memory_(shared_memory),
- rect_(rect),
+ size_(size),
stride_(stride),
capture_time_ms_(0),
capturer_id_(DesktopCapturerId::kUnknown) {
- RTC_DCHECK(rect.width() >= 0);
- RTC_DCHECK(rect.height() >= 0);
+ RTC_DCHECK(size_.width() >= 0);
+ RTC_DCHECK(size_.height() >= 0);
}
DesktopFrame::~DesktopFrame() = default;
@@ -71,18 +62,32 @@
return data() + stride() * pos.y() + DesktopFrame::kBytesPerPixel * pos.x();
}
-BasicDesktopFrame::BasicDesktopFrame(DesktopSize size)
- : BasicDesktopFrame(DesktopRect::MakeSize(size)) {}
+void DesktopFrame::CopyFrameInfoFrom(const DesktopFrame& other) {
+ set_dpi(other.dpi());
+ set_capture_time_ms(other.capture_time_ms());
+ set_capturer_id(other.capturer_id());
+ *mutable_updated_region() = other.updated_region();
+ set_top_left(other.top_left());
+}
-BasicDesktopFrame::BasicDesktopFrame(DesktopRect rect)
- : DesktopFrame(rect, kBytesPerPixel * rect.width(),
- new uint8_t[kBytesPerPixel * rect.width() * rect.height()],
+void DesktopFrame::MoveFrameInfoFrom(DesktopFrame* other) {
+ set_dpi(other->dpi());
+ set_capture_time_ms(other->capture_time_ms());
+ set_capturer_id(other->capturer_id());
+ mutable_updated_region()->Swap(other->mutable_updated_region());
+ set_top_left(other->top_left());
+}
+
+BasicDesktopFrame::BasicDesktopFrame(DesktopSize size)
+ : DesktopFrame(size, kBytesPerPixel * size.width(),
+ new uint8_t[kBytesPerPixel * size.width() * size.height()],
nullptr) {}
BasicDesktopFrame::~BasicDesktopFrame() {
delete[] data_;
}
+// static
DesktopFrame* BasicDesktopFrame::CopyOf(const DesktopFrame& frame) {
DesktopFrame* result = new BasicDesktopFrame(frame.size());
for (int y = 0; y < frame.size().height(); ++y) {
@@ -90,10 +95,7 @@
frame.data() + y * frame.stride(),
frame.size().width() * kBytesPerPixel);
}
- result->set_dpi(frame.dpi());
- result->set_capture_time_ms(frame.capture_time_ms());
- result->set_capturer_id(frame.capturer_id());
- *result->mutable_updated_region() = frame.updated_region();
+ result->CopyFrameInfoFrom(frame);
return result;
}
@@ -101,49 +103,34 @@
std::unique_ptr<DesktopFrame> SharedMemoryDesktopFrame::Create(
DesktopSize size,
SharedMemoryFactory* shared_memory_factory) {
- return Create(DesktopRect::MakeSize(size), shared_memory_factory);
-}
-
-// static
-std::unique_ptr<DesktopFrame> SharedMemoryDesktopFrame::Create(
- DesktopRect rect,
- SharedMemoryFactory* shared_memory_factory) {
RTC_DCHECK(shared_memory_factory);
- size_t buffer_size = rect.height() * rect.width() * kBytesPerPixel;
+ size_t buffer_size = size.height() * size.width() * kBytesPerPixel;
std::unique_ptr<SharedMemory> shared_memory =
shared_memory_factory->CreateSharedMemory(buffer_size);
if (!shared_memory)
return nullptr;
return rtc::MakeUnique<SharedMemoryDesktopFrame>(
- rect, rect.width() * kBytesPerPixel, std::move(shared_memory));
+ size, size.width() * kBytesPerPixel, std::move(shared_memory));
}
SharedMemoryDesktopFrame::SharedMemoryDesktopFrame(DesktopSize size,
int stride,
SharedMemory* shared_memory)
- : SharedMemoryDesktopFrame(DesktopRect::MakeSize(size),
- stride,
- shared_memory) {}
-
-SharedMemoryDesktopFrame::SharedMemoryDesktopFrame(
- DesktopRect rect,
- int stride,
- std::unique_ptr<SharedMemory> shared_memory)
- : SharedMemoryDesktopFrame(rect,
- stride,
- shared_memory.release()) {}
-
-SharedMemoryDesktopFrame::SharedMemoryDesktopFrame(
- DesktopRect rect,
- int stride,
- SharedMemory* shared_memory)
- : DesktopFrame(rect,
+ : DesktopFrame(size,
stride,
reinterpret_cast<uint8_t*>(shared_memory->data()),
shared_memory) {}
+SharedMemoryDesktopFrame::SharedMemoryDesktopFrame(
+ DesktopSize size,
+ int stride,
+ std::unique_ptr<SharedMemory> shared_memory)
+ : SharedMemoryDesktopFrame(size,
+ stride,
+ shared_memory.release()) {}
+
SharedMemoryDesktopFrame::~SharedMemoryDesktopFrame() {
delete shared_memory_;
}
diff --git a/modules/desktop_capture/desktop_frame.h b/modules/desktop_capture/desktop_frame.h
index 556a35d..cb1b597 100644
--- a/modules/desktop_capture/desktop_frame.h
+++ b/modules/desktop_capture/desktop_frame.h
@@ -31,14 +31,12 @@
virtual ~DesktopFrame();
// Size of the frame.
- DesktopSize size() const { return rect_.size(); }
+ const DesktopSize& size() const { return size_; }
// The top-left of the frame in full desktop coordinates. E.g. the top left
// monitor should start from (0, 0).
- DesktopVector top_left() const { return rect_.top_left(); }
-
- // Rectangle covered by the frame.
- const DesktopRect& rect() const { return rect_; }
+ const DesktopVector& top_left() const { return top_left_; }
+ void set_top_left(const DesktopVector& top_left) { top_left_ = top_left; }
// Distance in the buffer between two neighboring rows in bytes.
int stride() const { return stride_; }
@@ -84,18 +82,26 @@
capturer_id_ = capturer_id;
}
- protected:
- // Deprecated, use the constructor below.
- // TODO(zijiehe): Remove deprecated constructors. DesktopFrame should describe
- // its own position in the system. See
- // https://bugs.chromium.org/p/webrtc/issues/detail?id=7950
- DesktopFrame(DesktopSize size,
- int stride,
- uint8_t* data,
- SharedMemory* shared_memory);
+ // Copies various information from |other|. Anything initialized in
+ // constructor are not copied.
+ // This function is usually used when sharing a source DesktopFrame with
+ // several clients: the original DesktopFrame should be kept unchanged. For
+ // example, BasicDesktopFrame::CopyOf() and SharedDesktopFrame::Share().
+ void CopyFrameInfoFrom(const DesktopFrame& other);
- // Preferred.
- DesktopFrame(DesktopRect rect,
+ // Copies various information from |other|. Anything initialized in
+ // constructor are not copied. Not like CopyFrameInfoFrom() function, this
+ // function uses swap or move constructor to avoid data copy. It won't break
+ // the |other|, but some of its information may be missing after this
+ // operation. E.g. other->updated_region_;
+ // This function is usually used when wrapping a DesktopFrame: the wrapper
+ // instance takes the ownership of |other|, so other components cannot access
+ // |other| anymore. For example, CroppedDesktopFrame and
+ // DesktopFrameWithCursor.
+ void MoveFrameInfoFrom(DesktopFrame* other);
+
+ protected:
+ DesktopFrame(DesktopSize size,
int stride,
uint8_t* data,
SharedMemory* shared_memory);
@@ -107,10 +113,11 @@
SharedMemory* const shared_memory_;
private:
- const DesktopRect rect_;
+ const DesktopSize size_;
const int stride_;
DesktopRegion updated_region_;
+ DesktopVector top_left_;
DesktopVector dpi_;
int64_t capture_time_ms_;
uint32_t capturer_id_;
@@ -121,15 +128,12 @@
// A DesktopFrame that stores data in the heap.
class BasicDesktopFrame : public DesktopFrame {
public:
- // Deprecated, use the next constructor.
explicit BasicDesktopFrame(DesktopSize size);
- // Preferred.
- explicit BasicDesktopFrame(DesktopRect rect);
-
~BasicDesktopFrame() override;
// Creates a BasicDesktopFrame that contains copy of |frame|.
+ // TODO(zijiehe): Return std::unique_ptr<DesktopFrame>
static DesktopFrame* CopyOf(const DesktopFrame& frame);
private:
@@ -142,16 +146,10 @@
// May return nullptr if |shared_memory_factory| failed to create a
// SharedMemory instance.
// |shared_memory_factory| should not be nullptr.
- // Deprecated, use the next Create() function.
static std::unique_ptr<DesktopFrame> Create(
DesktopSize size,
SharedMemoryFactory* shared_memory_factory);
- // Preferred.
- static std::unique_ptr<DesktopFrame> Create(
- DesktopRect rect,
- SharedMemoryFactory* shared_memory_factory);
-
// Takes ownership of |shared_memory|.
// Deprecated, use the next constructor.
SharedMemoryDesktopFrame(DesktopSize size,
@@ -159,7 +157,7 @@
SharedMemory* shared_memory);
// Preferred.
- SharedMemoryDesktopFrame(DesktopRect rect,
+ SharedMemoryDesktopFrame(DesktopSize size,
int stride,
std::unique_ptr<SharedMemory> shared_memory);
diff --git a/modules/desktop_capture/shared_desktop_frame.cc b/modules/desktop_capture/shared_desktop_frame.cc
index 42d13d1..f80dc88 100644
--- a/modules/desktop_capture/shared_desktop_frame.cc
+++ b/modules/desktop_capture/shared_desktop_frame.cc
@@ -11,8 +11,10 @@
#include "webrtc/modules/desktop_capture/shared_desktop_frame.h"
#include <memory>
+#include <utility>
#include "webrtc/rtc_base/constructormagic.h"
+#include "webrtc/rtc_base/ptr_util.h"
namespace webrtc {
@@ -33,13 +35,12 @@
return core_->get();
}
+bool SharedDesktopFrame::ShareFrameWith(const SharedDesktopFrame& other) const {
+ return core_->get() == other.core_->get();
+}
+
std::unique_ptr<SharedDesktopFrame> SharedDesktopFrame::Share() {
- std::unique_ptr<SharedDesktopFrame> result(new SharedDesktopFrame(core_));
- result->set_dpi(dpi());
- result->set_capture_time_ms(capture_time_ms());
- result->set_capturer_id(capturer_id());
- *result->mutable_updated_region() = updated_region();
- return result;
+ return std::unique_ptr<SharedDesktopFrame>(new SharedDesktopFrame(core_));
}
bool SharedDesktopFrame::IsShared() {
@@ -47,10 +48,12 @@
}
SharedDesktopFrame::SharedDesktopFrame(rtc::scoped_refptr<Core> core)
- : DesktopFrame((*core)->rect(),
+ : DesktopFrame((*core)->size(),
(*core)->stride(),
(*core)->data(),
(*core)->shared_memory()),
- core_(core) {}
+ core_(core) {
+ CopyFrameInfoFrom(*(core_->get()));
+}
} // namespace webrtc
diff --git a/modules/desktop_capture/shared_desktop_frame.h b/modules/desktop_capture/shared_desktop_frame.h
index 4b1d7ba..6b2fea0 100644
--- a/modules/desktop_capture/shared_desktop_frame.h
+++ b/modules/desktop_capture/shared_desktop_frame.h
@@ -31,9 +31,15 @@
// TODO(sergeyu): remove this method.
static SharedDesktopFrame* Wrap(DesktopFrame* desktop_frame);
+ // Deprecated. Clients do not need to know the underlying DesktopFrame
+ // instance.
+ // TODO(zijiehe): Remove this method.
// Returns the underlying instance of DesktopFrame.
DesktopFrame* GetUnderlyingFrame();
+ // Returns whether |this| and |other| share the underlying DesktopFrame.
+ bool ShareFrameWith(const SharedDesktopFrame& other) const;
+
// Creates a clone of this object.
std::unique_ptr<SharedDesktopFrame> Share();
@@ -46,7 +52,7 @@
SharedDesktopFrame(rtc::scoped_refptr<Core> core);
- rtc::scoped_refptr<Core> core_;
+ const rtc::scoped_refptr<Core> core_;
RTC_DISALLOW_COPY_AND_ASSIGN(SharedDesktopFrame);
};
