From 26679d6d90aeb7114b6bcc86b26921768256e203 Mon Sep 17 00:00:00 2001 From: Magnus Jedvert Date: Tue, 7 Apr 2015 14:07:41 +0200 Subject: [PATCH] ViEFrameCallback::DeliverFrame: Make I420VideoFrame const ref. This CL makes ViEFrameCallback::DeliverFrame const and removes the potential frame copy in ViEFrameProviderBase by moving it to ViEEncoder::DeliverFrame instead, for clients that use the FrameCallback functionality to modify the frame content. BUG=1128 R=mflodman@webrtc.org Review URL: https://webrtc-codereview.appspot.com/43949004 Cr-Commit-Position: refs/heads/master@{#8934} --- .../mock/mock_vie_frame_provider_base.h | 2 +- webrtc/video_engine/vie_capturer.cc | 4 +- webrtc/video_engine/vie_capturer_unittest.cc | 8 ++-- webrtc/video_engine/vie_channel.cc | 2 +- webrtc/video_engine/vie_encoder.cc | 46 +++++++++++-------- webrtc/video_engine/vie_encoder.h | 2 +- .../video_engine/vie_frame_provider_base.cc | 23 ++-------- webrtc/video_engine/vie_frame_provider_base.h | 4 +- webrtc/video_engine/vie_renderer.cc | 4 +- webrtc/video_engine/vie_renderer.h | 2 +- 10 files changed, 46 insertions(+), 51 deletions(-) diff --git a/webrtc/video_engine/mock/mock_vie_frame_provider_base.h b/webrtc/video_engine/mock/mock_vie_frame_provider_base.h index a59fdbfddb..9228515851 100644 --- a/webrtc/video_engine/mock/mock_vie_frame_provider_base.h +++ b/webrtc/video_engine/mock/mock_vie_frame_provider_base.h @@ -19,7 +19,7 @@ class MockViEFrameCallback : public ViEFrameCallback { public: MOCK_METHOD3(DeliverFrame, void(int id, - I420VideoFrame* video_frame, + const I420VideoFrame& video_frame, const std::vector& csrcs)); MOCK_METHOD2(DelayChanged, void(int id, int frame_delay)); MOCK_METHOD3(GetPreferedFrameSettings, diff --git a/webrtc/video_engine/vie_capturer.cc b/webrtc/video_engine/vie_capturer.cc index 8a4880435d..d0456dbea3 100644 --- a/webrtc/video_engine/vie_capturer.cc +++ b/webrtc/video_engine/vie_capturer.cc @@ -478,7 +478,7 @@ bool ViECapturer::ViECaptureProcess() { void ViECapturer::DeliverI420Frame(I420VideoFrame* video_frame) { if (video_frame->native_handle() != NULL) { - ViEFrameProviderBase::DeliverFrame(video_frame, std::vector()); + ViEFrameProviderBase::DeliverFrame(*video_frame, std::vector()); return; } @@ -528,7 +528,7 @@ void ViECapturer::DeliverI420Frame(I420VideoFrame* video_frame) { } } // Deliver the captured frame to all observers (channels, renderer or file). - ViEFrameProviderBase::DeliverFrame(video_frame, std::vector()); + ViEFrameProviderBase::DeliverFrame(*video_frame, std::vector()); } bool ViECapturer::CaptureCapabilityFixed() { diff --git a/webrtc/video_engine/vie_capturer_unittest.cc b/webrtc/video_engine/vie_capturer_unittest.cc index 389f0bb175..e97a082a31 100644 --- a/webrtc/video_engine/vie_capturer_unittest.cc +++ b/webrtc/video_engine/vie_capturer_unittest.cc @@ -87,10 +87,10 @@ class ViECapturerTest : public ::testing::Test { data_callback_->OnIncomingCapturedFrame(0, *frame); } - void AddOutputFrame(const I420VideoFrame* frame) { - if (frame->native_handle() == NULL) - output_frame_ybuffers_.push_back(frame->buffer(kYPlane)); - output_frames_.push_back(new I420VideoFrame(*frame)); + void AddOutputFrame(const I420VideoFrame& frame) { + if (frame.native_handle() == NULL) + output_frame_ybuffers_.push_back(frame.buffer(kYPlane)); + output_frames_.push_back(new I420VideoFrame(frame)); output_frame_event_->Set(); } diff --git a/webrtc/video_engine/vie_channel.cc b/webrtc/video_engine/vie_channel.cc index 82555d79b6..48a6299ab5 100644 --- a/webrtc/video_engine/vie_channel.cc +++ b/webrtc/video_engine/vie_channel.cc @@ -1674,7 +1674,7 @@ int32_t ViEChannel::FrameToRender( no_of_csrcs = 1; } std::vector csrcs(arr_ofCSRC, arr_ofCSRC + no_of_csrcs); - DeliverFrame(&video_frame, csrcs); + DeliverFrame(video_frame, csrcs); return 0; } diff --git a/webrtc/video_engine/vie_encoder.cc b/webrtc/video_engine/vie_encoder.cc index a03d38e5ce..391766cb86 100644 --- a/webrtc/video_engine/vie_encoder.cc +++ b/webrtc/video_engine/vie_encoder.cc @@ -510,7 +510,7 @@ void ViEEncoder::TraceFrameDropEnd() { } void ViEEncoder::DeliverFrame(int id, - I420VideoFrame* video_frame, + const I420VideoFrame& video_frame, const std::vector& csrcs) { DCHECK(send_payload_router_ != NULL); DCHECK(csrcs.empty()); @@ -529,29 +529,29 @@ void ViEEncoder::DeliverFrame(int id, TraceFrameDropEnd(); } - TRACE_EVENT_ASYNC_STEP0("webrtc", "Video", video_frame->render_time_ms(), + TRACE_EVENT_ASYNC_STEP0("webrtc", "Video", video_frame.render_time_ms(), "Encode"); I420VideoFrame* decimated_frame = NULL; // TODO(wuchengli): support texture frames. - if (video_frame->native_handle() == NULL) { + if (video_frame.native_handle() == NULL) { { CriticalSectionScoped cs(callback_cs_.get()); if (effect_filter_) { size_t length = - CalcBufferSize(kI420, video_frame->width(), video_frame->height()); + CalcBufferSize(kI420, video_frame.width(), video_frame.height()); rtc::scoped_ptr video_buffer(new uint8_t[length]); - ExtractBuffer(*video_frame, length, video_buffer.get()); + ExtractBuffer(video_frame, length, video_buffer.get()); effect_filter_->Transform(length, video_buffer.get(), - video_frame->ntp_time_ms(), - video_frame->timestamp(), - video_frame->width(), - video_frame->height()); + video_frame.ntp_time_ms(), + video_frame.timestamp(), + video_frame.width(), + video_frame.height()); } } // Pass frame via preprocessor. - const int ret = vpm_.PreprocessFrame(*video_frame, &decimated_frame); + const int ret = vpm_.PreprocessFrame(video_frame, &decimated_frame); if (ret == 1) { // Drop this frame. return; @@ -560,18 +560,28 @@ void ViEEncoder::DeliverFrame(int id, return; } } - // If the frame was not resampled or scaled => use original. - if (decimated_frame == NULL) { - decimated_frame = video_frame; - } + // If we haven't resampled the frame and we have a FrameCallback, we need to + // make a deep copy of |video_frame|. + I420VideoFrame copied_frame; { CriticalSectionScoped cs(callback_cs_.get()); - if (pre_encode_callback_) + if (pre_encode_callback_) { + // If the frame was not resampled or scaled => use copy of original. + if (decimated_frame == NULL) { + copied_frame.CopyFrame(video_frame); + decimated_frame = &copied_frame; + } pre_encode_callback_->FrameCallback(decimated_frame); + } } - if (video_frame->native_handle() != NULL) { + // If the frame was not resampled, scaled, or touched by FrameCallback => use + // original. The frame is const from here. + const I420VideoFrame* output_frame = + (decimated_frame != NULL) ? decimated_frame : &video_frame; + + if (video_frame.native_handle() != NULL) { // TODO(wuchengli): add texture support. http://crbug.com/362437 return; } @@ -594,12 +604,12 @@ void ViEEncoder::DeliverFrame(int id, has_received_rpsi_ = false; } - vcm_.AddVideoFrame(*decimated_frame, vpm_.ContentMetrics(), + vcm_.AddVideoFrame(*output_frame, vpm_.ContentMetrics(), &codec_specific_info); return; } #endif - vcm_.AddVideoFrame(*decimated_frame); + vcm_.AddVideoFrame(*output_frame); } void ViEEncoder::DelayChanged(int id, int frame_delay) { diff --git a/webrtc/video_engine/vie_encoder.h b/webrtc/video_engine/vie_encoder.h index 8683b07803..50d1c934e9 100644 --- a/webrtc/video_engine/vie_encoder.h +++ b/webrtc/video_engine/vie_encoder.h @@ -105,7 +105,7 @@ class ViEEncoder // Implementing ViEFrameCallback. void DeliverFrame(int id, - I420VideoFrame* video_frame, + const I420VideoFrame& video_frame, const std::vector& csrcs) override; void DelayChanged(int id, int frame_delay) override; int GetPreferedFrameSettings(int* width, diff --git a/webrtc/video_engine/vie_frame_provider_base.cc b/webrtc/video_engine/vie_frame_provider_base.cc index c7493a4ff2..a9ee1a8063 100644 --- a/webrtc/video_engine/vie_frame_provider_base.cc +++ b/webrtc/video_engine/vie_frame_provider_base.cc @@ -45,7 +45,7 @@ int ViEFrameProviderBase::Id() const { return id_; } -void ViEFrameProviderBase::DeliverFrame(I420VideoFrame* video_frame, +void ViEFrameProviderBase::DeliverFrame(const I420VideoFrame& video_frame, const std::vector& csrcs) { DCHECK(frame_delivery_thread_checker_.CalledOnValidThread()); #ifdef DEBUG_ @@ -54,24 +54,9 @@ void ViEFrameProviderBase::DeliverFrame(I420VideoFrame* video_frame, CriticalSectionScoped cs(provider_cs_.get()); // Deliver the frame to all registered callbacks. - if (frame_callbacks_.size() == 1) { - // We don't have to copy the frame. - frame_callbacks_.front()->DeliverFrame(id_, video_frame, csrcs); - } else { - for (ViEFrameCallback* callback : frame_callbacks_) { - if (video_frame->native_handle() != NULL) { - callback->DeliverFrame(id_, video_frame, csrcs); - } else { - // Make a copy of the frame for all callbacks. - if (!extra_frame_.get()) { - extra_frame_.reset(new I420VideoFrame()); - } - // TODO(mflodman): We can get rid of this frame copy. - extra_frame_->CopyFrame(*video_frame); - callback->DeliverFrame(id_, extra_frame_.get(), csrcs); - } - } - } + for (ViEFrameCallback* callback : frame_callbacks_) + callback->DeliverFrame(id_, video_frame, csrcs); + #ifdef DEBUG_ const int process_time = static_cast((TickTime::Now() - start_process_time).Milliseconds()); diff --git a/webrtc/video_engine/vie_frame_provider_base.h b/webrtc/video_engine/vie_frame_provider_base.h index b3ace3bdfc..8ec2f4bdf9 100644 --- a/webrtc/video_engine/vie_frame_provider_base.h +++ b/webrtc/video_engine/vie_frame_provider_base.h @@ -29,7 +29,7 @@ class I420VideoFrame; class ViEFrameCallback { public: virtual void DeliverFrame(int id, - I420VideoFrame* video_frame, + const I420VideoFrame& video_frame, const std::vector& csrcs) = 0; // The capture delay has changed from the provider. |frame_delay| is given in @@ -77,7 +77,7 @@ class ViEFrameProviderBase { virtual int FrameCallbackChanged() = 0; protected: - void DeliverFrame(I420VideoFrame* video_frame, + void DeliverFrame(const I420VideoFrame& video_frame, const std::vector& csrcs); void SetFrameDelay(int frame_delay); int FrameDelay(); diff --git a/webrtc/video_engine/vie_renderer.cc b/webrtc/video_engine/vie_renderer.cc index bfeeba216a..0deaf2f4bc 100644 --- a/webrtc/video_engine/vie_renderer.cc +++ b/webrtc/video_engine/vie_renderer.cc @@ -122,9 +122,9 @@ int32_t ViERenderer::Init(const uint32_t z_order, } void ViERenderer::DeliverFrame(int id, - I420VideoFrame* video_frame, + const I420VideoFrame& video_frame, const std::vector& csrcs) { - render_callback_->RenderFrame(render_id_, *video_frame); + render_callback_->RenderFrame(render_id_, video_frame); } void ViERenderer::DelayChanged(int id, int frame_delay) {} diff --git a/webrtc/video_engine/vie_renderer.h b/webrtc/video_engine/vie_renderer.h index 5a7fdc19f1..55765a706d 100644 --- a/webrtc/video_engine/vie_renderer.h +++ b/webrtc/video_engine/vie_renderer.h @@ -97,7 +97,7 @@ class ViERenderer: public ViEFrameCallback { // Implement ViEFrameCallback virtual void DeliverFrame(int id, - I420VideoFrame* video_frame, + const I420VideoFrame& video_frame, const std::vector& csrcs); virtual void DelayChanged(int id, int frame_delay); virtual int GetPreferedFrameSettings(int* width,