MediaStreamRemoteVideoSource: Wrap cricket::VideoFrame instead of copying.

content/renderer/media/webrtc/media_stream_remote_video_source.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/renderer/media/webrtc/media_stream_remote_video_source.h"
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/location.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "base/threading/thread_checker.h"
 #include "base/trace_event/trace_event.h"
 #include "content/renderer/media/native_handle_impl.h"
 #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"
 
 namespace content {
 
 // Internal class used for receiving frames from the webrtc track on a
 // libjingle thread and forward it to the IO-thread.
 class MediaStreamRemoteVideoSource::RemoteVideoSourceDelegate
     : public base::RefCountedThreadSafe<RemoteVideoSourceDelegate>,
       public webrtc::VideoRendererInterface {
@@ skipping 13 matching lines @@
   void RenderFrame(const cricket::VideoFrame* frame) override;
 
   void DoRenderFrameOnIOThread(
       const scoped_refptr<media::VideoFrame>& video_frame);
 
  private:
   // Bound to the render thread.
   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_;
 };
 
 MediaStreamRemoteVideoSource::
 RemoteVideoSourceDelegate::RemoteVideoSourceDelegate(
     const scoped_refptr<base::MessageLoopProxy>& io_message_loop,
     const VideoCaptureDeliverFrameCB& new_frame_callback)
     : io_message_loop_(io_message_loop),
@@ skipping 16 matching lines @@
       frame->GetElapsedTime() / rtc::kNumNanosecsPerMicrosec);
 
   scoped_refptr<media::VideoFrame> video_frame;
   if (frame->GetNativeHandle() != NULL) {
     NativeHandleImpl* handle =
         static_cast<NativeHandleImpl*>(frame->GetNativeHandle());
     video_frame = static_cast<media::VideoFrame*>(handle->GetHandle());
     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(
       FROM_HERE,
       base::Bind(&RemoteVideoSourceDelegate::DoRenderFrameOnIOThread,
                  this, video_frame));
 }
 
 void MediaStreamRemoteVideoSource::
 RemoteVideoSourceDelegate::DoRenderFrameOnIOThread(
@@ skipping 68 matching lines @@
     case webrtc::MediaStreamTrackInterface::kEnded:
       SetReadyState(blink::WebMediaStreamSource::ReadyStateEnded);
       break;
     default:
       NOTREACHED();
       break;
   }
 }
 
 }  // namespace content