From f4c10d24dc8f1c3ce6859644077d7df6fb678dcd Mon Sep 17 00:00:00 2001 From: "pbos@webrtc.org" Date: Tue, 10 Feb 2015 10:19:32 +0000 Subject: [PATCH] Always use DeliverI420Frame in WebRtcVideoEngine. Moves native_handle() path to DeliverI420Frame and CHECKs that DeliverFrame is not being used anymore. R=magjed@webrtc.org, mflodman@webrtc.org BUG= Review URL: https://webrtc-codereview.appspot.com/38019004 Cr-Commit-Position: refs/heads/master@{#8312} git-svn-id: http://webrtc.googlecode.com/svn/trunk@8312 4adac7df-926f-26a2-2b94-8c16560cd09d --- talk/media/webrtc/webrtcvideoengine.cc | 57 ++++--------------- webrtc/video/video_receive_stream.cc | 6 +- webrtc/video/video_receive_stream.h | 2 +- webrtc/video_engine/include/vie_render.h | 2 +- .../primitives/framedrop_primitives.cc | 4 +- .../primitives/framedrop_primitives.h | 2 +- .../auto_test/source/vie_autotest_render.cc | 7 +-- .../helpers/vie_to_file_renderer.cc | 13 ++--- .../libvietest/include/vie_to_file_renderer.h | 2 +- webrtc/video_engine/vie_renderer.cc | 29 ++++++---- webrtc/video_engine/vie_renderer.h | 2 + 11 files changed, 49 insertions(+), 77 deletions(-) diff --git a/talk/media/webrtc/webrtcvideoengine.cc b/talk/media/webrtc/webrtcvideoengine.cc index 2e500df33f..32de61c20f 100644 --- a/talk/media/webrtc/webrtcvideoengine.cc +++ b/talk/media/webrtc/webrtcvideoengine.cc @@ -382,68 +382,31 @@ class WebRtcRenderAdapter : public webrtc::ExternalRenderer { int64_t ntp_time_ms, int64_t render_time, void* handle) { - rtc::CritScope cs(&crit_); - const int64_t elapsed_time_ms = ElapsedTimeMs(rtp_time_stamp); - UpdateFrameStats(elapsed_time_ms, ntp_time_ms); - if (!renderer_) { - return 0; - } - // Convert elapsed_time_ms to ns timestamp. - int64 elapsed_time_ns = - elapsed_time_ms * rtc::kNumNanosecsPerMillisec; - // Convert milisecond render time to ns timestamp. - int64 render_time_ns = render_time * - rtc::kNumNanosecsPerMillisec; - // Note that here we send the |elapsed_time_ns| to renderer as the - // cricket::VideoFrame's elapsed_time_ and the |render_time_ns| as the - // cricket::VideoFrame's time_stamp_. - if (!handle) { - return DeliverBufferFrame(buffer, buffer_size, render_time_ns, - elapsed_time_ns); - } else { - return DeliverTextureFrame(handle, render_time_ns, - elapsed_time_ns); - } + CHECK(false) << "All frames should be delivered as I420 frames through " + "DeliverI420Frame."; + return 0; } - virtual int DeliverI420Frame(const webrtc::I420VideoFrame* webrtc_frame) { + virtual int DeliverI420Frame(const webrtc::I420VideoFrame& webrtc_frame) { rtc::CritScope cs(&crit_); - DCHECK(webrtc_frame); - const int64_t elapsed_time_ms = ElapsedTimeMs(webrtc_frame->timestamp()); - UpdateFrameStats(elapsed_time_ms, webrtc_frame->ntp_time_ms()); + const int64_t elapsed_time_ms = ElapsedTimeMs(webrtc_frame.timestamp()); + UpdateFrameStats(elapsed_time_ms, webrtc_frame.ntp_time_ms()); if (!renderer_) { return 0; } - if (!webrtc_frame->native_handle()) { - WebRtcVideoRenderFrame cricket_frame(webrtc_frame, elapsed_time_ms); + if (webrtc_frame.native_handle() == NULL) { + WebRtcVideoRenderFrame cricket_frame(&webrtc_frame, elapsed_time_ms); return renderer_->RenderFrame(&cricket_frame) ? 0 : -1; } else { return DeliverTextureFrame( - webrtc_frame->native_handle(), - webrtc_frame->render_time_ms() * rtc::kNumNanosecsPerMillisec, + webrtc_frame.native_handle(), + webrtc_frame.render_time_ms() * rtc::kNumNanosecsPerMillisec, elapsed_time_ms * rtc::kNumNanosecsPerMillisec); } } virtual bool IsTextureSupported() { return true; } - int DeliverBufferFrame(unsigned char* buffer, size_t buffer_size, - int64 time_stamp, int64 elapsed_time) { - WebRtcVideoFrame video_frame; - video_frame.Alias(buffer, buffer_size, width_, height_, 1, 1, elapsed_time, - time_stamp, webrtc::kVideoRotation_0); - - // Sanity check on decoded frame size. - if (buffer_size != VideoFrame::SizeOf(width_, height_)) { - LOG(LS_WARNING) << "WebRtcRenderAdapter (channel " << channel_id_ - << ") received a strange frame size: " - << buffer_size; - } - - int ret = renderer_->RenderFrame(&video_frame) ? 0 : -1; - return ret; - } - int DeliverTextureFrame(void* handle, int64 time_stamp, int64 elapsed_time) { WebRtcTextureVideoFrame video_frame( static_cast(handle), width_, height_, diff --git a/webrtc/video/video_receive_stream.cc b/webrtc/video/video_receive_stream.cc index 25c4758ab4..fee90f44ff 100644 --- a/webrtc/video/video_receive_stream.cc +++ b/webrtc/video/video_receive_stream.cc @@ -361,11 +361,11 @@ int VideoReceiveStream::DeliverFrame(unsigned char* buffer, return 0; } -int VideoReceiveStream::DeliverI420Frame(const I420VideoFrame* video_frame) { +int VideoReceiveStream::DeliverI420Frame(const I420VideoFrame& video_frame) { if (config_.renderer != NULL) config_.renderer->RenderFrame( - *video_frame, - video_frame->render_time_ms() - clock_->TimeInMilliseconds()); + video_frame, + video_frame.render_time_ms() - clock_->TimeInMilliseconds()); stats_proxy_->OnRenderedFrame(); diff --git a/webrtc/video/video_receive_stream.h b/webrtc/video/video_receive_stream.h index eec18fb8b5..83ee53fb1c 100644 --- a/webrtc/video/video_receive_stream.h +++ b/webrtc/video/video_receive_stream.h @@ -66,7 +66,7 @@ class VideoReceiveStream : public webrtc::VideoReceiveStream, int64_t ntp_time_ms, int64_t render_time_ms, void* handle) override; - virtual int DeliverI420Frame(const I420VideoFrame* webrtc_frame) override; + virtual int DeliverI420Frame(const I420VideoFrame& webrtc_frame) override; virtual bool IsTextureSupported() override; void SignalNetworkState(Call::NetworkState state); diff --git a/webrtc/video_engine/include/vie_render.h b/webrtc/video_engine/include/vie_render.h index e268c4a7c5..dde2155070 100644 --- a/webrtc/video_engine/include/vie_render.h +++ b/webrtc/video_engine/include/vie_render.h @@ -49,7 +49,7 @@ class ExternalRenderer { void* handle) = 0; // Alternative interface for I420 frames. - virtual int DeliverI420Frame(const I420VideoFrame* webrtc_frame) = 0; + virtual int DeliverI420Frame(const I420VideoFrame& webrtc_frame) = 0; // Returns true if the renderer supports textures. DeliverFrame can be called // with NULL |buffer| and non-NULL |handle|. diff --git a/webrtc/video_engine/test/auto_test/primitives/framedrop_primitives.cc b/webrtc/video_engine/test/auto_test/primitives/framedrop_primitives.cc index b360c90f60..e9ff5378e9 100644 --- a/webrtc/video_engine/test/auto_test/primitives/framedrop_primitives.cc +++ b/webrtc/video_engine/test/auto_test/primitives/framedrop_primitives.cc @@ -617,8 +617,8 @@ void FrameDropMonitoringRemoteFileRenderer::ReportFrameStats( } int FrameDropMonitoringRemoteFileRenderer::DeliverI420Frame( - const webrtc::I420VideoFrame* webrtc_frame) { - ReportFrameStats(webrtc_frame->timestamp(), webrtc_frame->render_time_ms()); + const webrtc::I420VideoFrame& webrtc_frame) { + ReportFrameStats(webrtc_frame.timestamp(), webrtc_frame.render_time_ms()); return ViEToFileRenderer::DeliverI420Frame(webrtc_frame); } diff --git a/webrtc/video_engine/test/auto_test/primitives/framedrop_primitives.h b/webrtc/video_engine/test/auto_test/primitives/framedrop_primitives.h index 317b760065..a8cdfbd7bc 100644 --- a/webrtc/video_engine/test/auto_test/primitives/framedrop_primitives.h +++ b/webrtc/video_engine/test/auto_test/primitives/framedrop_primitives.h @@ -230,7 +230,7 @@ class FrameDropMonitoringRemoteFileRenderer : public ViEToFileRenderer { int64_t ntp_time_ms, int64_t render_time, void* handle) OVERRIDE; - int DeliverI420Frame(const webrtc::I420VideoFrame* webrtc_frame) OVERRIDE; + int DeliverI420Frame(const webrtc::I420VideoFrame& webrtc_frame) OVERRIDE; private: void ReportFrameStats(uint32_t time_stamp, int64_t render_time); diff --git a/webrtc/video_engine/test/auto_test/source/vie_autotest_render.cc b/webrtc/video_engine/test/auto_test/source/vie_autotest_render.cc index 3e5be49340..46a5d7f301 100644 --- a/webrtc/video_engine/test/auto_test/source/vie_autotest_render.cc +++ b/webrtc/video_engine/test/auto_test/source/vie_autotest_render.cc @@ -71,10 +71,9 @@ public: return 0; } - virtual int DeliverI420Frame(const webrtc::I420VideoFrame* webrtc_frame) { - EXPECT_TRUE(webrtc_frame); - EXPECT_EQ(webrtc_frame->width(), _width); - EXPECT_EQ(webrtc_frame->height(), _height); + virtual int DeliverI420Frame(const webrtc::I420VideoFrame& webrtc_frame) { + EXPECT_EQ(webrtc_frame.width(), _width); + EXPECT_EQ(webrtc_frame.height(), _height); return 0; } diff --git a/webrtc/video_engine/test/libvietest/helpers/vie_to_file_renderer.cc b/webrtc/video_engine/test/libvietest/helpers/vie_to_file_renderer.cc index 7ec6db8858..6da7e0c490 100644 --- a/webrtc/video_engine/test/libvietest/helpers/vie_to_file_renderer.cc +++ b/webrtc/video_engine/test/libvietest/helpers/vie_to_file_renderer.cc @@ -146,19 +146,18 @@ int ViEToFileRenderer::DeliverFrame(unsigned char *buffer, } int ViEToFileRenderer::DeliverI420Frame( - const webrtc::I420VideoFrame* input_frame) { - assert(input_frame); - const size_t buffer_size = CalcBufferSize(webrtc::kI420, input_frame->width(), - input_frame->height()); + const webrtc::I420VideoFrame& input_frame) { + const size_t buffer_size = + CalcBufferSize(webrtc::kI420, input_frame.width(), input_frame.height()); webrtc::CriticalSectionScoped lock(frame_queue_cs_.get()); test::Frame* frame = NewFrame(buffer_size); const int length = - ExtractBuffer(*input_frame, frame->buffer_size, frame->buffer.get()); + ExtractBuffer(input_frame, frame->buffer_size, frame->buffer.get()); assert(static_cast(length) == buffer_size); if (length < 0) return -1; - frame->timestamp = input_frame->timestamp(); - frame->render_time = input_frame->render_time_ms(); + frame->timestamp = input_frame.timestamp(); + frame->render_time = input_frame.render_time_ms(); render_queue_.push_back(frame); // Signal that a frame is ready to be written to file. diff --git a/webrtc/video_engine/test/libvietest/include/vie_to_file_renderer.h b/webrtc/video_engine/test/libvietest/include/vie_to_file_renderer.h index fc75c42756..bf6dd8c2cd 100644 --- a/webrtc/video_engine/test/libvietest/include/vie_to_file_renderer.h +++ b/webrtc/video_engine/test/libvietest/include/vie_to_file_renderer.h @@ -64,7 +64,7 @@ class ViEToFileRenderer: public webrtc::ExternalRenderer { int64_t render_time, void* handle) OVERRIDE; - int DeliverI420Frame(const webrtc::I420VideoFrame* webrtc_frame) OVERRIDE; + int DeliverI420Frame(const webrtc::I420VideoFrame& webrtc_frame) OVERRIDE; bool IsTextureSupported() OVERRIDE; diff --git a/webrtc/video_engine/vie_renderer.cc b/webrtc/video_engine/vie_renderer.cc index 976cef6730..5c053c3c71 100644 --- a/webrtc/video_engine/vie_renderer.cc +++ b/webrtc/video_engine/vie_renderer.cc @@ -170,9 +170,25 @@ int ViEExternalRendererImpl::SetViEExternalRenderer( return 0; } -int32_t ViEExternalRendererImpl::RenderFrame( - const uint32_t stream_id, - I420VideoFrame& video_frame) { +int32_t ViEExternalRendererImpl::RenderFrame(const uint32_t stream_id, + I420VideoFrame& video_frame) { + if (external_renderer_format_ != kVideoI420) + return ConvertAndRenderFrame(stream_id, video_frame); + + // Fast path for I420 without frame copy. + NotifyFrameSizeChange(stream_id, video_frame); + if (video_frame.native_handle() == NULL || + external_renderer_->IsTextureSupported()) { + external_renderer_->DeliverI420Frame(video_frame); + } else { + // TODO(wuchengli): readback the pixels and deliver the frame. + } + return 0; +} + +int32_t ViEExternalRendererImpl::ConvertAndRenderFrame( + uint32_t stream_id, + I420VideoFrame& video_frame) { if (video_frame.native_handle() != NULL) { NotifyFrameSizeChange(stream_id, video_frame); @@ -189,13 +205,6 @@ int32_t ViEExternalRendererImpl::RenderFrame( return 0; } - // Fast path for I420 without frame copy. - if (external_renderer_format_ == kVideoI420) { - NotifyFrameSizeChange(stream_id, video_frame); - external_renderer_->DeliverI420Frame(&video_frame); - return 0; - } - VideoFrame* out_frame = converted_frame_.get(); // Convert to requested format. diff --git a/webrtc/video_engine/vie_renderer.h b/webrtc/video_engine/vie_renderer.h index 67efaee800..c2466f6209 100644 --- a/webrtc/video_engine/vie_renderer.h +++ b/webrtc/video_engine/vie_renderer.h @@ -37,6 +37,8 @@ class ViEExternalRendererImpl : public VideoRenderCallback { private: void NotifyFrameSizeChange(const uint32_t stream_id, I420VideoFrame& video_frame); + int32_t ConvertAndRenderFrame(uint32_t stream_id, + I420VideoFrame& video_frame); ExternalRenderer* external_renderer_; RawVideoType external_renderer_format_; int external_renderer_width_;