From afdd5dd372d69be7244a3d90d70de9d5ecd60eb9 Mon Sep 17 00:00:00 2001 From: "magjed@webrtc.org" Date: Thu, 12 Mar 2015 13:11:25 +0000 Subject: [PATCH] Revert "Revert "Remove frame copy from cricket::VideoFrame to I420VideoFrame"" This reverts r8683 and is a reland of r8682. Reason for revert: The thread checker in Chromium that crashed has been fixed now. BUG=1128 TBR=tommi,pbos,pthatcher Review URL: https://webrtc-codereview.appspot.com/40319004 Cr-Commit-Position: refs/heads/master@{#8696} git-svn-id: http://webrtc.googlecode.com/svn/trunk@8696 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, 97 insertions(+), 43 deletions(-) diff --git a/talk/media/base/videoframe.cc b/talk/media/base/videoframe.cc index 77417a276..e96f676d0 100644 --- a/talk/media/base/videoframe.cc +++ b/talk/media/base/videoframe.cc @@ -33,8 +33,67 @@ #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. @@ -80,6 +139,11 @@ 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 b41965df1..0ba01672e 100644 --- a/talk/media/base/videoframe.h +++ b/talk/media/base/videoframe.h @@ -30,6 +30,7 @@ #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 { @@ -107,6 +108,14 @@ 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 0247a4e5c..f64cd40a9 100644 --- a/talk/media/webrtc/fakewebrtcvideoengine.h +++ b/talk/media/webrtc/fakewebrtcvideoengine.h @@ -374,6 +374,11 @@ 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 cb949c5ad..0e1ff527e 100644 --- a/talk/media/webrtc/webrtcvideoengine.cc +++ b/talk/media/webrtc/webrtcvideoengine.cc @@ -3276,18 +3276,8 @@ bool WebRtcVideoMediaChannel::SendFrame( frame_out = processed_frame.get(); } - 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()); - + webrtc::I420VideoFrame webrtc_frame(frame_out->GetVideoFrameBuffer(), 0, 0, + frame_out->GetVideoRotation()); 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. @@ -3308,9 +3298,9 @@ bool WebRtcVideoMediaChannel::SendFrame( rtc::UnixTimestampNanosecsToNtpMillisecs(frame_timestamp); } #endif - - return send_channel->external_capture()->IncomingFrameI420( - frame_i420, timestamp_ntp_ms) == 0; + webrtc_frame.set_ntp_time_ms(timestamp_ntp_ms); + send_channel->external_capture()->SwapFrame(&webrtc_frame); + return true; } bool WebRtcVideoMediaChannel::CreateChannel(uint32 ssrc_key, diff --git a/talk/media/webrtc/webrtcvideoengine2.cc b/talk/media/webrtc/webrtcvideoengine2.cc index 5886a2e62..03218e687 100644 --- a/talk/media/webrtc/webrtcvideoengine2.cc +++ b/talk/media/webrtc/webrtcvideoengine2.cc @@ -1374,32 +1374,14 @@ 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(); - // Lock before copying, can be called concurrently when swapping input source. - rtc::CritScope frame_cs(&frame_lock_); - ConvertToI420VideoFrame(*frame, &video_frame_); - + webrtc::I420VideoFrame video_frame(frame->GetVideoFrameBuffer(), 0, 0, + frame->GetVideoRotation()); rtc::CritScope cs(&lock_); if (stream_ == NULL) { LOG(LS_WARNING) << "Capturer inputting frames before send codecs are " @@ -1419,19 +1401,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 5dc1b044f..72563f0f9 100644 --- a/talk/media/webrtc/webrtcvideoengine2.h +++ b/talk/media/webrtc/webrtcvideoengine2.h @@ -376,9 +376,6 @@ 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 052e6f29e..86db24430 100644 --- a/talk/media/webrtc/webrtcvideoframe.cc +++ b/talk/media/webrtc/webrtcvideoframe.cc @@ -164,6 +164,11 @@ 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 8aa28ae59..14809c015 100644 --- a/talk/media/webrtc/webrtcvideoframe.h +++ b/talk/media/webrtc/webrtcvideoframe.h @@ -104,6 +104,8 @@ 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_; }