Use webrtc::DesktopCapturer for screen capturer implementation.

media/video/capture/screen/screen_capture_frame_queue.cc

Index: media/video/capture/screen/screen_capture_frame_queue.cc
diff --git a/media/video/capture/screen/screen_capture_frame_queue.cc b/media/video/capture/screen/screen_capture_frame_queue.cc
index 610a0628cfb753bf618d1779d003609c1ca2a79c..541ea159ce53b34bfe4c363b8e47664b9f761a91 100644
--- a/media/video/capture/screen/screen_capture_frame_queue.cc
+++ b/media/video/capture/screen/screen_capture_frame_queue.cc
@@ -7,33 +7,106 @@
 #include <algorithm>
 
 #include "base/basictypes.h"
-#include "media/video/capture/screen/screen_capture_frame.h"
+#include "base/logging.h"
+#include "base/threading/non_thread_safe.h"
+#include "third_party/webrtc/modules/desktop_capture/desktop_frame.h"
 
 namespace media {
 
+// EmittedFrame is a DesktopFrame wrapper that shares ownership of a
+// DesktopFrame with the creating ScreenCaptureFrameQueue. If EmittedFrame is
+// deleted before the queue is deleted then it returns the frame back to the
+// queue.
+class ScreenCaptureFrameQueue::EmittedFrame

[alexeypa (please no reviews), 2013/05/08 22:24:59]

How about allowing multiple EmittedFrame object to own a single
webrtc::DesktopFrame object? Something like:

class EmittedFrame {
  EmittedFrame(webrtc::DesktopFrame* frame):
    owned_frame_(frame),
    shared_ref_counter_(new int(1)) {}

  EmittedFrame(EmittedFrame& frame):
    owned_frame_(frame.owned_frame_),
    shared_ref_counter_(frame.shared_ref_counter_) {
    *shared_ref_counter_ += 1;
  }

  ~EmittedFrame() {
    *shared_ref_counter_ -= 1;
    if (!*shared_ref_counter_) {
      delete shared_ref_counter_;
      delete owned_frame_;
    }
  }

  webrtc::DesktopFrame* owned_frame_;
  int* shared_ref_counter_;
}

The reason it might be better is that EmittedFrame does not know anything about
the queue so it is easier to reason what it is doing.

The queue will keep an instance of EmittedFrame for each frame and give out
another EmittedFrame to the consumer.

Let's discuss this offline.

[Sergey Ulanov, 2013/05/09 18:49:02]

Problem is that if consumer tries to keep a frame we may start changing it while
consumer, that's why I don't think we should ever share the same buffer between
multiple emitted frames (we ensure it by verifying that each frame has been
released before we try reusing it).

+    : public webrtc::DesktopFrame,
+      public base::NonThreadSafe {
+ public:
+  EmittedFrame(webrtc::DesktopFrame* frame,
+               ScreenCaptureFrameQueue* queue,
+               int index)
+      : DesktopFrame(frame->size(), frame->stride(), frame->data(),
+                     frame->shared_memory()),
+        frame_(frame),
+        queue_(queue),
+        index_(index) {
+    set_dpi(frame->dpi());
+    set_capture_time_ms(frame->capture_time_ms());
+    updated_region_ = frame->updated_region();
+  }
+
+  virtual ~EmittedFrame() {
+    if (queue_)
+      queue_->ReturnEmittedFrame(frame_.release(), index_);

[alexeypa (please no reviews), 2013/05/08 22:24:59]

Can the emitted frames be returned out of order? They are likely returned in
order now, but it is an assumption that is easy to break.

[Sergey Ulanov, 2013/05/09 18:49:02]

Order in which frames are returned doesn't matter - that's why we pass index_
here. We only need to make sure that frames are returned before we try reusing
them.

+  }
+
+  void DetachFromQueue() {

[alexeypa (please no reviews), 2013/05/08 22:24:59]

nit: DetachFromQueue() is only called in a situation when all currently emitted
frames are being detached from the queue. Will a weak pointer to the queue be a
cleaner solution? With a weak pointer the queue will not need to keep a list of
emitted frames.

[Sergey Ulanov, 2013/05/09 18:49:02]

It's also called from queue destructor - for the case when capturer is deleted
before the frames it has emitted. We could use weak pointers here, but the goal
is to move this code to webrtc, and there are no weak pointers in webrtc.

+    queue_ = NULL;
+  }
+
+ private:
+  scoped_ptr<webrtc::DesktopFrame> frame_;
+  ScreenCaptureFrameQueue* queue_;
+  int index_;
+};
+
 ScreenCaptureFrameQueue::ScreenCaptureFrameQueue()
-    : current_(0),
-      previous_(NULL) {
-  SetAllFramesNeedUpdate();
+    : current_(0) {
+  memset(frames_, 0, sizeof(frames_));
+  memset(emitted_frames_, 0, sizeof(emitted_frames_));
 }
 
 ScreenCaptureFrameQueue::~ScreenCaptureFrameQueue() {
+  for (int i = 0; i < kQueueLength; ++i) {
+    if (emitted_frames_[i]) {
+      emitted_frames_[i]->DetachFromQueue();
+    } else {
+      delete frames_[i];
+    }
+  }
 }
 
-void ScreenCaptureFrameQueue::DoneWithCurrentFrame() {
-  previous_ = current_frame();
+void ScreenCaptureFrameQueue::MoveToNextFrame() {
   current_ = (current_ + 1) % kQueueLength;
+
+  // Verify that the frame has been released by the consumer before it tries to
+  // capture another one.
+  DCHECK(!emitted_frames_[current_]);
+}
+
+webrtc::DesktopFrame* ScreenCaptureFrameQueue::EmitCurrentFrame() {
+  DCHECK(!emitted_frames_[current_]);
+
+  // Wrap the frame with EmittedFrame.
+  EmittedFrame* result = new EmittedFrame(frames_[current_], this, current_);
+  emitted_frames_[current_] = result;
+  return result;
 }
 
 void ScreenCaptureFrameQueue::ReplaceCurrentFrame(
-    scoped_ptr<ScreenCaptureFrame> frame) {
-  frames_[current_] = frame.Pass();
-  needs_update_[current_] = false;
+    scoped_ptr<webrtc::DesktopFrame> frame) {
+  DCHECK(!emitted_frames_[current_]);
+  frames_[current_] = frame.release();
+}
+
+void ScreenCaptureFrameQueue::Reset() {
+  for (int i = 0; i < kQueueLength; ++i) {
+    if (emitted_frames_[i]) {
+      emitted_frames_[i]->DetachFromQueue();
+      emitted_frames_[i] = NULL;
+    } else {
+      delete frames_[i];
+    }
+    frames_[i] = NULL;
+  }
 }
 
-void ScreenCaptureFrameQueue::SetAllFramesNeedUpdate() {
-  std::fill(needs_update_, needs_update_ + arraysize(needs_update_), true);
-  previous_ = NULL;
+void ScreenCaptureFrameQueue::ReturnEmittedFrame(webrtc::DesktopFrame* frame,
+                                                 int index) {
+  if (frame == frames_[index]) {
+    emitted_frames_[index] = NULL;
+  } else {
+    delete frame;
+  }
 }
 
 }  // namespace media