Avoid using MakeExclusive, which is going to be deleted in webrtc.

remoting/protocol/webrtc_video_capturer_adapter.cc

Index: remoting/protocol/webrtc_video_capturer_adapter.cc
diff --git a/remoting/protocol/webrtc_video_capturer_adapter.cc b/remoting/protocol/webrtc_video_capturer_adapter.cc
index c0aeea1a193fe1cca5953777784155ef47c3e01c..05d878155e8bcf76a0d34b7e160f2f43a0559a80 100644
--- a/remoting/protocol/webrtc_video_capturer_adapter.cc
+++ b/remoting/protocol/webrtc_video_capturer_adapter.cc
@@ -174,16 +174,12 @@ void WebrtcVideoCapturerAdapter::OnCaptureCompleted(
 
   size_t width = frame->size().width();
   size_t height = frame->size().height();

[Sergey Ulanov, 2016/03/22 17:27:09]

change these two to int then you wouldn't need the casts below.

[nisse-chromium (ooo August 14), 2016/03/24 13:11:26]

Done.

-  if (!yuv_frame_ || yuv_frame_->GetWidth() != width ||
-      yuv_frame_->GetHeight() != height) {
+  if (!yuv_frame_ || static_cast<size_t>(yuv_frame_->width()) != width ||
+      static_cast<size_t>(yuv_frame_->height()) != height) {
     if (!size_callback_.is_null())
       size_callback_.Run(frame->size());
 
-    scoped_ptr<cricket::WebRtcVideoFrame> webrtc_frame(
-        new cricket::WebRtcVideoFrame());
-    webrtc_frame->InitToEmptyBuffer(width, height, 0);
-    yuv_frame_ = std::move(webrtc_frame);
-
+    yuv_frame_ = new rtc::RefCountedObject<webrtc::I420Buffer>(width, height);
     // Set updated_region so the whole frame is converted to YUV below.
     frame->mutable_updated_region()->SetRect(
         webrtc::DesktopRect::MakeWH(width, height));
@@ -191,10 +187,11 @@ void WebrtcVideoCapturerAdapter::OnCaptureCompleted(
 
   // TODO(sergeyu): This will copy the buffer if it's being used. Optimize it by

[perkj_chrome, 2016/03/23 14:43:54]

About this todo: Is the whole yuv_frame updated below or just a part of it for
each captured frame? 
If the complete frame is always updated, there is a webrtc::I420BufferPool we
can use instead.

[Sergey Ulanov, 2016/03/23 23:46:43]

The for loop below performs YUV conversion only for parts that are changing.

   // keeping a queue of frames.
-  CHECK(yuv_frame_->MakeExclusive());
-
-  yuv_frame_->SetTimeStamp(base::TimeTicks::Now().ToInternalValue() *
-                           base::Time::kNanosecondsPerMicrosecond);
+  if (!yuv_frame_->HasOneRef()) {
+    // Frame is still used, typically by the encoder. We have to make

[Sergey Ulanov, 2016/03/22 17:27:09]

Please move this comment above the TODO or move the TODO after this comment.

[nisse-chromium (ooo August 14), 2016/03/24 13:11:26]

Done.

+    // a copy before modifying it.
+    yuv_frame_ = yuv_frame_->Copy();
+  }
 
   for (webrtc::DesktopRegion::Iterator i(frame->updated_region()); !i.IsAtEnd();
        i.Advance()) {
@@ -214,16 +211,23 @@ void WebrtcVideoCapturerAdapter::OnCaptureCompleted(
     libyuv::ARGBToI420(
         frame->data() + frame->stride() * top +
             left * webrtc::DesktopFrame::kBytesPerPixel,
-        frame->stride(),
-        yuv_frame_->GetYPlane() + yuv_frame_->GetYPitch() * top + left,
-        yuv_frame_->GetYPitch(),
-        yuv_frame_->GetUPlane() + yuv_frame_->GetUPitch() * top / 2 + left / 2,
-        yuv_frame_->GetUPitch(),
-        yuv_frame_->GetVPlane() + yuv_frame_->GetVPitch() * top / 2 + left / 2,
-        yuv_frame_->GetVPitch(), width, height);
+        frame->stride(), yuv_frame_->MutableData(webrtc::kYPlane) +
+                             yuv_frame_->stride(webrtc::kYPlane) * top + left,
+        yuv_frame_->stride(webrtc::kYPlane),
+        yuv_frame_->MutableData(webrtc::kUPlane) +
+            yuv_frame_->stride(webrtc::kUPlane) * top / 2 + left / 2,
+        yuv_frame_->stride(webrtc::kUPlane),
+        yuv_frame_->MutableData(webrtc::kVPlane) +
+            yuv_frame_->stride(webrtc::kVPlane) * top / 2 + left / 2,
+        yuv_frame_->stride(webrtc::kVPlane), width, height);

[Sergey Ulanov, 2016/03/22 17:27:09]

nit: this ARGBToI420() call would be cleaner if you add variables for the
strides above it, i.e.
  int y_stride = yuv_frame_->stride(webrtc::kYPlane);
  int u_stride = yuv_frame_->stride(webrtc::kUPlane);
  int v_stride = yuv_frame_->stride(webrtc::kVPlane);

FWIW the stride() method should really be called Stride() with capital S. It's
virtual, not inlineable and has a switch statement in it, i.e. calling it is
much more expensive that accessing a local variable.
https://google.github.io/styleguide/cppguide.html#Function_Names

[nisse-chromium (ooo August 14), 2016/03/24 13:11:26]

Done.

   }
 
-  SignalVideoFrame(this, yuv_frame_.get());
+  scoped_ptr<cricket::VideoFrame> video_frame(new cricket::WebRtcVideoFrame(
+      yuv_frame_, base::TimeTicks::Now().ToInternalValue() *

[Sergey Ulanov, 2016/03/22 17:27:09]

This is not a proper way to convert TimeTicks to microseconds. You assume that
internal value is in nanoseconds, but that may not be the case. I suggest
changing this to 
  (base::TimeTicks::Now() - base::TimeTicks()) /
      base::TimeDelta::FromMicroseconds(1)

This expression doesn't depend on how the ticks represented internally.

[nisse-chromium (ooo August 14), 2016/03/24 13:11:26]

Done.

+                      base::Time::kNanosecondsPerMicrosecond,
+      webrtc::kVideoRotation_0));
+
+  SignalVideoFrame(this, video_frame.get());
 }
 
 void WebrtcVideoCapturerAdapter::CaptureNextFrame() {