From d30a1117f842f43e089404981361350b53b4f159 Mon Sep 17 00:00:00 2001 From: nisse Date: Mon, 18 Apr 2016 05:15:22 -0700 Subject: [PATCH] Change pre_encode_callback to get a const frame. Used only by tests. Deleted the EndToEndTest.UsesFrameCallbacks, which modified pixel data. Change callback from in EndToEndTest.GetStats to call SleepMs, rath than modifying the timestamp. BUG= Review URL: https://codereview.webrtc.org/1891733002 Cr-Commit-Position: refs/heads/master@{#12406} --- webrtc/video/DEPS | 1 + webrtc/video/end_to_end_tests.cc | 110 +----------------------- webrtc/video/video_send_stream_tests.cc | 13 +-- webrtc/video/vie_encoder.cc | 9 +- webrtc/video/vie_encoder.h | 7 +- webrtc/video_send_stream.h | 3 +- 6 files changed, 20 insertions(+), 123 deletions(-) diff --git a/webrtc/video/DEPS b/webrtc/video/DEPS index 426f47c423..8c54066fd9 100644 --- a/webrtc/video/DEPS +++ b/webrtc/video/DEPS @@ -2,6 +2,7 @@ include_rules = [ "+webrtc/base", "+webrtc/call", "+webrtc/common_video", + "+webrtc/media/base", "+webrtc/modules/bitrate_controller", "+webrtc/modules/congestion_controller", "+webrtc/modules/pacing", diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc index e000b3f8b5..8c7d09d22a 100644 --- a/webrtc/video/end_to_end_tests.cc +++ b/webrtc/video/end_to_end_tests.cc @@ -864,107 +864,6 @@ TEST_F(EndToEndTest, DecodesRetransmittedFrameByRedOverRtx) { DecodesRetransmittedFrame(true, true); } -TEST_F(EndToEndTest, UsesFrameCallbacks) { - static const int kWidth = 320; - static const int kHeight = 240; - - class Renderer : public rtc::VideoSinkInterface { - public: - Renderer() : event_(false, false) {} - - void OnFrame(const VideoFrame& video_frame) override { - EXPECT_EQ(0, *video_frame.buffer(kYPlane)) - << "Rendered frame should have zero luma which is applied by the " - "pre-render callback."; - event_.Set(); - } - - bool Wait() { return event_.Wait(kDefaultTimeoutMs); } - rtc::Event event_; - } renderer; - - class TestFrameCallback : public I420FrameCallback { - public: - TestFrameCallback(int expected_luma_byte, int next_luma_byte) - : event_(false, false), - expected_luma_byte_(expected_luma_byte), - next_luma_byte_(next_luma_byte) {} - - bool Wait() { return event_.Wait(kDefaultTimeoutMs); } - - private: - virtual void FrameCallback(VideoFrame* frame) { - EXPECT_EQ(kWidth, frame->width()) - << "Width not as expected, callback done before resize?"; - EXPECT_EQ(kHeight, frame->height()) - << "Height not as expected, callback done before resize?"; - - // Previous luma specified, observed luma should be fairly close. - if (expected_luma_byte_ != -1) { - EXPECT_NEAR(expected_luma_byte_, *frame->buffer(kYPlane), 10); - } - - memset(frame->buffer(kYPlane), - next_luma_byte_, - frame->allocated_size(kYPlane)); - - event_.Set(); - } - - rtc::Event event_; - int expected_luma_byte_; - int next_luma_byte_; - }; - - TestFrameCallback pre_encode_callback(-1, 255); // Changes luma to 255. - TestFrameCallback pre_render_callback(255, 0); // Changes luma from 255 to 0. - - CreateCalls(Call::Config(), Call::Config()); - - test::DirectTransport sender_transport(sender_call_.get()); - test::DirectTransport receiver_transport(receiver_call_.get()); - sender_transport.SetReceiver(receiver_call_->Receiver()); - receiver_transport.SetReceiver(sender_call_->Receiver()); - - CreateSendConfig(1, 0, &sender_transport); - std::unique_ptr encoder( - VideoEncoder::Create(VideoEncoder::kVp8)); - video_send_config_.encoder_settings.encoder = encoder.get(); - video_send_config_.encoder_settings.payload_name = "VP8"; - ASSERT_EQ(1u, video_encoder_config_.streams.size()) << "Test setup error."; - video_encoder_config_.streams[0].width = kWidth; - video_encoder_config_.streams[0].height = kHeight; - video_send_config_.pre_encode_callback = &pre_encode_callback; - - CreateMatchingReceiveConfigs(&receiver_transport); - video_receive_configs_[0].pre_render_callback = &pre_render_callback; - video_receive_configs_[0].renderer = &renderer; - - CreateVideoStreams(); - Start(); - - // Create frames that are smaller than the send width/height, this is done to - // check that the callbacks are done after processing video. - std::unique_ptr frame_generator( - test::FrameGenerator::CreateChromaGenerator(kWidth / 2, kHeight / 2)); - video_send_stream_->Input()->IncomingCapturedFrame( - *frame_generator->NextFrame()); - - EXPECT_TRUE(pre_encode_callback.Wait()) - << "Timed out while waiting for pre-encode callback."; - 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."; - - Stop(); - - sender_transport.StopSending(); - receiver_transport.StopSending(); - - DestroyStreams(); -} - void EndToEndTest::ReceivesPliAndRecovers(int rtp_history_ms) { static const int kPacketsToDrop = 1; @@ -2571,7 +2470,8 @@ TEST_F(EndToEndTest, ReportsSetEncoderRates) { TEST_F(EndToEndTest, GetStats) { static const int kStartBitrateBps = 3000000; static const int kExpectedRenderDelayMs = 20; - class StatsObserver : public test::EndToEndTest, public I420FrameCallback { + class StatsObserver : public test::EndToEndTest, + public rtc::VideoSinkInterface { public: StatsObserver() : EndToEndTest(kLongTimeoutMs), @@ -2601,11 +2501,9 @@ TEST_F(EndToEndTest, GetStats) { return SEND_PACKET; } - void FrameCallback(VideoFrame* video_frame) override { + void OnFrame(const VideoFrame& video_frame) override { // Ensure that we have at least 5ms send side delay. - int64_t render_time = video_frame->render_time_ms(); - if (render_time > 0) - video_frame->set_render_time_ms(render_time - 5); + SleepMs(5); } bool CheckReceiveStats() { diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc index 57a3420e45..d3bf63a9bf 100644 --- a/webrtc/video/video_send_stream_tests.cc +++ b/webrtc/video/video_send_stream_tests.cc @@ -807,7 +807,8 @@ TEST_F(VideoSendStreamTest, FragmentsVp8AccordingToMaxPacketSizeWithFec) { TEST_F(VideoSendStreamTest, SuspendBelowMinBitrate) { static const int kSuspendTimeFrames = 60; // Suspend for 2 seconds @ 30 fps. - class RembObserver : public test::SendTest, public I420FrameCallback { + class RembObserver : public test::SendTest, + public rtc::VideoSinkInterface { public: RembObserver() : SendTest(kDefaultTimeoutMs), @@ -858,8 +859,8 @@ TEST_F(VideoSendStreamTest, SuspendBelowMinBitrate) { return SEND_PACKET; } - // This method implements the I420FrameCallback. - void FrameCallback(VideoFrame* video_frame) override { + // This method implements the rtc::VideoSinkInterface + void OnFrame(const VideoFrame& video_frame) override { rtc::CritScope lock(&crit_); if (test_state_ == kDuringSuspend && ++suspended_frame_count_ > kSuspendTimeFrames) { @@ -1168,12 +1169,12 @@ TEST_F(VideoSendStreamTest, CanReconfigureToUseStartBitrateAbovePreviousMax) { } TEST_F(VideoSendStreamTest, CapturesTextureAndVideoFrames) { - class FrameObserver : public I420FrameCallback { + class FrameObserver : public rtc::VideoSinkInterface { public: FrameObserver() : output_frame_event_(false, false) {} - void FrameCallback(VideoFrame* video_frame) override { - output_frames_.push_back(*video_frame); + void OnFrame(const VideoFrame& video_frame) override { + output_frames_.push_back(video_frame); output_frame_event_.Set(); } diff --git a/webrtc/video/vie_encoder.cc b/webrtc/video/vie_encoder.cc index e7d3539f9e..98169d2271 100644 --- a/webrtc/video/vie_encoder.cc +++ b/webrtc/video/vie_encoder.cc @@ -82,7 +82,7 @@ ViEEncoder::ViEEncoder(uint32_t number_of_cores, const std::vector& ssrcs, ProcessThread* module_process_thread, SendStatisticsProxy* stats_proxy, - I420FrameCallback* pre_encode_callback, + rtc::VideoSinkInterface* pre_encode_callback, OveruseFrameDetector* overuse_detector, PacedSender* pacer, PayloadRouter* payload_router) @@ -343,13 +343,8 @@ void ViEEncoder::EncodeVideoFrame(const VideoFrame& video_frame) { } } - // If we haven't resampled the frame and we have a FrameCallback, we need to - // make a deep copy of |video_frame|. - VideoFrame copied_frame; if (pre_encode_callback_) { - copied_frame.CopyFrame(*frame_to_send); - pre_encode_callback_->FrameCallback(&copied_frame); - frame_to_send = &copied_frame; + pre_encode_callback_->OnFrame(*frame_to_send); } if (codec_type == webrtc::kVideoCodecVP8) { diff --git a/webrtc/video/vie_encoder.h b/webrtc/video/vie_encoder.h index a0d93a3062..994c223ed0 100644 --- a/webrtc/video/vie_encoder.h +++ b/webrtc/video/vie_encoder.h @@ -18,7 +18,7 @@ #include "webrtc/base/scoped_ref_ptr.h" #include "webrtc/base/thread_annotations.h" #include "webrtc/common_types.h" -#include "webrtc/frame_callback.h" +#include "webrtc/media/base/videosinkinterface.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "webrtc/modules/video_coding/include/video_coding_defines.h" #include "webrtc/modules/video_coding/utility/ivf_file_writer.h" @@ -50,7 +50,8 @@ class ViEEncoder : public VideoEncoderRateObserver, const std::vector& ssrcs, ProcessThread* module_process_thread, SendStatisticsProxy* stats_proxy, - I420FrameCallback* pre_encode_callback, + // TODO(nisse): Used only for tests, delete? + rtc::VideoSinkInterface* pre_encode_callback, OveruseFrameDetector* overuse_detector, PacedSender* pacer, PayloadRouter* payload_router); @@ -134,7 +135,7 @@ class ViEEncoder : public VideoEncoderRateObserver, rtc::CriticalSection data_cs_; SendStatisticsProxy* const stats_proxy_; - I420FrameCallback* const pre_encode_callback_; + rtc::VideoSinkInterface* const pre_encode_callback_; OveruseFrameDetector* const overuse_detector_; PacedSender* const pacer_; PayloadRouter* const send_payload_router_; diff --git a/webrtc/video_send_stream.h b/webrtc/video_send_stream.h index 7815acfc80..a6ef1e0e85 100644 --- a/webrtc/video_send_stream.h +++ b/webrtc/video_send_stream.h @@ -17,6 +17,7 @@ #include "webrtc/common_types.h" #include "webrtc/config.h" #include "webrtc/frame_callback.h" +#include "webrtc/media/base/videosinkinterface.h" #include "webrtc/stream.h" #include "webrtc/transport.h" #include "webrtc/media/base/videosinkinterface.h" @@ -139,7 +140,7 @@ class VideoSendStream : public SendStream { // Called for each I420 frame before encoding the frame. Can be used for // effects, snapshots etc. 'nullptr' disables the callback. - I420FrameCallback* pre_encode_callback = nullptr; + rtc::VideoSinkInterface* pre_encode_callback = nullptr; // Called for each encoded frame, e.g. used for file storage. 'nullptr' // disables the callback. Also measures timing and passes the time