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}
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 a59fdbf..9228515 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 @@
public:
MOCK_METHOD3(DeliverFrame,
void(int id,
- I420VideoFrame* video_frame,
+ const I420VideoFrame& video_frame,
const std::vector<uint32_t>& 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 8a48804..d0456db 100644
--- a/webrtc/video_engine/vie_capturer.cc
+++ b/webrtc/video_engine/vie_capturer.cc
@@ -478,7 +478,7 @@
void ViECapturer::DeliverI420Frame(I420VideoFrame* video_frame) {
if (video_frame->native_handle() != NULL) {
- ViEFrameProviderBase::DeliverFrame(video_frame, std::vector<uint32_t>());
+ ViEFrameProviderBase::DeliverFrame(*video_frame, std::vector<uint32_t>());
return;
}
@@ -528,7 +528,7 @@
}
}
// Deliver the captured frame to all observers (channels, renderer or file).
- ViEFrameProviderBase::DeliverFrame(video_frame, std::vector<uint32_t>());
+ ViEFrameProviderBase::DeliverFrame(*video_frame, std::vector<uint32_t>());
}
bool ViECapturer::CaptureCapabilityFixed() {
diff --git a/webrtc/video_engine/vie_capturer_unittest.cc b/webrtc/video_engine/vie_capturer_unittest.cc
index 389f0bb..e97a082 100644
--- a/webrtc/video_engine/vie_capturer_unittest.cc
+++ b/webrtc/video_engine/vie_capturer_unittest.cc
@@ -87,10 +87,10 @@
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 82555d7..48a6299 100644
--- a/webrtc/video_engine/vie_channel.cc
+++ b/webrtc/video_engine/vie_channel.cc
@@ -1674,7 +1674,7 @@
no_of_csrcs = 1;
}
std::vector<uint32_t> 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 a03d38e..391766c 100644
--- a/webrtc/video_engine/vie_encoder.cc
+++ b/webrtc/video_engine/vie_encoder.cc
@@ -510,7 +510,7 @@
}
void ViEEncoder::DeliverFrame(int id,
- I420VideoFrame* video_frame,
+ const I420VideoFrame& video_frame,
const std::vector<uint32_t>& csrcs) {
DCHECK(send_payload_router_ != NULL);
DCHECK(csrcs.empty());
@@ -529,29 +529,29 @@
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<uint8_t[]> 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 @@
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 @@
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 8683b07..50d1c93 100644
--- a/webrtc/video_engine/vie_encoder.h
+++ b/webrtc/video_engine/vie_encoder.h
@@ -105,7 +105,7 @@
// Implementing ViEFrameCallback.
void DeliverFrame(int id,
- I420VideoFrame* video_frame,
+ const I420VideoFrame& video_frame,
const std::vector<uint32_t>& 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 c7493a4..a9ee1a8 100644
--- a/webrtc/video_engine/vie_frame_provider_base.cc
+++ b/webrtc/video_engine/vie_frame_provider_base.cc
@@ -45,7 +45,7 @@
return id_;
}
-void ViEFrameProviderBase::DeliverFrame(I420VideoFrame* video_frame,
+void ViEFrameProviderBase::DeliverFrame(const I420VideoFrame& video_frame,
const std::vector<uint32_t>& csrcs) {
DCHECK(frame_delivery_thread_checker_.CalledOnValidThread());
#ifdef DEBUG_
@@ -54,24 +54,9 @@
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<int>((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 b3ace3b..8ec2f4b 100644
--- a/webrtc/video_engine/vie_frame_provider_base.h
+++ b/webrtc/video_engine/vie_frame_provider_base.h
@@ -29,7 +29,7 @@
class ViEFrameCallback {
public:
virtual void DeliverFrame(int id,
- I420VideoFrame* video_frame,
+ const I420VideoFrame& video_frame,
const std::vector<uint32_t>& csrcs) = 0;
// The capture delay has changed from the provider. |frame_delay| is given in
@@ -77,7 +77,7 @@
virtual int FrameCallbackChanged() = 0;
protected:
- void DeliverFrame(I420VideoFrame* video_frame,
+ void DeliverFrame(const I420VideoFrame& video_frame,
const std::vector<uint32_t>& 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 bfeeba2..0deaf2f 100644
--- a/webrtc/video_engine/vie_renderer.cc
+++ b/webrtc/video_engine/vie_renderer.cc
@@ -122,9 +122,9 @@
}
void ViERenderer::DeliverFrame(int id,
- I420VideoFrame* video_frame,
+ const I420VideoFrame& video_frame,
const std::vector<uint32_t>& 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 5a7fdc1..55765a7 100644
--- a/webrtc/video_engine/vie_renderer.h
+++ b/webrtc/video_engine/vie_renderer.h
@@ -97,7 +97,7 @@
// Implement ViEFrameCallback
virtual void DeliverFrame(int id,
- I420VideoFrame* video_frame,
+ const I420VideoFrame& video_frame,
const std::vector<uint32_t>& csrcs);
virtual void DelayChanged(int id, int frame_delay);
virtual int GetPreferedFrameSettings(int* width,
