From b218ff553148b9a26c82e3b3a46d626c4438cedd Mon Sep 17 00:00:00 2001 From: "magjed@webrtc.org" Date: Wed, 11 Mar 2015 15:29:07 +0000 Subject: [PATCH] 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 | 64 ----------------------- talk/media/base/videoframe.h | 9 ---- talk/media/webrtc/fakewebrtcvideoengine.h | 5 -- talk/media/webrtc/webrtcvideoengine.cc | 20 +++++-- talk/media/webrtc/webrtcvideoengine2.cc | 32 +++++++++--- talk/media/webrtc/webrtcvideoengine2.h | 3 ++ talk/media/webrtc/webrtcvideoframe.cc | 5 -- talk/media/webrtc/webrtcvideoframe.h | 2 - 8 files changed, 43 insertions(+), 97 deletions(-) diff --git a/talk/media/base/videoframe.cc b/talk/media/base/videoframe.cc index e96f676d02..77417a2769 100644 --- a/talk/media/base/videoframe.cc +++ b/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(frame_->GetWidth()); } - int height() const override { return static_cast(frame_->GetHeight()); } - - rtc::scoped_refptr native_handle() const override { - return static_cast(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( - static_cast(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 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 VideoFrame::GetVideoFrameBuffer() - const { - return new rtc::RefCountedObject(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(); diff --git a/talk/media/base/videoframe.h b/talk/media/base/videoframe.h index 0ba01672e7..b41965df12 100644 --- a/talk/media/base/videoframe.h +++ b/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 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; diff --git a/talk/media/webrtc/fakewebrtcvideoengine.h b/talk/media/webrtc/fakewebrtcvideoengine.h index f64cd40a95..0247a4e5c6 100644 --- a/talk/media/webrtc/fakewebrtcvideoengine.h +++ b/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_; diff --git a/talk/media/webrtc/webrtcvideoengine.cc b/talk/media/webrtc/webrtcvideoengine.cc index 0e1ff527e7..cb949c5ad9 100644 --- a/talk/media/webrtc/webrtcvideoengine.cc +++ b/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, - frame_out->GetVideoRotation()); + webrtc::ViEVideoFrameI420 frame_i420; + // TODO(ronghuawu): Update the webrtc::ViEVideoFrameI420 + // to use const unsigned char* + frame_i420.y_plane = const_cast(frame_out->GetYPlane()); + frame_i420.u_plane = const_cast(frame_out->GetUPlane()); + frame_i420.v_plane = const_cast(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(frame_out->GetWidth()); + frame_i420.height = static_cast(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 true; + + return send_channel->external_capture()->IncomingFrameI420( + frame_i420, timestamp_ntp_ms) == 0; } bool WebRtcVideoMediaChannel::CreateChannel(uint32 ssrc_key, diff --git a/talk/media/webrtc/webrtcvideoengine2.cc b/talk/media/webrtc/webrtcvideoengine2.cc index 03218e6879..5886a2e62e 100644 --- a/talk/media/webrtc/webrtcvideoengine2.cc +++ b/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(frame.GetYPitch() * frame.GetHeight()), + frame.GetYPlane(), + static_cast(frame.GetUPitch() * ((frame.GetHeight() + 1) / 2)), + frame.GetUPlane(), + static_cast(frame.GetVPitch() * ((frame.GetHeight() + 1) / 2)), + frame.GetVPlane(), + static_cast(frame.GetWidth()), + static_cast(frame.GetHeight()), + static_cast(frame.GetYPitch()), + static_cast(frame.GetUPitch()), + static_cast(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, - frame->GetVideoRotation()); + // Lock before copying, can be called concurrently when swapping input source. + 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(frame->GetWidth()), static_cast(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" - << video_frame.height() << " -> (codec) " + LOG(LS_VERBOSE) << "SwapFrame: " << video_frame_.width() << "x" + << 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( diff --git a/talk/media/webrtc/webrtcvideoengine2.h b/talk/media/webrtc/webrtcvideoengine2.h index 72563f0f95..5dc1b044f9 100644 --- a/talk/media/webrtc/webrtcvideoengine2.h +++ b/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 diff --git a/talk/media/webrtc/webrtcvideoframe.cc b/talk/media/webrtc/webrtcvideoframe.cc index 86db244303..052e6f29e2 100644 --- a/talk/media/webrtc/webrtcvideoframe.cc +++ b/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 -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_); diff --git a/talk/media/webrtc/webrtcvideoframe.h b/talk/media/webrtc/webrtcvideoframe.h index 14809c015f..8aa28ae59a 100644 --- a/talk/media/webrtc/webrtcvideoframe.h +++ b/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 GetVideoFrameBuffer() - const; virtual size_t GetPixelWidth() const { return pixel_width_; } virtual size_t GetPixelHeight() const { return pixel_height_; }