Revert "Remove frame copy from cricket::VideoFrame to I420VideoFrame"

This reverts r8682.

Reason for revert: Fails on Chromium FYI content_browsertests

BUG=1128
TBR=tommi,pbos,pthatcher

Review URL: https://webrtc-codereview.appspot.com/47529004

Cr-Commit-Position: refs/heads/master@{#8683}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8683 4adac7df-926f-26a2-2b94-8c16560cd09d

talk/media/base/videoframe.cc

@@ -33,67 +33,8 @@
 #include "libyuv/planar_functions.h"
 #include "libyuv/scale.h"
 #include "talk/media/base/videocommon.h"
-#include "webrtc/base/checks.h"
 #include "webrtc/base/logging.h"
 
-namespace {
-
-// TODO(magjed): Remove this once all subclasses implements GetVideoFrameBuffer.
-// Wraps cricket::VideoFrame into webrtc::VideoFrameBuffer without deep copy.
-class CricketVideoFrameBuffer : public webrtc::VideoFrameBuffer {
- public:
-  explicit CricketVideoFrameBuffer(const cricket::VideoFrame* frame)
-      // Note that cricket::VideoFrame::Copy is shallow. See declaration of that
-      // function for more info.
-      : frame_(frame->Copy()) {}
-
-  int width() const override { return static_cast<int>(frame_->GetWidth()); }
-  int height() const override { return static_cast<int>(frame_->GetHeight()); }
-
-  rtc::scoped_refptr<webrtc::NativeHandle> native_handle() const override {
-    return static_cast<webrtc::NativeHandle*>(frame_->GetNativeHandle());
-  }
-
-  const uint8_t* data(webrtc::PlaneType type) const override {
-    switch (type) {
-      case webrtc::kYPlane:
-        return frame_->GetYPlane();
-      case webrtc::kUPlane:
-        return frame_->GetUPlane();
-      case webrtc::kVPlane:
-        return frame_->GetVPlane();
-      default:
-        RTC_NOTREACHED();
-        return nullptr;
-    }
-  }
-
-  uint8_t* data(webrtc::PlaneType type) override {
-    LOG(LS_WARNING) << "Unsafe non-const pixel access.";
-    return const_cast<uint8_t*>(
-        static_cast<const CricketVideoFrameBuffer*>(this)->data(type));
-  }
-
-  int stride(webrtc::PlaneType type) const override {
-    switch (type) {
-      case webrtc::kYPlane:
-        return frame_->GetYPitch();
-      case webrtc::kUPlane:
-        return frame_->GetUPitch();
-      case webrtc::kVPlane:
-        return frame_->GetVPitch();
-      default:
-        RTC_NOTREACHED();
-        return 0;
-    }
-  }
-
- private:
-  const rtc::scoped_ptr<const cricket::VideoFrame> frame_;
-};
-
-}  // namespace
-
 namespace cricket {
 
 // Round to 2 pixels because Chroma channels are half size.
@@ -139,11 +80,6 @@ rtc::StreamResult VideoFrame::Write(rtc::StreamInterface* stream,
   return result;
 }
 
-rtc::scoped_refptr<webrtc::VideoFrameBuffer> VideoFrame::GetVideoFrameBuffer()
-    const {
-  return new rtc::RefCountedObject<CricketVideoFrameBuffer>(this);
-}
-
 size_t VideoFrame::CopyToBuffer(uint8* buffer, size_t size) const {
   const size_t y_size = GetHeight() * GetYPitch();
   const size_t u_size = GetUPitch() * GetChromaHeight();

talk/media/base/videoframe.h

@@ -30,7 +30,6 @@
 
 #include "webrtc/base/basictypes.h"
 #include "webrtc/base/stream.h"
-#include "webrtc/common_video/interface/video_frame_buffer.h"
 #include "webrtc/common_video/rotation.h"
 
 namespace cricket {
@@ -108,14 +107,6 @@ class VideoFrame {
   // longer in use, so the underlying resource can be freed.
   virtual void* GetNativeHandle() const = 0;
 
-  // Returns the underlying video frame buffer. The default implementation
-  // returns a shallow wrapper, converting itself into a
-  // webrtc::VideoFrameBuffer. This function is ok to call multiple times, but
-  // the returned object will refer to the same memory.
-  // TODO(magjed): Make pure virtual when all subclasses implement this.
-  virtual rtc::scoped_refptr<webrtc::VideoFrameBuffer> GetVideoFrameBuffer()
-      const;
-
   // For retrieving the aspect ratio of each pixel. Usually this is 1x1, but
   // the aspect_ratio_idc parameter of H.264 can specify non-square pixels.
   virtual size_t GetPixelWidth() const = 0;

talk/media/webrtc/fakewebrtcvideoengine.h

@@ -374,11 +374,6 @@ class FakeWebRtcVideoEngine
       return 0;
     }
 
-    virtual void SwapFrame(webrtc::I420VideoFrame* frame) {
-      last_capture_time_ = frame->render_time_ms();
-      ++incoming_frame_num_;
-    }
-
    private:
     int channel_id_;
     bool denoising_;

talk/media/webrtc/webrtcvideoengine.cc

@@ -3276,8 +3276,18 @@ bool WebRtcVideoMediaChannel::SendFrame(
     frame_out = processed_frame.get();
   }
 
-  webrtc::I420VideoFrame webrtc_frame(frame_out->GetVideoFrameBuffer(), 0, 0,
+  webrtc::ViEVideoFrameI420 frame_i420;
-                                      frame_out->GetVideoRotation());
+  // TODO(ronghuawu): Update the webrtc::ViEVideoFrameI420
+  // to use const unsigned char*
+  frame_i420.y_plane = const_cast<unsigned char*>(frame_out->GetYPlane());
+  frame_i420.u_plane = const_cast<unsigned char*>(frame_out->GetUPlane());
+  frame_i420.v_plane = const_cast<unsigned char*>(frame_out->GetVPlane());
+  frame_i420.y_pitch = frame_out->GetYPitch();
+  frame_i420.u_pitch = frame_out->GetUPitch();
+  frame_i420.v_pitch = frame_out->GetVPitch();
+  frame_i420.width = static_cast<uint16>(frame_out->GetWidth());
+  frame_i420.height = static_cast<uint16>(frame_out->GetHeight());
+
   int64 timestamp_ntp_ms = 0;
   // TODO(justinlin): Reenable after Windows issues with clock drift are fixed.
   // Currently reverted to old behavior of discarding capture timestamp.
@@ -3298,9 +3308,9 @@ bool WebRtcVideoMediaChannel::SendFrame(
         rtc::UnixTimestampNanosecsToNtpMillisecs(frame_timestamp);
   }
 #endif
-  webrtc_frame.set_ntp_time_ms(timestamp_ntp_ms);
+
-  send_channel->external_capture()->SwapFrame(&webrtc_frame);
+  return send_channel->external_capture()->IncomingFrameI420(
-  return true;
+      frame_i420, timestamp_ntp_ms) == 0;
 }
 
 bool WebRtcVideoMediaChannel::CreateChannel(uint32 ssrc_key,

talk/media/webrtc/webrtcvideoengine2.cc

@@ -1374,14 +1374,32 @@ static void CreateBlackFrame(webrtc::I420VideoFrame* video_frame,
          video_frame->allocated_size(webrtc::kVPlane));
 }
 
+static void ConvertToI420VideoFrame(const VideoFrame& frame,
+                                    webrtc::I420VideoFrame* i420_frame) {
+  i420_frame->CreateFrame(
+      static_cast<int>(frame.GetYPitch() * frame.GetHeight()),
+      frame.GetYPlane(),
+      static_cast<int>(frame.GetUPitch() * ((frame.GetHeight() + 1) / 2)),
+      frame.GetUPlane(),
+      static_cast<int>(frame.GetVPitch() * ((frame.GetHeight() + 1) / 2)),
+      frame.GetVPlane(),
+      static_cast<int>(frame.GetWidth()),
+      static_cast<int>(frame.GetHeight()),
+      static_cast<int>(frame.GetYPitch()),
+      static_cast<int>(frame.GetUPitch()),
+      static_cast<int>(frame.GetVPitch()));
+}
+
 void WebRtcVideoChannel2::WebRtcVideoSendStream::InputFrame(
     VideoCapturer* capturer,
     const VideoFrame* frame) {
   TRACE_EVENT0("webrtc", "WebRtcVideoSendStream::InputFrame");
   LOG(LS_VERBOSE) << "InputFrame: " << frame->GetWidth() << "x"
                   << frame->GetHeight();
-  webrtc::I420VideoFrame video_frame(frame->GetVideoFrameBuffer(), 0, 0,
+  // Lock before copying, can be called concurrently when swapping input source.
-                                     frame->GetVideoRotation());
+  rtc::CritScope frame_cs(&frame_lock_);
+  ConvertToI420VideoFrame(*frame, &video_frame_);
+
   rtc::CritScope cs(&lock_);
   if (stream_ == NULL) {
     LOG(LS_WARNING) << "Capturer inputting frames before send codecs are "
@@ -1401,19 +1419,19 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::InputFrame(
   }
   if (muted_) {
     // Create a black frame to transmit instead.
-    CreateBlackFrame(&video_frame,
+    CreateBlackFrame(&video_frame_,
                      static_cast<int>(frame->GetWidth()),
                      static_cast<int>(frame->GetHeight()));
   }
   // Reconfigure codec if necessary.
   SetDimensions(
-      video_frame.width(), video_frame.height(), capturer->IsScreencast());
+      video_frame_.width(), video_frame_.height(), capturer->IsScreencast());
 
-  LOG(LS_VERBOSE) << "SwapFrame: " << video_frame.width() << "x"
+  LOG(LS_VERBOSE) << "SwapFrame: " << video_frame_.width() << "x"
-                  << video_frame.height() << " -> (codec) "
+                  << video_frame_.height() << " -> (codec) "
                   << parameters_.encoder_config.streams.back().width << "x"
                   << parameters_.encoder_config.streams.back().height;
-  stream_->Input()->SwapFrame(&video_frame);
+  stream_->Input()->SwapFrame(&video_frame_);
 }
 
 bool WebRtcVideoChannel2::WebRtcVideoSendStream::SetCapturer(

talk/media/webrtc/webrtcvideoengine2.h

@@ -376,6 +376,9 @@ class WebRtcVideoChannel2 : public rtc::MessageHandler,
     bool muted_ GUARDED_BY(lock_);
     VideoFormat format_ GUARDED_BY(lock_);
     int old_adapt_changes_ GUARDED_BY(lock_);
+
+    rtc::CriticalSection frame_lock_;
+    webrtc::I420VideoFrame video_frame_ GUARDED_BY(frame_lock_);
   };
 
   // Wrapper for the receiver part, contains configs etc. that are needed to

talk/media/webrtc/webrtcvideoframe.cc

@@ -164,11 +164,6 @@ void* WebRtcVideoFrame::GetNativeHandle() const {
   return video_frame_buffer_ ? video_frame_buffer_->native_handle() : nullptr;
 }
 
-rtc::scoped_refptr<webrtc::VideoFrameBuffer>
-WebRtcVideoFrame::GetVideoFrameBuffer() const {
-  return video_frame_buffer_;
-}
-
 VideoFrame* WebRtcVideoFrame::Copy() const {
   WebRtcVideoFrame* new_frame = new WebRtcVideoFrame(
       video_frame_buffer_, elapsed_time_ns_, time_stamp_ns_);

talk/media/webrtc/webrtcvideoframe.h

@@ -104,8 +104,6 @@ class WebRtcVideoFrame : public VideoFrame {
   virtual int32 GetUPitch() const;
   virtual int32 GetVPitch() const;
   virtual void* GetNativeHandle() const;
-  virtual rtc::scoped_refptr<webrtc::VideoFrameBuffer> GetVideoFrameBuffer()
-      const;
 
   virtual size_t GetPixelWidth() const { return pixel_width_; }
   virtual size_t GetPixelHeight() const { return pixel_height_; }