DirectX based screen capturer logic

webrtc/modules/desktop_capture/desktop_frame.h

 /*
  *  Copyright (c) 2013 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #ifndef WEBRTC_MODULES_DESKTOP_CAPTURE_DESKTOP_FRAME_H_
 #define WEBRTC_MODULES_DESKTOP_CAPTURE_DESKTOP_FRAME_H_
 
 #include <memory>
 
 #include "webrtc/modules/desktop_capture/desktop_geometry.h"
 #include "webrtc/modules/desktop_capture/desktop_region.h"
 #include "webrtc/modules/desktop_capture/shared_memory.h"
 #include "webrtc/typedefs.h"
 
 namespace webrtc {
 
 // DesktopFrame represents a video frame captured from the screen.
 class DesktopFrame {
  public:
   // DesktopFrame objects always hold RGBA data.
   static const int kBytesPerPixel = 4;
 
+  DesktopFrame(DesktopSize size,

[Sergey Ulanov, 2016/04/14 23:10:42]

I don't think we want this constructor public. It doesn't take ownership of the
buffer, so it's easy to use it incorrectly

[Hzj_jie, 2016/04/15 19:42:17]

Sure, I will write a new class in screen_capturer_win_directx.cc to achieve the
similar functionality.

+               int stride,
+               uint8_t* data,
+               SharedMemory* shared_memory);
+
   virtual ~DesktopFrame();
 
   // Size of the frame.
   const DesktopSize& size() const { return size_; }
 
   // Distance in the buffer between two neighboring rows in bytes.
   int stride() const { return stride_; }
 
   // Data buffer used for the frame.
   uint8_t* data() const { return data_; }
@@ skipping 16 matching lines @@
   int64_t capture_time_ms() const { return capture_time_ms_; }
   void set_capture_time_ms(int64_t time_ms) { capture_time_ms_ = time_ms; }
 
   // Optional shape for the frame. Frames may be shaped e.g. if
   // capturing the contents of a shaped window.
   const DesktopRegion* shape() const { return shape_.get(); }
   void set_shape(DesktopRegion* shape) { shape_.reset(shape); }
 
   // Copies pixels from a buffer or another frame. |dest_rect| rect must lay
   // within bounds of this frame.
-  void CopyPixelsFrom(uint8_t* src_buffer, int src_stride,
+  void CopyPixelsFrom(uint8_t* src_buffer,
+                      int src_stride,
                       const DesktopRect& dest_rect);
   void CopyPixelsFrom(const DesktopFrame& src_frame,
                       const DesktopVector& src_pos,
                       const DesktopRect& dest_rect);
 
   // A helper to return the data pointer of a frame at the specified position.
   uint8_t* GetFrameDataAtPos(const DesktopVector& pos) const;
 
  protected:
-  DesktopFrame(DesktopSize size,
-               int stride,
-               uint8_t* data,
-               SharedMemory* shared_memory);
-
-  const DesktopSize size_;
-  const int stride_;
-
   // Ownership of the buffers is defined by the classes that inherit from this
   // class. They must guarantee that the buffer is not deleted before the frame
   // is deleted.
   uint8_t* const data_;
   SharedMemory* const shared_memory_;
 
+ private:

[Sergey Ulanov, 2016/04/14 23:10:42]

If you are making these members private then I think |data_| and
|shared_memory_| should also be private.

[Hzj_jie, 2016/04/15 19:42:17]

Unfortunately SharedMemoryDesktopFrame uses it in destructor.

+  const DesktopSize size_;
+  const int stride_;
+
   DesktopRegion updated_region_;
   DesktopVector dpi_;
   int64_t capture_time_ms_;
   std::unique_ptr<DesktopRegion> shape_;
 
- private:
   RTC_DISALLOW_COPY_AND_ASSIGN(DesktopFrame);
 };
 
 // A DesktopFrame that stores data in the heap.
 class BasicDesktopFrame : public DesktopFrame {
  public:
   explicit BasicDesktopFrame(DesktopSize size);
   ~BasicDesktopFrame() override;
 
   // Creates a BasicDesktopFrame that contains copy of |frame|.
   static DesktopFrame* CopyOf(const DesktopFrame& frame);
 
  private:
   RTC_DISALLOW_COPY_AND_ASSIGN(BasicDesktopFrame);
 };
 
 // A DesktopFrame that stores data in shared memory.
 class SharedMemoryDesktopFrame : public DesktopFrame {
  public:
   static std::unique_ptr<DesktopFrame> Create(
       DesktopSize size,
       SharedMemoryFactory* shared_memory_factory);
 
+  static std::unique_ptr<DesktopFrame> Create(
+      DesktopSize size,
+      int stride,

[Sergey Ulanov, 2016/04/14 23:10:42]

Why do you need this constructor?

[Hzj_jie, 2016/04/15 19:42:17]

In Windows, the pitch (stride in DesktopFrame) may not be consistent with
size.width() * 4. i.e. GPU renders a slightly larger area than the screen, I
have reproduced the issue on Thinkpad X130e.

And the reason I have removed unique_ptr based constructor is because the order
of evaluating parameters are not guaranteed to be fixed. i.e.
shared_memory.release() may be called before shared_memory->data(), and cause a
crash.

[Sergey Ulanov, 2016/04/19 23:51:07]

I understand that, but this constructor allocates new buffer which doesn't have
to have the same stride as the source buffer. CopyPixelsFrom() should be able
handle the case when stride is different.
It's still better to use unique_ptr<> whenever ownership is passed. Add a
uint8_t* data parameter and make that constructor private. Also while you are
here please add a Create() method that takes std::unique_ptr<SharedMemory>
instead of shared_memory_factory - that will allow to address your TODO below.

[Hzj_jie, 2016/04/26 23:00:07]

This constructor is used by remoting/protocol/fake_desktop_capturer.cc, I will
change fake_desktop_capturer.cc before privatize this constructor.

+      SharedMemoryFactory* shared_memory_factory);
+
   // Takes ownership of |shared_memory|.
-  // TODO(sergeyu): Remove this constructor and keep the second one.
+  // TODO(zijiehe): Hide constructors, Create function is preferred.
   SharedMemoryDesktopFrame(DesktopSize size,
                            int stride,
                            SharedMemory* shared_memory);
-  SharedMemoryDesktopFrame(DesktopSize size,
-                           int stride,
-                           std::unique_ptr<SharedMemory> shared_memory);
   ~SharedMemoryDesktopFrame() override;
 
  private:
   RTC_DISALLOW_COPY_AND_ASSIGN(SharedMemoryDesktopFrame);
 };
 
 }  // namespace webrtc
 
 #endif  // WEBRTC_MODULES_DESKTOP_CAPTURE_DESKTOP_FRAME_H_