diff --git a/webrtc/modules/desktop_capture/cropped_desktop_frame.cc b/webrtc/modules/desktop_capture/cropped_desktop_frame.cc index 5aa1449447..ac3cd1c5e5 100644 --- a/webrtc/modules/desktop_capture/cropped_desktop_frame.cc +++ b/webrtc/modules/desktop_capture/cropped_desktop_frame.cc @@ -17,6 +17,17 @@ 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: @@ -48,7 +59,7 @@ std::unique_ptr CreateCroppedDesktopFrame( CroppedDesktopFrame::CroppedDesktopFrame(std::unique_ptr frame, const DesktopRect& rect) - : DesktopFrame(rect.size(), + : DesktopFrame(TranslateRect(rect, frame->top_left()), frame->stride(), frame->GetFrameDataAtPos(rect.top_left()), frame->shared_memory()) { diff --git a/webrtc/modules/desktop_capture/desktop_and_cursor_composer.cc b/webrtc/modules/desktop_capture/desktop_and_cursor_composer.cc index eef8eaf479..cc7c522cfc 100644 --- a/webrtc/modules/desktop_capture/desktop_and_cursor_composer.cc +++ b/webrtc/modules/desktop_capture/desktop_and_cursor_composer.cc @@ -79,7 +79,7 @@ DesktopFrameWithCursor::DesktopFrameWithCursor( std::unique_ptr frame, const MouseCursor& cursor, const DesktopVector& position) - : DesktopFrame(frame->size(), + : DesktopFrame(frame->rect(), frame->stride(), frame->data(), frame->shared_memory()) { diff --git a/webrtc/modules/desktop_capture/desktop_frame.cc b/webrtc/modules/desktop_capture/desktop_frame.cc index 12a49cb1ff..a58762fb1c 100644 --- a/webrtc/modules/desktop_capture/desktop_frame.cc +++ b/webrtc/modules/desktop_capture/desktop_frame.cc @@ -16,6 +16,7 @@ #include "webrtc/modules/desktop_capture/desktop_geometry.h" #include "webrtc/rtc_base/checks.h" +#include "webrtc/rtc_base/ptr_util.h" namespace webrtc { @@ -37,7 +38,11 @@ DesktopFrame::DesktopFrame(DesktopRect rect, rect_(rect), stride_(stride), capture_time_ms_(0), - capturer_id_(DesktopCapturerId::kUnknown) {} + capturer_id_(DesktopCapturerId::kUnknown) { + RTC_DCHECK(stride_ >= 0); + RTC_DCHECK(rect.width() >= 0); + RTC_DCHECK(rect.height() >= 0); +} DesktopFrame::~DesktopFrame() = default; @@ -68,10 +73,12 @@ uint8_t* DesktopFrame::GetFrameDataAtPos(const DesktopVector& pos) const { } BasicDesktopFrame::BasicDesktopFrame(DesktopSize size) - : DesktopFrame(size, kBytesPerPixel * size.width(), - new uint8_t[kBytesPerPixel * size.width() * size.height()], - NULL) { -} + : BasicDesktopFrame(DesktopRect::MakeSize(size)) {} + +BasicDesktopFrame::BasicDesktopFrame(DesktopRect rect) + : DesktopFrame(rect, kBytesPerPixel * rect.width(), + new uint8_t[kBytesPerPixel * rect.width() * rect.height()], + nullptr) {} BasicDesktopFrame::~BasicDesktopFrame() { delete[] data_; @@ -95,29 +102,45 @@ DesktopFrame* BasicDesktopFrame::CopyOf(const DesktopFrame& frame) { std::unique_ptr SharedMemoryDesktopFrame::Create( DesktopSize size, SharedMemoryFactory* shared_memory_factory) { - size_t buffer_size = size.height() * size.width() * kBytesPerPixel; + return Create(DesktopRect::MakeSize(size), shared_memory_factory); +} + +// static +std::unique_ptr SharedMemoryDesktopFrame::Create( + DesktopRect rect, + SharedMemoryFactory* shared_memory_factory) { + RTC_DCHECK(shared_memory_factory); + + size_t buffer_size = rect.height() * rect.width() * kBytesPerPixel; std::unique_ptr shared_memory = shared_memory_factory->CreateSharedMemory(buffer_size); if (!shared_memory) return nullptr; - return Create(size, std::move(shared_memory)); -} - -// static -std::unique_ptr SharedMemoryDesktopFrame::Create( - DesktopSize size, - std::unique_ptr shared_memory) { - RTC_DCHECK(shared_memory); - int stride = size.width() * kBytesPerPixel; - return std::unique_ptr(new SharedMemoryDesktopFrame( - size, stride, shared_memory.release())); + return rtc::MakeUnique( + rect, rect.width() * kBytesPerPixel, std::move(shared_memory)); } SharedMemoryDesktopFrame::SharedMemoryDesktopFrame(DesktopSize size, int stride, SharedMemory* shared_memory) - : DesktopFrame(size, + : SharedMemoryDesktopFrame(DesktopRect::MakeSize(size), + stride, + shared_memory) {} + +SharedMemoryDesktopFrame::SharedMemoryDesktopFrame( + DesktopRect rect, + int stride, + std::unique_ptr shared_memory) + : SharedMemoryDesktopFrame(rect, + stride, + shared_memory.release()) {} + +SharedMemoryDesktopFrame::SharedMemoryDesktopFrame( + DesktopRect rect, + int stride, + SharedMemory* shared_memory) + : DesktopFrame(rect, stride, reinterpret_cast(shared_memory->data()), shared_memory) {} diff --git a/webrtc/modules/desktop_capture/desktop_frame.h b/webrtc/modules/desktop_capture/desktop_frame.h index c3f0d733c6..556a35d5e6 100644 --- a/webrtc/modules/desktop_capture/desktop_frame.h +++ b/webrtc/modules/desktop_capture/desktop_frame.h @@ -86,6 +86,9 @@ class DesktopFrame { 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, @@ -118,7 +121,12 @@ class DesktopFrame { // 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|. @@ -131,23 +139,44 @@ class BasicDesktopFrame : public DesktopFrame { // A DesktopFrame that stores data in shared memory. class SharedMemoryDesktopFrame : public DesktopFrame { public: + // 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 Create( DesktopSize size, SharedMemoryFactory* shared_memory_factory); + // Preferred. static std::unique_ptr Create( - DesktopSize size, - std::unique_ptr shared_memory); + DesktopRect rect, + SharedMemoryFactory* shared_memory_factory); // Takes ownership of |shared_memory|. - // TODO(zijiehe): Hide constructors after fake_desktop_capturer.cc has been - // migrated, Create() is preferred. + // Deprecated, use the next constructor. SharedMemoryDesktopFrame(DesktopSize size, int stride, SharedMemory* shared_memory); + + // Preferred. + SharedMemoryDesktopFrame(DesktopRect rect, + int stride, + std::unique_ptr shared_memory); + ~SharedMemoryDesktopFrame() override; private: + // Avoid unexpected order of parameter evaluation. + // Executing both std::unique_ptr::operator->() and + // std::unique_ptr::release() in the member initializer list is not safe. + // Depends on the order of parameter evaluation, + // std::unique_ptr::operator->() may trigger assertion failure if it has + // been evaluated after std::unique_ptr::release(). By using this + // constructor, std::unique_ptr::operator->() won't be involved anymore. + SharedMemoryDesktopFrame(DesktopRect rect, + int stride, + SharedMemory* shared_memory); + RTC_DISALLOW_COPY_AND_ASSIGN(SharedMemoryDesktopFrame); }; diff --git a/webrtc/modules/desktop_capture/shared_desktop_frame.cc b/webrtc/modules/desktop_capture/shared_desktop_frame.cc index 651e43af60..42d13d1b82 100644 --- a/webrtc/modules/desktop_capture/shared_desktop_frame.cc +++ b/webrtc/modules/desktop_capture/shared_desktop_frame.cc @@ -47,7 +47,7 @@ bool SharedDesktopFrame::IsShared() { } SharedDesktopFrame::SharedDesktopFrame(rtc::scoped_refptr core) - : DesktopFrame((*core)->size(), + : DesktopFrame((*core)->rect(), (*core)->stride(), (*core)->data(), (*core)->shared_memory()),