MediaStreamRemoteVideoSource: Wrap cricket::VideoFrame instead of copying.

content/renderer/media/webrtc/media_stream_remote_video_source.cc

Index: content/renderer/media/webrtc/media_stream_remote_video_source.cc
diff --git a/content/renderer/media/webrtc/media_stream_remote_video_source.cc b/content/renderer/media/webrtc/media_stream_remote_video_source.cc
index 786ed84fa39d43c3ec0fb6b6231270b73d8b73e7..e8ed5e682bcb5791af19b836e9e6867a50c058f8 100644
--- a/content/renderer/media/webrtc/media_stream_remote_video_source.cc
+++ b/content/renderer/media/webrtc/media_stream_remote_video_source.cc
@@ -14,7 +14,6 @@
 #include "content/renderer/media/webrtc/track_observer.h"
 #include "media/base/bind_to_current_loop.h"
 #include "media/base/video_frame.h"
-#include "media/base/video_frame_pool.h"
 #include "media/base/video_util.h"
 #include "third_party/libjingle/source/talk/media/base/videoframe.h"
 
@@ -48,9 +47,6 @@ class MediaStreamRemoteVideoSource::RemoteVideoSourceDelegate
   base::ThreadChecker thread_checker_;
 
   scoped_refptr<base::MessageLoopProxy> io_message_loop_;
-  // |frame_pool_| is only accessed on whatever
-  // thread webrtc::VideoRendererInterface::RenderFrame is called on.
-  media::VideoFramePool frame_pool_;
 
   // |frame_callback_| is accessed on the IO thread.
   VideoCaptureDeliverFrameCB frame_callback_;
@@ -87,21 +83,18 @@ RemoteVideoSourceDelegate::RenderFrame(
     video_frame->set_timestamp(timestamp);
   } else {
     gfx::Size size(frame->GetWidth(), frame->GetHeight());
-    video_frame = frame_pool_.CreateFrame(
-        media::VideoFrame::YV12, size, gfx::Rect(size), size, timestamp);
 
     // Non-square pixels are unsupported.
     DCHECK_EQ(frame->GetPixelWidth(), 1u);
     DCHECK_EQ(frame->GetPixelHeight(), 1u);
 
-    int y_rows = frame->GetHeight();
-    int uv_rows = frame->GetChromaHeight();
-    CopyYPlane(
-        frame->GetYPlane(), frame->GetYPitch(), y_rows, video_frame.get());
-    CopyUPlane(
-        frame->GetUPlane(), frame->GetUPitch(), uv_rows, video_frame.get());
-    CopyVPlane(
-        frame->GetVPlane(), frame->GetVPitch(), uv_rows, video_frame.get());
+    // Make a shallow copy. Both |frame| and |video_frame| will share a single
+    // reference counted frame buffer.
+    video_frame = media::VideoFrame::WrapExternalYuvConstData(
+        media::VideoFrame::YV12, size, gfx::Rect(size), size,
+        frame->GetYPitch(), frame->GetUPitch(), frame->GetVPitch(),
+        frame->GetYPlane(), frame->GetUPlane(), frame->GetVPlane(), timestamp,

[mcasas, 2015/03/06 17:29:59]

If I read correctly [1], there is a non-const version of
cricket::VideoFrame::GetYPlane() etc, and although I dig
the meaning of this CL, I'm wary of adding yet another
factory method to the already-fat-enough VideoFrame.
Could we just somehow reuse WrapExternalYuvData()?


[1]
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/libjin...

[magjed_chromium, 2015/03/07 14:15:10]

Yes we can reuse WrapExternalYuvData, by const casting. That is what I did in
patch set #3 together with DCHECKing that no one overwrites the data. I have
inlined it now without any checks, but I don’t really like that.

+        base::Bind(&base::DeletePointer<cricket::VideoFrame>, frame->Copy()));
   }
 
   io_message_loop_->PostTask(