From 76bc8e858ff288449163c97aa66928f2eecdfd28 Mon Sep 17 00:00:00 2001 From: nisse Date: Tue, 7 Feb 2017 09:37:41 -0800 Subject: [PATCH] Delete VideoReceiveStream::Config::pre_render_callback. Also delete the class I420FrameCallback. BUG=webrtc:7124 Review-Url: https://codereview.webrtc.org/2678343002 Cr-Commit-Position: refs/heads/master@{#16478} --- webrtc/common_video/include/frame_callback.h | 10 ----- webrtc/video/end_to_end_tests.cc | 44 ++++++++------------ webrtc/video/video_receive_stream.cc | 4 +- webrtc/video/video_send_stream.cc | 2 +- webrtc/video/video_stream_decoder.cc | 11 +---- webrtc/video/video_stream_decoder.h | 21 ++++------ webrtc/video_receive_stream.h | 8 ---- 7 files changed, 29 insertions(+), 71 deletions(-) diff --git a/webrtc/common_video/include/frame_callback.h b/webrtc/common_video/include/frame_callback.h index 0fa4458f5c..45624f1554 100644 --- a/webrtc/common_video/include/frame_callback.h +++ b/webrtc/common_video/include/frame_callback.h @@ -50,16 +50,6 @@ struct EncodedFrame { const uint32_t timestamp_; }; -class I420FrameCallback { - public: - // This function is called with a I420 frame allowing the user to modify the - // frame content. - virtual void FrameCallback(VideoFrame* video_frame) = 0; - - protected: - virtual ~I420FrameCallback() {} -}; - class EncodedFrameObserver { public: virtual void EncodedFrameCallback(const EncodedFrame& encoded_frame) = 0; diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc index dbaa87b1b2..af9dfe15bf 100644 --- a/webrtc/video/end_to_end_tests.cc +++ b/webrtc/video/end_to_end_tests.cc @@ -195,34 +195,22 @@ TEST_P(EndToEndTest, RendersSingleDelayedFrame) { // This constant is chosen to be higher than the timeout in the video_render // module. This makes sure that frames aren't dropped if there are no other // frames in the queue. - static const int kDelayRenderCallbackMs = 1000; + static const int kRenderDelayMs = 1000; class Renderer : public rtc::VideoSinkInterface { public: Renderer() : event_(false, false) {} - void OnFrame(const VideoFrame& video_frame) override { event_.Set(); } + void OnFrame(const VideoFrame& video_frame) override { + SleepMs(kRenderDelayMs); + event_.Set(); + } bool Wait() { return event_.Wait(kDefaultTimeoutMs); } rtc::Event event_; } renderer; - class TestFrameCallback : public I420FrameCallback { - public: - TestFrameCallback() : event_(false, false) {} - - bool Wait() { return event_.Wait(kDefaultTimeoutMs); } - - private: - void FrameCallback(VideoFrame* frame) override { - SleepMs(kDelayRenderCallbackMs); - event_.Set(); - } - - rtc::Event event_; - }; - CreateCalls(Call::Config(&event_log_), Call::Config(&event_log_)); test::DirectTransport sender_transport(sender_call_.get()); @@ -233,8 +221,6 @@ TEST_P(EndToEndTest, RendersSingleDelayedFrame) { CreateSendConfig(1, 0, 0, &sender_transport); CreateMatchingReceiveConfigs(&receiver_transport); - TestFrameCallback pre_render_callback; - video_receive_configs_[0].pre_render_callback = &pre_render_callback; video_receive_configs_[0].renderer = &renderer; CreateVideoStreams(); @@ -249,8 +235,6 @@ TEST_P(EndToEndTest, RendersSingleDelayedFrame) { &frame_forwarder, VideoSendStream::DegradationPreference::kBalanced); frame_forwarder.IncomingCapturedFrame(*frame_generator->NextFrame()); - EXPECT_TRUE(pre_render_callback.Wait()) - << "Timed out while waiting for pre-render callback."; EXPECT_TRUE(renderer.Wait()) << "Timed out while waiting for the frame to render."; @@ -1052,7 +1036,7 @@ TEST_P(EndToEndTest, ReceivedUlpfecPacketsNotNacked) { void EndToEndTest::DecodesRetransmittedFrame(bool enable_rtx, bool enable_red) { static const int kDroppedFrameNumber = 10; class RetransmissionObserver : public test::EndToEndTest, - public I420FrameCallback { + public rtc::VideoSinkInterface { public: RetransmissionObserver(bool enable_rtx, bool enable_red) : EndToEndTest(kDefaultTimeoutMs), @@ -1108,11 +1092,12 @@ void EndToEndTest::DecodesRetransmittedFrame(bool enable_rtx, bool enable_red) { return SEND_PACKET; } - void FrameCallback(VideoFrame* frame) override { + void OnFrame(const VideoFrame& frame) override { rtc::CritScope lock(&crit_); - if (frame->timestamp() == retransmitted_timestamp_) + if (frame.timestamp() == retransmitted_timestamp_) observation_complete_.Set(); - rendered_timestamps_.push_back(frame->timestamp()); + rendered_timestamps_.push_back(frame.timestamp()); + orig_renderer_->OnFrame(frame); } void ModifyVideoConfigs( @@ -1120,7 +1105,13 @@ void EndToEndTest::DecodesRetransmittedFrame(bool enable_rtx, bool enable_red) { std::vector* receive_configs, VideoEncoderConfig* encoder_config) override { send_config->rtp.nack.rtp_history_ms = kNackRtpHistoryMs; - (*receive_configs)[0].pre_render_callback = this; + + // Insert ourselves into the rendering pipeline. + RTC_DCHECK(!orig_renderer_); + orig_renderer_ = (*receive_configs)[0].renderer; + RTC_DCHECK(orig_renderer_); + (*receive_configs)[0].renderer = this; + (*receive_configs)[0].rtp.nack.rtp_history_ms = kNackRtpHistoryMs; if (payload_type_ == kRedPayloadType) { @@ -1168,6 +1159,7 @@ void EndToEndTest::DecodesRetransmittedFrame(bool enable_rtx, bool enable_red) { } rtc::CriticalSection crit_; + rtc::VideoSinkInterface* orig_renderer_ = nullptr; const int payload_type_; const uint32_t retransmission_ssrc_; const int retransmission_payload_type_; diff --git a/webrtc/video/video_receive_stream.cc b/webrtc/video/video_receive_stream.cc index 36a1a8b290..bb4e9f603f 100644 --- a/webrtc/video/video_receive_stream.cc +++ b/webrtc/video/video_receive_stream.cc @@ -67,8 +67,6 @@ std::string VideoReceiveStream::Config::ToString() const { ss << ", sync_group: " << sync_group; ss << ", pre_decode_callback: " << (pre_decode_callback ? "(EncodedFrameObserver)" : "nullptr"); - ss << ", pre_render_callback: " - << (pre_render_callback ? "(I420FrameCallback)" : "nullptr"); ss << ", target_delay_ms: " << target_delay_ms; ss << '}'; @@ -309,7 +307,7 @@ void VideoReceiveStream::Start() { video_stream_decoder_.reset(new VideoStreamDecoder( &video_receiver_, &rtp_stream_receiver_, &rtp_stream_receiver_, rtp_stream_receiver_.IsRetransmissionsEnabled(), protected_by_fec, - &stats_proxy_, renderer, config_.pre_render_callback)); + &stats_proxy_, renderer)); // Register the channel to receive stats updates. call_stats_->RegisterStatsObserver(video_stream_decoder_.get()); // Start the decode thread diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index 08ffa3fb43..466b5c2f00 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -206,7 +206,7 @@ std::string VideoSendStream::Config::ToString() const { ss << "{encoder_settings: " << encoder_settings.ToString(); ss << ", rtp: " << rtp.ToString(); ss << ", pre_encode_callback: " - << (pre_encode_callback ? "(I420FrameCallback)" : "nullptr"); + << (pre_encode_callback ? "(VideoSinkInterface)" : "nullptr"); ss << ", post_encode_callback: " << (post_encode_callback ? "(EncodedFrameObserver)" : "nullptr"); ss << ", render_delay_ms: " << render_delay_ms; diff --git a/webrtc/video/video_stream_decoder.cc b/webrtc/video/video_stream_decoder.cc index a7466eeca7..b0308f25d8 100644 --- a/webrtc/video/video_stream_decoder.cc +++ b/webrtc/video/video_stream_decoder.cc @@ -32,12 +32,10 @@ VideoStreamDecoder::VideoStreamDecoder( bool enable_nack, bool enable_fec, ReceiveStatisticsProxy* receive_statistics_proxy, - rtc::VideoSinkInterface* incoming_video_stream, - I420FrameCallback* pre_render_callback) + rtc::VideoSinkInterface* incoming_video_stream) : video_receiver_(video_receiver), receive_stats_callback_(receive_statistics_proxy), incoming_video_stream_(incoming_video_stream), - pre_render_callback_(pre_render_callback), last_rtt_ms_(0) { RTC_DCHECK(video_receiver_); @@ -77,13 +75,6 @@ VideoStreamDecoder::~VideoStreamDecoder() { // Release. Acquiring the same lock in the path of decode callback can deadlock. int32_t VideoStreamDecoder::FrameToRender(VideoFrame& video_frame, rtc::Optional qp) { - if (pre_render_callback_) { - // Post processing is not supported if the frame is backed by a texture. - if (!video_frame.video_frame_buffer()->native_handle()) { - pre_render_callback_->FrameCallback(&video_frame); - } - } - receive_stats_callback_->OnDecodedFrame(qp); incoming_video_stream_->OnFrame(video_frame); diff --git a/webrtc/video/video_stream_decoder.h b/webrtc/video/video_stream_decoder.h index be2c938887..270c16b986 100644 --- a/webrtc/video/video_stream_decoder.h +++ b/webrtc/video/video_stream_decoder.h @@ -29,7 +29,6 @@ namespace webrtc { class CallStatsObserver; class ChannelStatsObserver; class EncodedImageCallback; -class I420FrameCallback; class ReceiveStatisticsProxy; class VideoRenderCallback; @@ -49,14 +48,14 @@ class VideoStreamDecoder : public VCMReceiveCallback, public: friend class ChannelStatsObserver; - VideoStreamDecoder(vcm::VideoReceiver* video_receiver, - VCMFrameTypeCallback* vcm_frame_type_callback, - VCMPacketRequestCallback* vcm_packet_request_callback, - bool enable_nack, - bool enable_fec, - ReceiveStatisticsProxy* receive_statistics_proxy, - rtc::VideoSinkInterface* incoming_video_stream, - I420FrameCallback* pre_render_callback); + VideoStreamDecoder( + vcm::VideoReceiver* video_receiver, + VCMFrameTypeCallback* vcm_frame_type_callback, + VCMPacketRequestCallback* vcm_packet_request_callback, + bool enable_nack, + bool enable_fec, + ReceiveStatisticsProxy* receive_statistics_proxy, + rtc::VideoSinkInterface* incoming_video_stream); ~VideoStreamDecoder(); // Implements VCMReceiveCallback. @@ -103,10 +102,6 @@ class VideoStreamDecoder : public VCMReceiveCallback, ReceiveStatisticsProxy* const receive_stats_callback_; rtc::VideoSinkInterface* const incoming_video_stream_; - // TODO(tommi): This callback is basically the same thing as the one above. - // We shouldn't need to support both. - I420FrameCallback* const pre_render_callback_; - int64_t last_rtt_ms_ GUARDED_BY(crit_); }; diff --git a/webrtc/video_receive_stream.h b/webrtc/video_receive_stream.h index 3158ebde89..12629d0637 100644 --- a/webrtc/video_receive_stream.h +++ b/webrtc/video_receive_stream.h @@ -180,14 +180,6 @@ class VideoReceiveStream { // saving the stream to a file. 'nullptr' disables the callback. EncodedFrameObserver* pre_decode_callback = nullptr; - // Called for each decoded frame. E.g. used when adding effects to the - // decoded - // stream. 'nullptr' disables the callback. - // TODO(tommi): This seems to be only used by a test or two. Consider - // removing it (and use an appropriate alternative in the tests) as well - // as the associated code in VideoStreamDecoder. - I420FrameCallback* pre_render_callback = nullptr; - // Target delay in milliseconds. A positive value indicates this stream is // used for streaming instead of a real-time call. int target_delay_ms = 0;