From 83f831a919ff4c2d736ec3c5a33c6afb4d5eea47 Mon Sep 17 00:00:00 2001 From: philipel Date: Sat, 12 Mar 2016 03:30:23 -0800 Subject: [PATCH] Experiment for the nack module. Testing the nack module by implementing it into the current jitter buffer under the experiment WebRTC-NewVideoJitterBuffer. BUG=webrtc:5514 Review URL: https://codereview.webrtc.org/1778503002 Cr-Commit-Position: refs/heads/master@{#11969} --- webrtc/modules/rtp_rtcp/include/rtp_rtcp.h | 9 + webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h | 1 + .../modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 10 +- .../modules/rtp_rtcp/source/rtp_rtcp_impl.h | 3 + webrtc/modules/video_coding/BUILD.gn | 4 + .../video_coding/include/video_coding.h | 13 + webrtc/modules/video_coding/jitter_buffer.cc | 56 ++++- webrtc/modules/video_coding/jitter_buffer.h | 13 +- .../video_coding/jitter_buffer_unittest.cc | 228 +++++++++++++----- webrtc/modules/video_coding/nack_module.cc | 6 + webrtc/modules/video_coding/nack_module.h | 1 + webrtc/modules/video_coding/receiver.cc | 44 +++- webrtc/modules/video_coding/receiver.h | 23 ++ .../modules/video_coding/video_coding_impl.cc | 46 +++- .../modules/video_coding/video_coding_impl.h | 5 +- webrtc/modules/video_coding/video_receiver.cc | 17 +- webrtc/video/video_receive_stream.cc | 15 +- webrtc/video/video_receive_stream.h | 10 +- 18 files changed, 421 insertions(+), 83 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h b/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h index 50bb6a0878..1d01009f35 100644 --- a/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h @@ -18,6 +18,7 @@ #include "webrtc/modules/include/module.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" +#include "webrtc/modules/video_coding/include/video_coding_defines.h" namespace webrtc { // Forward declarations. @@ -534,8 +535,16 @@ class RtpRtcp : public Module { * * return -1 on failure else 0 */ + // TODO(philipel): Deprecate this and start using SendNack instead, + // mostly because we want a function that actually send + // NACK for the specified packets. virtual int32_t SendNACK(const uint16_t* nackList, uint16_t size) = 0; + /* + * Send NACK for the packets specified. + */ + virtual void SendNack(const std::vector& sequence_numbers) = 0; + /* * Store the sent packets, needed to answer to a Negative acknowledgement * requests diff --git a/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h index 2281e36db1..4a4bb22e00 100644 --- a/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h @@ -212,6 +212,7 @@ class MockRtpRtcp : public RtpRtcp { int(uint8_t settings)); MOCK_METHOD2(SendNACK, int32_t(const uint16_t* nackList, const uint16_t size)); + MOCK_METHOD1(SendNack, void(const std::vector& sequence_numbers)); MOCK_METHOD2(SetStorePacketsStatus, void(const bool enable, const uint16_t numberToStore)); MOCK_CONST_METHOD0(StorePackets, bool()); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 86ed6cec3a..e67b635214 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -742,8 +742,14 @@ int32_t ModuleRtpRtcpImpl::SendNACK(const uint16_t* nack_list, } nack_last_seq_number_sent_ = nack_list[start_id + nack_length - 1]; - return rtcp_sender_.SendRTCP( - GetFeedbackState(), kRtcpNack, nack_length, &nack_list[start_id]); + return rtcp_sender_.SendRTCP(GetFeedbackState(), kRtcpNack, nack_length, + &nack_list[start_id]); +} + +void ModuleRtpRtcpImpl::SendNack( + const std::vector& sequence_numbers) { + rtcp_sender_.SendRTCP(GetFeedbackState(), kRtcpNack, sequence_numbers.size(), + sequence_numbers.data()); } bool ModuleRtpRtcpImpl::TimeToSendFullNackList(int64_t now) const { diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h index ba0ed0e4c7..76faca0f7e 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -219,8 +219,11 @@ class ModuleRtpRtcpImpl : public RtpRtcp { int SetSelectiveRetransmissions(uint8_t settings) override; // Send a Negative acknowledgment packet. + // TODO(philipel): Deprecate SendNACK and use SendNack instead. int32_t SendNACK(const uint16_t* nack_list, uint16_t size) override; + void SendNack(const std::vector& sequence_numbers) override; + // Store the sent packets, needed to answer to a negative acknowledgment // requests. void SetStorePacketsStatus(bool enable, uint16_t number_to_store) override; diff --git a/webrtc/modules/video_coding/BUILD.gn b/webrtc/modules/video_coding/BUILD.gn index afec84f9a7..37a0d2cb8f 100644 --- a/webrtc/modules/video_coding/BUILD.gn +++ b/webrtc/modules/video_coding/BUILD.gn @@ -29,6 +29,8 @@ source_set("video_coding") { "generic_decoder.h", "generic_encoder.cc", "generic_encoder.h", + "histogram.cc", + "histogram.h", "include/video_coding.h", "include/video_coding_defines.h", "inter_frame_delay.cc", @@ -44,6 +46,8 @@ source_set("video_coding") { "media_optimization.cc", "media_optimization.h", "nack_fec_tables.h", + "nack_module.cc", + "nack_module.h", "packet.cc", "packet.h", "percentile_filter.cc", diff --git a/webrtc/modules/video_coding/include/video_coding.h b/webrtc/modules/video_coding/include/video_coding.h index 6433f5628d..0c508b7739 100644 --- a/webrtc/modules/video_coding/include/video_coding.h +++ b/webrtc/modules/video_coding/include/video_coding.h @@ -73,8 +73,21 @@ class VideoCodingModule : public Module { VideoEncoderRateObserver* encoder_rate_observer, VCMQMSettingsCallback* qm_settings_callback); + static VideoCodingModule* Create( + Clock* clock, + VideoEncoderRateObserver* encoder_rate_observer, + VCMQMSettingsCallback* qm_settings_callback, + NackSender* nack_sender, + KeyFrameRequestSender* keyframe_request_sender); + static VideoCodingModule* Create(Clock* clock, EventFactory* event_factory); + static VideoCodingModule* Create( + Clock* clock, + EventFactory* event_factory, + NackSender* nack_sender, + KeyFrameRequestSender* keyframe_request_sender); + // Get supported codec settings using codec type // // Input: diff --git a/webrtc/modules/video_coding/jitter_buffer.cc b/webrtc/modules/video_coding/jitter_buffer.cc index ca8c0d9571..a3e0b2bc07 100644 --- a/webrtc/modules/video_coding/jitter_buffer.cc +++ b/webrtc/modules/video_coding/jitter_buffer.cc @@ -12,6 +12,7 @@ #include #include +#include #include #include "webrtc/base/checks.h" @@ -28,10 +29,10 @@ #include "webrtc/system_wrappers/include/clock.h" #include "webrtc/system_wrappers/include/critical_section_wrapper.h" #include "webrtc/system_wrappers/include/event_wrapper.h" +#include "webrtc/system_wrappers/include/field_trial.h" #include "webrtc/system_wrappers/include/metrics.h" namespace webrtc { - // Interval for updating SS data. static const uint32_t kSsCleanupIntervalSec = 60; @@ -214,8 +215,24 @@ void Vp9SsMap::UpdateFrames(FrameList* frames) { } } +static NackModule* +MaybeCreateNackModule(Clock* clock, + NackSender* nack_sender, + KeyFrameRequestSender* keyframe_request_sender) { +if (field_trial::FindFullName("WebRTC-NewVideoJitterBuffer") == "Enabled") { + // Only create a nack module if a nack sender is available. + if (nack_sender) { + RTC_DCHECK(keyframe_request_sender); + return new NackModule(clock, nack_sender, keyframe_request_sender); + } + } + return nullptr; +} + VCMJitterBuffer::VCMJitterBuffer(Clock* clock, - std::unique_ptr event) + std::unique_ptr event, + NackSender* nack_sender, + KeyFrameRequestSender* keyframe_request_sender) : clock_(clock), running_(false), crit_sect_(CriticalSectionWrapper::CreateCriticalSection()), @@ -249,7 +266,10 @@ VCMJitterBuffer::VCMJitterBuffer(Clock* clock, max_incomplete_time_ms_(0), decode_error_mode_(kNoErrors), average_packets_per_frame_(0.0f), - frame_counter_(0) { + frame_counter_(0), + nack_module_(MaybeCreateNackModule(clock, + nack_sender, + keyframe_request_sender)) { for (int i = 0; i < kStartNumberOfFrames; i++) free_frames_.push_back(new VCMFrameBuffer()); } @@ -329,6 +349,8 @@ void VCMJitterBuffer::Stop() { UpdateHistograms(); running_ = false; last_decoded_state_.Reset(); + if (nack_module_) + nack_module_->Stop(); // Make sure all frames are free and reset. for (FrameList::iterator it = decodable_frames_.begin(); @@ -369,6 +391,8 @@ void VCMJitterBuffer::Flush() { waiting_for_completion_.latest_packet_time = -1; first_packet_since_reset_ = true; missing_sequence_numbers_.clear(); + if (nack_module_) + nack_module_->Clear(); } // Get received key and delta frames @@ -605,6 +629,8 @@ VCMEncodedFrame* VCMJitterBuffer::ExtractAndSetDecode(uint32_t timestamp) { if ((*frame).IsSessionComplete()) UpdateAveragePacketsPerFrame(frame->NumPackets()); + if (nack_module_) + nack_module_->ClearUpTo(frame->GetHighSeqNum()); return frame; } @@ -665,6 +691,9 @@ VCMFrameBufferEnum VCMJitterBuffer::InsertPacket(const VCMPacket& packet, bool* retransmitted) { CriticalSectionScoped cs(crit_sect_); + if (nack_module_) + nack_module_->OnReceivedPacket(packet); + ++num_packets_; if (num_packets_ == 1) { time_first_packet_ms_ = clock_->TimeInMilliseconds(); @@ -927,6 +956,8 @@ void VCMJitterBuffer::UpdateRtt(int64_t rtt_ms) { CriticalSectionScoped cs(crit_sect_); rtt_ms_ = rtt_ms; jitter_estimate_.UpdateRtt(rtt_ms); + if (nack_module_) + nack_module_->UpdateRtt(rtt_ms); } void VCMJitterBuffer::SetNackMode(VCMNackMode mode, @@ -1046,6 +1077,14 @@ std::vector VCMJitterBuffer::GetNackList(bool* request_key_frame) { } } } + // The experiment is running, the nack module will send Nacks. + // The reason we allow the major part of the GetNackList function to + // run even if we are running the experiment is because this + // function is also used to signal that the jitter buffer wants to + // request a keyframe. + if (nack_module_) + return std::vector(); + std::vector nack_list(missing_sequence_numbers_.begin(), missing_sequence_numbers_.end()); return nack_list; @@ -1343,4 +1382,15 @@ bool VCMJitterBuffer::WaitForRetransmissions() { } return true; } + +int64_t VCMJitterBuffer::TimeUntilNextProcess() { + if (nack_module_) + return nack_module_->TimeUntilNextProcess(); + return std::numeric_limits::max(); +} + +void VCMJitterBuffer::Process() { + if (nack_module_) + nack_module_->Process(); +} } // namespace webrtc diff --git a/webrtc/modules/video_coding/jitter_buffer.h b/webrtc/modules/video_coding/jitter_buffer.h index 8ba338f0c7..0cc03dd810 100644 --- a/webrtc/modules/video_coding/jitter_buffer.h +++ b/webrtc/modules/video_coding/jitter_buffer.h @@ -20,12 +20,14 @@ #include "webrtc/base/constructormagic.h" #include "webrtc/base/thread_annotations.h" #include "webrtc/modules/include/module_common_types.h" +#include "webrtc/modules/utility/include/process_thread.h" #include "webrtc/modules/video_coding/include/video_coding.h" #include "webrtc/modules/video_coding/include/video_coding_defines.h" #include "webrtc/modules/video_coding/decoding_state.h" #include "webrtc/modules/video_coding/inter_frame_delay.h" #include "webrtc/modules/video_coding/jitter_buffer_common.h" #include "webrtc/modules/video_coding/jitter_estimator.h" +#include "webrtc/modules/video_coding/nack_module.h" #include "webrtc/system_wrappers/include/critical_section_wrapper.h" #include "webrtc/typedefs.h" @@ -104,7 +106,10 @@ class Vp9SsMap { class VCMJitterBuffer { public: - VCMJitterBuffer(Clock* clock, std::unique_ptr event); + VCMJitterBuffer(Clock* clock, + std::unique_ptr event, + NackSender* nack_sender = nullptr, + KeyFrameRequestSender* keyframe_request_sender = nullptr); ~VCMJitterBuffer(); @@ -213,6 +218,9 @@ class VCMJitterBuffer { void RegisterStatsCallback(VCMReceiveStatisticsCallback* callback); + int64_t TimeUntilNextProcess(); + void Process(); + private: class SequenceNumberLessThan { public: @@ -383,6 +391,9 @@ class VCMJitterBuffer { // average_packets_per_frame converges fast if we have fewer than this many // frames. int frame_counter_; + + std::unique_ptr nack_module_; + RTC_DISALLOW_COPY_AND_ASSIGN(VCMJitterBuffer); }; } // namespace webrtc diff --git a/webrtc/modules/video_coding/jitter_buffer_unittest.cc b/webrtc/modules/video_coding/jitter_buffer_unittest.cc index c5384681b3..df70ea9826 100644 --- a/webrtc/modules/video_coding/jitter_buffer_unittest.cc +++ b/webrtc/modules/video_coding/jitter_buffer_unittest.cc @@ -8,12 +8,13 @@ * be found in the AUTHORS file in the root of the source tree. */ -#include +#include #include #include #include "testing/gtest/include/gtest/gtest.h" +#include "testing/gmock/include/gmock/gmock.h" #include "webrtc/modules/video_coding/frame_buffer.h" #include "webrtc/modules/video_coding/jitter_buffer.h" #include "webrtc/modules/video_coding/media_opt_util.h" @@ -21,7 +22,9 @@ #include "webrtc/modules/video_coding/test/stream_generator.h" #include "webrtc/modules/video_coding/test/test_util.h" #include "webrtc/system_wrappers/include/clock.h" +#include "webrtc/system_wrappers/include/field_trial.h" #include "webrtc/system_wrappers/include/metrics.h" +#include "webrtc/test/field_trial.h" #include "webrtc/test/histogram.h" namespace webrtc { @@ -185,13 +188,40 @@ TEST_F(Vp9SsMapTest, UpdatePacket) { EXPECT_EQ(2, packet_.codecSpecificHeader.codecHeader.VP9.pid_diff[1]); } -class TestBasicJitterBuffer : public ::testing::Test { +class ProcessThreadMock : public ProcessThread { + public: + MOCK_METHOD0(Start, void()); + MOCK_METHOD0(Stop, void()); + MOCK_METHOD1(WakeUp, void(Module* module)); + MOCK_METHOD1(RegisterModule, void(Module* module)); + MOCK_METHOD1(DeRegisterModule, void(Module* module)); + void PostTask(rtc::scoped_ptr task) {} +}; + +class TestBasicJitterBuffer : public ::testing::TestWithParam, + public NackSender, + public KeyFrameRequestSender { + public: + void SendNack(const std::vector& sequence_numbers) override { + nack_sent_.insert(nack_sent_.end(), sequence_numbers.begin(), + sequence_numbers.end()); + } + + void RequestKeyFrame() override { ++keyframe_requests_; } + + ::testing::NiceMock process_thread_mock_; + std::vector nack_sent_; + int keyframe_requests_; + protected: + TestBasicJitterBuffer() : scoped_field_trial_(GetParam()) {} virtual void SetUp() { clock_.reset(new SimulatedClock(0)); jitter_buffer_.reset(new VCMJitterBuffer( clock_.get(), - std::unique_ptr(event_factory_.CreateEvent()))); + std::unique_ptr(event_factory_.CreateEvent()), + this, + this)); jitter_buffer_->Start(); seq_num_ = 1234; timestamp_ = 0; @@ -278,9 +308,30 @@ class TestBasicJitterBuffer : public ::testing::Test { std::unique_ptr clock_; NullEventFactory event_factory_; std::unique_ptr jitter_buffer_; + test::ScopedFieldTrials scoped_field_trial_; }; -class TestRunningJitterBuffer : public ::testing::Test { +INSTANTIATE_TEST_CASE_P( + TestWithNackModule, + TestBasicJitterBuffer, + ::testing::Values("WebRTC-NewVideoJitterBuffer/Enabled/", + "WebRTC-NewVideoJitterBuffer/Disabled/")); + +class TestRunningJitterBuffer : public ::testing::TestWithParam, + public NackSender, + public KeyFrameRequestSender { + public: + void SendNack(const std::vector& sequence_numbers) { + nack_sent_.insert(nack_sent_.end(), sequence_numbers.begin(), + sequence_numbers.end()); + } + + void RequestKeyFrame() { ++keyframe_requests_; } + + ::testing::NiceMock process_thread_mock_; + std::vector nack_sent_; + int keyframe_requests_; + protected: enum { kDataBufferSize = 10 }; @@ -290,7 +341,8 @@ class TestRunningJitterBuffer : public ::testing::Test { oldest_packet_to_nack_ = 250; jitter_buffer_ = new VCMJitterBuffer( clock_.get(), - std::unique_ptr(event_factory_.CreateEvent())); + std::unique_ptr(event_factory_.CreateEvent()), + this, this); stream_generator_ = new StreamGenerator(0, clock_->TimeInMilliseconds()); jitter_buffer_->Start(); jitter_buffer_->SetNackSettings(max_nack_list_size_, oldest_packet_to_nack_, @@ -390,15 +442,24 @@ class TestRunningJitterBuffer : public ::testing::Test { class TestJitterBufferNack : public TestRunningJitterBuffer { protected: + TestJitterBufferNack() : scoped_field_trial_(GetParam()) {} virtual void SetUp() { TestRunningJitterBuffer::SetUp(); jitter_buffer_->SetNackMode(kNack, -1, -1); } virtual void TearDown() { TestRunningJitterBuffer::TearDown(); } + + test::ScopedFieldTrials scoped_field_trial_; }; -TEST_F(TestBasicJitterBuffer, StopRunning) { +INSTANTIATE_TEST_CASE_P( + TestWithNackModule, + TestJitterBufferNack, + ::testing::Values("WebRTC-NewVideoJitterBuffer/Enabled/", + "WebRTC-NewVideoJitterBuffer/Disabled/")); + +TEST_P(TestBasicJitterBuffer, StopRunning) { jitter_buffer_->Stop(); EXPECT_TRUE(NULL == DecodeCompleteFrame()); EXPECT_TRUE(NULL == DecodeIncompleteFrame()); @@ -418,7 +479,7 @@ TEST_F(TestBasicJitterBuffer, StopRunning) { EXPECT_TRUE(NULL == DecodeIncompleteFrame()); } -TEST_F(TestBasicJitterBuffer, SinglePacketFrame) { +TEST_P(TestBasicJitterBuffer, SinglePacketFrame) { // Always start with a complete key frame when not allowing errors. jitter_buffer_->SetDecodeErrorMode(kNoErrors); packet_->frameType = kVideoFrameKey; @@ -436,7 +497,7 @@ TEST_F(TestBasicJitterBuffer, SinglePacketFrame) { jitter_buffer_->ReleaseFrame(frame_out); } -TEST_F(TestBasicJitterBuffer, VerifyHistogramStats) { +TEST_P(TestBasicJitterBuffer, VerifyHistogramStats) { test::ClearHistograms(); // Always start with a complete key frame when not allowing errors. jitter_buffer_->SetDecodeErrorMode(kNoErrors); @@ -478,7 +539,7 @@ TEST_F(TestBasicJitterBuffer, VerifyHistogramStats) { 1, test::NumHistogramSamples("WebRTC.Video.KeyFramesReceivedInPermille")); } -TEST_F(TestBasicJitterBuffer, DualPacketFrame) { +TEST_P(TestBasicJitterBuffer, DualPacketFrame) { packet_->frameType = kVideoFrameKey; packet_->isFirstPacket = true; packet_->markerBit = false; @@ -505,7 +566,7 @@ TEST_F(TestBasicJitterBuffer, DualPacketFrame) { jitter_buffer_->ReleaseFrame(frame_out); } -TEST_F(TestBasicJitterBuffer, 100PacketKeyFrame) { +TEST_P(TestBasicJitterBuffer, 100PacketKeyFrame) { packet_->frameType = kVideoFrameKey; packet_->isFirstPacket = true; packet_->markerBit = false; @@ -548,7 +609,7 @@ TEST_F(TestBasicJitterBuffer, 100PacketKeyFrame) { jitter_buffer_->ReleaseFrame(frame_out); } -TEST_F(TestBasicJitterBuffer, 100PacketDeltaFrame) { +TEST_P(TestBasicJitterBuffer, 100PacketDeltaFrame) { // Always start with a complete key frame. packet_->frameType = kVideoFrameKey; packet_->isFirstPacket = true; @@ -604,7 +665,7 @@ TEST_F(TestBasicJitterBuffer, 100PacketDeltaFrame) { jitter_buffer_->ReleaseFrame(frame_out); } -TEST_F(TestBasicJitterBuffer, PacketReorderingReverseOrder) { +TEST_P(TestBasicJitterBuffer, PacketReorderingReverseOrder) { // Insert the "first" packet last. seq_num_ += 100; packet_->frameType = kVideoFrameKey; @@ -651,7 +712,7 @@ TEST_F(TestBasicJitterBuffer, PacketReorderingReverseOrder) { jitter_buffer_->ReleaseFrame(frame_out); } -TEST_F(TestBasicJitterBuffer, FrameReordering2Frames2PacketsEach) { +TEST_P(TestBasicJitterBuffer, FrameReordering2Frames2PacketsEach) { packet_->frameType = kVideoFrameDelta; packet_->isFirstPacket = true; packet_->markerBit = false; @@ -711,7 +772,7 @@ TEST_F(TestBasicJitterBuffer, FrameReordering2Frames2PacketsEach) { jitter_buffer_->ReleaseFrame(frame_out); } -TEST_F(TestBasicJitterBuffer, TestReorderingWithPadding) { +TEST_P(TestBasicJitterBuffer, TestReorderingWithPadding) { packet_->frameType = kVideoFrameKey; packet_->isFirstPacket = true; packet_->markerBit = true; @@ -768,7 +829,7 @@ TEST_F(TestBasicJitterBuffer, TestReorderingWithPadding) { jitter_buffer_->ReleaseFrame(frame_out); } -TEST_F(TestBasicJitterBuffer, DuplicatePackets) { +TEST_P(TestBasicJitterBuffer, DuplicatePackets) { packet_->frameType = kVideoFrameKey; packet_->isFirstPacket = true; packet_->markerBit = false; @@ -811,7 +872,7 @@ TEST_F(TestBasicJitterBuffer, DuplicatePackets) { jitter_buffer_->ReleaseFrame(frame_out); } -TEST_F(TestBasicJitterBuffer, DuplicatePreviousDeltaFramePacket) { +TEST_P(TestBasicJitterBuffer, DuplicatePreviousDeltaFramePacket) { packet_->frameType = kVideoFrameKey; packet_->isFirstPacket = true; packet_->markerBit = true; @@ -863,7 +924,7 @@ TEST_F(TestBasicJitterBuffer, DuplicatePreviousDeltaFramePacket) { } } -TEST_F(TestBasicJitterBuffer, TestSkipForwardVp9) { +TEST_P(TestBasicJitterBuffer, TestSkipForwardVp9) { // Verify that JB skips forward to next base layer frame. // ------------------------------------------------- // | 65485 | 65486 | 65487 | 65488 | 65489 | ... @@ -916,7 +977,7 @@ TEST_F(TestBasicJitterBuffer, TestSkipForwardVp9) { jitter_buffer_->ReleaseFrame(frame_out); } -TEST_F(TestBasicJitterBuffer, ReorderedVp9SsData_3TlLayers) { +TEST_P(TestBasicJitterBuffer, ReorderedVp9SsData_3TlLayers) { // Verify that frames are updated with SS data when SS packet is reordered. // -------------------------------- // | 65486 | 65487 | 65485 |... @@ -990,7 +1051,7 @@ TEST_F(TestBasicJitterBuffer, ReorderedVp9SsData_3TlLayers) { jitter_buffer_->ReleaseFrame(frame_out); } -TEST_F(TestBasicJitterBuffer, ReorderedVp9SsData_2Tl2SLayers) { +TEST_P(TestBasicJitterBuffer, ReorderedVp9SsData_2Tl2SLayers) { // Verify that frames are updated with SS data when SS packet is reordered. // ----------------------------------------- // | 65486 | 65487 | 65485 | 65484 |... @@ -1074,7 +1135,7 @@ TEST_F(TestBasicJitterBuffer, ReorderedVp9SsData_2Tl2SLayers) { jitter_buffer_->ReleaseFrame(frame_out); } -TEST_F(TestBasicJitterBuffer, H264InsertStartCode) { +TEST_P(TestBasicJitterBuffer, H264InsertStartCode) { packet_->frameType = kVideoFrameKey; packet_->isFirstPacket = true; packet_->markerBit = false; @@ -1106,7 +1167,7 @@ TEST_F(TestBasicJitterBuffer, H264InsertStartCode) { } // Test threshold conditions of decodable state. -TEST_F(TestBasicJitterBuffer, PacketLossWithSelectiveErrorsThresholdCheck) { +TEST_P(TestBasicJitterBuffer, PacketLossWithSelectiveErrorsThresholdCheck) { jitter_buffer_->SetDecodeErrorMode(kSelectiveErrors); // Always start with a key frame. Use 10 packets to test Decodable State // boundaries. @@ -1192,7 +1253,7 @@ TEST_F(TestBasicJitterBuffer, PacketLossWithSelectiveErrorsThresholdCheck) { } // Make sure first packet is present before a frame can be decoded. -TEST_F(TestBasicJitterBuffer, PacketLossWithSelectiveErrorsIncompleteKey) { +TEST_P(TestBasicJitterBuffer, PacketLossWithSelectiveErrorsIncompleteKey) { jitter_buffer_->SetDecodeErrorMode(kSelectiveErrors); // Always start with a key frame. packet_->frameType = kVideoFrameKey; @@ -1256,7 +1317,7 @@ TEST_F(TestBasicJitterBuffer, PacketLossWithSelectiveErrorsIncompleteKey) { } // Make sure first packet is present before a frame can be decoded. -TEST_F(TestBasicJitterBuffer, PacketLossWithSelectiveErrorsMissingFirstPacket) { +TEST_P(TestBasicJitterBuffer, PacketLossWithSelectiveErrorsMissingFirstPacket) { jitter_buffer_->SetDecodeErrorMode(kSelectiveErrors); // Always start with a key frame. packet_->frameType = kVideoFrameKey; @@ -1318,7 +1379,7 @@ TEST_F(TestBasicJitterBuffer, PacketLossWithSelectiveErrorsMissingFirstPacket) { jitter_buffer_->ReleaseFrame(frame_out); } -TEST_F(TestBasicJitterBuffer, DiscontinuousStreamWhenDecodingWithErrors) { +TEST_P(TestBasicJitterBuffer, DiscontinuousStreamWhenDecodingWithErrors) { // Will use one packet per frame. jitter_buffer_->SetDecodeErrorMode(kWithErrors); packet_->frameType = kVideoFrameKey; @@ -1361,7 +1422,7 @@ TEST_F(TestBasicJitterBuffer, DiscontinuousStreamWhenDecodingWithErrors) { EXPECT_EQ(packet_->timestamp - 33 * 90, next_timestamp); } -TEST_F(TestBasicJitterBuffer, PacketLoss) { +TEST_P(TestBasicJitterBuffer, PacketLoss) { // Verify missing packets statistics and not decodable packets statistics. // Insert 10 frames consisting of 4 packets and remove one from all of them. // The last packet is an empty (non-media) packet. @@ -1462,7 +1523,7 @@ TEST_F(TestBasicJitterBuffer, PacketLoss) { EXPECT_EQ(3, jitter_buffer_->num_discarded_packets()); } -TEST_F(TestBasicJitterBuffer, DeltaFrame100PacketsWithSeqNumWrap) { +TEST_P(TestBasicJitterBuffer, DeltaFrame100PacketsWithSeqNumWrap) { seq_num_ = 0xfff0; packet_->frameType = kVideoFrameKey; packet_->isFirstPacket = true; @@ -1511,7 +1572,7 @@ TEST_F(TestBasicJitterBuffer, DeltaFrame100PacketsWithSeqNumWrap) { jitter_buffer_->ReleaseFrame(frame_out); } -TEST_F(TestBasicJitterBuffer, PacketReorderingReverseWithNegSeqNumWrap) { +TEST_P(TestBasicJitterBuffer, PacketReorderingReverseWithNegSeqNumWrap) { // Insert "first" packet last seqnum. seq_num_ = 10; packet_->frameType = kVideoFrameKey; @@ -1560,7 +1621,7 @@ TEST_F(TestBasicJitterBuffer, PacketReorderingReverseWithNegSeqNumWrap) { jitter_buffer_->ReleaseFrame(frame_out); } -TEST_F(TestBasicJitterBuffer, TestInsertOldFrame) { +TEST_P(TestBasicJitterBuffer, TestInsertOldFrame) { // ------- ------- // | 2 | | 1 | // ------- ------- @@ -1594,7 +1655,7 @@ TEST_F(TestBasicJitterBuffer, TestInsertOldFrame) { EXPECT_EQ(kOldPacket, jitter_buffer_->InsertPacket(*packet_, &retransmitted)); } -TEST_F(TestBasicJitterBuffer, TestInsertOldFrameWithSeqNumWrap) { +TEST_P(TestBasicJitterBuffer, TestInsertOldFrameWithSeqNumWrap) { // ------- ------- // | 2 | | 1 | // ------- ------- @@ -1633,7 +1694,7 @@ TEST_F(TestBasicJitterBuffer, TestInsertOldFrameWithSeqNumWrap) { EXPECT_EQ(kOldPacket, jitter_buffer_->InsertPacket(*packet_, &retransmitted)); } -TEST_F(TestBasicJitterBuffer, TimestampWrap) { +TEST_P(TestBasicJitterBuffer, TimestampWrap) { // --------------- --------------- // | 1 | 2 | | 3 | 4 | // --------------- --------------- @@ -1693,7 +1754,7 @@ TEST_F(TestBasicJitterBuffer, TimestampWrap) { jitter_buffer_->ReleaseFrame(frame_out); } -TEST_F(TestBasicJitterBuffer, 2FrameWithTimestampWrap) { +TEST_P(TestBasicJitterBuffer, 2FrameWithTimestampWrap) { // ------- ------- // | 1 | | 2 | // ------- ------- @@ -1735,7 +1796,7 @@ TEST_F(TestBasicJitterBuffer, 2FrameWithTimestampWrap) { jitter_buffer_->ReleaseFrame(frame_out2); } -TEST_F(TestBasicJitterBuffer, Insert2FramesReOrderedWithTimestampWrap) { +TEST_P(TestBasicJitterBuffer, Insert2FramesReOrderedWithTimestampWrap) { // ------- ------- // | 2 | | 1 | // ------- ------- @@ -1778,7 +1839,7 @@ TEST_F(TestBasicJitterBuffer, Insert2FramesReOrderedWithTimestampWrap) { jitter_buffer_->ReleaseFrame(frame_out2); } -TEST_F(TestBasicJitterBuffer, DeltaFrameWithMoreThanMaxNumberOfPackets) { +TEST_P(TestBasicJitterBuffer, DeltaFrameWithMoreThanMaxNumberOfPackets) { int loop = 0; bool firstPacket = true; bool retransmitted = false; @@ -1813,7 +1874,7 @@ TEST_F(TestBasicJitterBuffer, DeltaFrameWithMoreThanMaxNumberOfPackets) { EXPECT_TRUE(NULL == DecodeCompleteFrame()); } -TEST_F(TestBasicJitterBuffer, ExceedNumOfFrameWithSeqNumWrap) { +TEST_P(TestBasicJitterBuffer, ExceedNumOfFrameWithSeqNumWrap) { // TEST fill JB with more than max number of frame (50 delta frames + // 51 key frames) with wrap in seq_num_ // @@ -1873,7 +1934,7 @@ TEST_F(TestBasicJitterBuffer, ExceedNumOfFrameWithSeqNumWrap) { jitter_buffer_->ReleaseFrame(frame_out); } -TEST_F(TestBasicJitterBuffer, EmptyLastFrame) { +TEST_P(TestBasicJitterBuffer, EmptyLastFrame) { jitter_buffer_->SetDecodeErrorMode(kWithErrors); seq_num_ = 3; // Insert one empty packet per frame, should never return the last timestamp @@ -1899,7 +1960,7 @@ TEST_F(TestBasicJitterBuffer, EmptyLastFrame) { } } -TEST_F(TestBasicJitterBuffer, H264IncompleteNalu) { +TEST_P(TestBasicJitterBuffer, H264IncompleteNalu) { jitter_buffer_->SetNackMode(kNoNack, -1, -1); jitter_buffer_->SetDecodeErrorMode(kWithErrors); ++seq_num_; @@ -2080,7 +2141,7 @@ TEST_F(TestBasicJitterBuffer, H264IncompleteNalu) { jitter_buffer_->ReleaseFrame(frame_out); } -TEST_F(TestBasicJitterBuffer, NextFrameWhenIncomplete) { +TEST_P(TestBasicJitterBuffer, NextFrameWhenIncomplete) { // Test that a we cannot get incomplete frames from the JB if we haven't // received the marker bit, unless we have received a packet from a later // timestamp. @@ -2257,7 +2318,7 @@ TEST_F(TestRunningJitterBuffer, TwoPacketsNonContinuous) { EXPECT_TRUE(DecodeCompleteFrame()); } -TEST_F(TestJitterBufferNack, EmptyPackets) { +TEST_P(TestJitterBufferNack, EmptyPackets) { // Make sure empty packets doesn't clog the jitter buffer. jitter_buffer_->SetNackMode(kNack, media_optimization::kLowRttNackMs, -1); EXPECT_GE(InsertFrames(kMaxNumberOfFrames, kEmptyFrame), kNoError); @@ -2265,7 +2326,7 @@ TEST_F(TestJitterBufferNack, EmptyPackets) { EXPECT_TRUE(DecodeCompleteFrame()); } -TEST_F(TestJitterBufferNack, NackTooOldPackets) { +TEST_P(TestJitterBufferNack, NackTooOldPackets) { // Insert a key frame and decode it. EXPECT_GE(InsertFrame(kVideoFrameKey), kNoError); EXPECT_TRUE(DecodeCompleteFrame()); @@ -2298,7 +2359,7 @@ TEST_F(TestJitterBufferNack, NackTooOldPackets) { EXPECT_TRUE(DecodeCompleteFrame()); } -TEST_F(TestJitterBufferNack, NackLargeJitterBuffer) { +TEST_P(TestJitterBufferNack, NackLargeJitterBuffer) { // Insert a key frame and decode it. EXPECT_GE(InsertFrame(kVideoFrameKey), kNoError); EXPECT_TRUE(DecodeCompleteFrame()); @@ -2317,7 +2378,7 @@ TEST_F(TestJitterBufferNack, NackLargeJitterBuffer) { EXPECT_TRUE(DecodeCompleteFrame()); } -TEST_F(TestJitterBufferNack, NackListFull) { +TEST_P(TestJitterBufferNack, NackListFull) { // Insert a key frame and decode it. EXPECT_GE(InsertFrame(kVideoFrameKey), kNoError); EXPECT_TRUE(DecodeCompleteFrame()); @@ -2350,7 +2411,7 @@ TEST_F(TestJitterBufferNack, NackListFull) { EXPECT_TRUE(DecodeCompleteFrame()); } -TEST_F(TestJitterBufferNack, NoNackListReturnedBeforeFirstDecode) { +TEST_P(TestJitterBufferNack, NoNackListReturnedBeforeFirstDecode) { DropFrame(10); // Insert a frame and try to generate a NACK list. Shouldn't get one. EXPECT_GE(InsertFrame(kVideoFrameDelta), kNoError); @@ -2362,7 +2423,7 @@ TEST_F(TestJitterBufferNack, NoNackListReturnedBeforeFirstDecode) { EXPECT_TRUE(request_key_frame); } -TEST_F(TestJitterBufferNack, NackListBuiltBeforeFirstDecode) { +TEST_P(TestJitterBufferNack, NackListBuiltBeforeFirstDecode) { stream_generator_->Init(0, clock_->TimeInMilliseconds()); InsertFrame(kVideoFrameKey); stream_generator_->GenerateFrame(kVideoFrameDelta, 2, 0, @@ -2372,10 +2433,13 @@ TEST_F(TestJitterBufferNack, NackListBuiltBeforeFirstDecode) { EXPECT_TRUE(DecodeCompleteFrame()); bool extended = false; std::vector nack_list = jitter_buffer_->GetNackList(&extended); - EXPECT_EQ(1u, nack_list.size()); + if (field_trial::FindFullName("WebRTC-NewVideoJitterBuffer") == "Enabled") + EXPECT_EQ(1u, nack_sent_.size()); + else + EXPECT_EQ(1u, nack_list.size()); } -TEST_F(TestJitterBufferNack, VerifyRetransmittedFlag) { +TEST_P(TestJitterBufferNack, VerifyRetransmittedFlag) { stream_generator_->Init(0, clock_->TimeInMilliseconds()); stream_generator_->GenerateFrame(kVideoFrameKey, 3, 0, clock_->TimeInMilliseconds()); @@ -2391,16 +2455,23 @@ TEST_F(TestJitterBufferNack, VerifyRetransmittedFlag) { EXPECT_FALSE(DecodeCompleteFrame()); bool extended = false; std::vector nack_list = jitter_buffer_->GetNackList(&extended); - EXPECT_EQ(1u, nack_list.size()); + uint16_t seq_num; + if (field_trial::FindFullName("WebRTC-NewVideoJitterBuffer") == "Enabled") { + EXPECT_EQ(1u, nack_sent_.size()); + seq_num = nack_sent_[0]; + } else { + EXPECT_EQ(1u, nack_list.size()); + seq_num = nack_list[0]; + } stream_generator_->PopPacket(&packet, 0); - EXPECT_EQ(packet.seqNum, nack_list[0]); + EXPECT_EQ(packet.seqNum, seq_num); EXPECT_EQ(kCompleteSession, jitter_buffer_->InsertPacket(packet, &retransmitted)); EXPECT_TRUE(retransmitted); EXPECT_TRUE(DecodeCompleteFrame()); } -TEST_F(TestJitterBufferNack, UseNackToRecoverFirstKeyFrame) { +TEST_P(TestJitterBufferNack, UseNackToRecoverFirstKeyFrame) { stream_generator_->Init(0, clock_->TimeInMilliseconds()); stream_generator_->GenerateFrame(kVideoFrameKey, 3, 0, clock_->TimeInMilliseconds()); @@ -2410,13 +2481,20 @@ TEST_F(TestJitterBufferNack, UseNackToRecoverFirstKeyFrame) { EXPECT_FALSE(DecodeCompleteFrame()); bool extended = false; std::vector nack_list = jitter_buffer_->GetNackList(&extended); - EXPECT_EQ(1u, nack_list.size()); + uint16_t seq_num; + if (field_trial::FindFullName("WebRTC-NewVideoJitterBuffer") == "Enabled") { + ASSERT_EQ(1u, nack_sent_.size()); + seq_num = nack_sent_[0]; + } else { + ASSERT_EQ(1u, nack_list.size()); + seq_num = nack_list[0]; + } VCMPacket packet; stream_generator_->GetPacket(&packet, 0); - EXPECT_EQ(packet.seqNum, nack_list[0]); + EXPECT_EQ(packet.seqNum, seq_num); } -TEST_F(TestJitterBufferNack, UseNackToRecoverFirstKeyFrameSecondInQueue) { +TEST_P(TestJitterBufferNack, UseNackToRecoverFirstKeyFrameSecondInQueue) { VCMPacket packet; stream_generator_->Init(0, clock_->TimeInMilliseconds()); // First frame is delta. @@ -2435,12 +2513,19 @@ TEST_F(TestJitterBufferNack, UseNackToRecoverFirstKeyFrameSecondInQueue) { EXPECT_FALSE(DecodeCompleteFrame()); bool extended = false; std::vector nack_list = jitter_buffer_->GetNackList(&extended); - EXPECT_EQ(1u, nack_list.size()); + uint16_t seq_num; + if (field_trial::FindFullName("WebRTC-NewVideoJitterBuffer") == "Enabled") { + ASSERT_EQ(2u, nack_sent_.size()); + seq_num = nack_sent_[1]; + } else { + ASSERT_EQ(1u, nack_list.size()); + seq_num = nack_list[0]; + } stream_generator_->GetPacket(&packet, 0); - EXPECT_EQ(packet.seqNum, nack_list[0]); + EXPECT_EQ(packet.seqNum, seq_num); } -TEST_F(TestJitterBufferNack, NormalOperation) { +TEST_P(TestJitterBufferNack, NormalOperation) { EXPECT_EQ(kNack, jitter_buffer_->nack_mode()); jitter_buffer_->SetDecodeErrorMode(kWithErrors); @@ -2468,16 +2553,20 @@ TEST_F(TestJitterBufferNack, NormalOperation) { EXPECT_FALSE(DecodeCompleteFrame()); EXPECT_FALSE(DecodeIncompleteFrame()); bool request_key_frame = false; + + // Verify the NACK list. std::vector nack_list = jitter_buffer_->GetNackList(&request_key_frame); - // Verify the NACK list. const size_t kExpectedNackSize = 9; - ASSERT_EQ(kExpectedNackSize, nack_list.size()); + if (field_trial::FindFullName("WebRTC-NewVideoJitterBuffer") == "Enabled") + ASSERT_EQ(kExpectedNackSize, nack_sent_.size()); + else + ASSERT_EQ(kExpectedNackSize, nack_list.size()); for (size_t i = 0; i < nack_list.size(); ++i) EXPECT_EQ((1 + i) * 10, nack_list[i]); } -TEST_F(TestJitterBufferNack, NormalOperationWrap) { +TEST_P(TestJitterBufferNack, NormalOperationWrap) { bool request_key_frame = false; // ------- ------------------------------------------------------------ // | 65532 | | 65533 | 65534 | 65535 | x | 1 | .. | 9 | x | 11 |.....| 96 | @@ -2506,12 +2595,18 @@ TEST_F(TestJitterBufferNack, NormalOperationWrap) { std::vector nack_list = jitter_buffer_->GetNackList(&extended); // Verify the NACK list. const size_t kExpectedNackSize = 10; - ASSERT_EQ(kExpectedNackSize, nack_list.size()); - for (size_t i = 0; i < nack_list.size(); ++i) - EXPECT_EQ(i * 10, nack_list[i]); + if (field_trial::FindFullName("WebRTC-NewVideoJitterBuffer") == "Enabled") { + ASSERT_EQ(kExpectedNackSize, nack_sent_.size()); + for (size_t i = 0; i < nack_sent_.size(); ++i) + EXPECT_EQ(i * 10, nack_sent_[i]); + } else { + ASSERT_EQ(kExpectedNackSize, nack_list.size()); + for (size_t i = 0; i < nack_list.size(); ++i) + EXPECT_EQ(i * 10, nack_list[i]); + } } -TEST_F(TestJitterBufferNack, NormalOperationWrap2) { +TEST_P(TestJitterBufferNack, NormalOperationWrap2) { bool request_key_frame = false; // ----------------------------------- // | 65532 | 65533 | 65534 | x | 0 | 1 | @@ -2539,11 +2634,16 @@ TEST_F(TestJitterBufferNack, NormalOperationWrap2) { bool extended = false; std::vector nack_list = jitter_buffer_->GetNackList(&extended); // Verify the NACK list. - ASSERT_EQ(1u, nack_list.size()); - EXPECT_EQ(65535, nack_list[0]); + if (field_trial::FindFullName("WebRTC-NewVideoJitterBuffer") == "Enabled") { + ASSERT_EQ(1u, nack_sent_.size()); + EXPECT_EQ(65535, nack_sent_[0]); + } else { + ASSERT_EQ(1u, nack_list.size()); + EXPECT_EQ(65535, nack_list[0]); + } } -TEST_F(TestJitterBufferNack, ResetByFutureKeyFrameDoesntError) { +TEST_P(TestJitterBufferNack, ResetByFutureKeyFrameDoesntError) { stream_generator_->Init(0, clock_->TimeInMilliseconds()); InsertFrame(kVideoFrameKey); EXPECT_TRUE(DecodeCompleteFrame()); diff --git a/webrtc/modules/video_coding/nack_module.cc b/webrtc/modules/video_coding/nack_module.cc index 319a902ca2..80f84968eb 100644 --- a/webrtc/modules/video_coding/nack_module.cc +++ b/webrtc/modules/video_coding/nack_module.cc @@ -118,6 +118,12 @@ void NackModule::UpdateRtt(int64_t rtt_ms) { rtt_ms_ = rtt_ms; } +void NackModule::Clear() { + rtc::CritScope lock(&crit_); + nack_list_.clear(); + keyframe_list_.clear(); +} + void NackModule::Stop() { rtc::CritScope lock(&crit_); running_ = false; diff --git a/webrtc/modules/video_coding/nack_module.h b/webrtc/modules/video_coding/nack_module.h index 32692bef69..8e0a79b2cb 100644 --- a/webrtc/modules/video_coding/nack_module.h +++ b/webrtc/modules/video_coding/nack_module.h @@ -35,6 +35,7 @@ class NackModule : public Module { void OnReceivedPacket(const VCMPacket& packet); void ClearUpTo(uint16_t seq_num); void UpdateRtt(int64_t rtt_ms); + void Clear(); void Stop(); // Module implementation diff --git a/webrtc/modules/video_coding/receiver.cc b/webrtc/modules/video_coding/receiver.cc index 988f1ef5c7..a02fd01de6 100644 --- a/webrtc/modules/video_coding/receiver.cc +++ b/webrtc/modules/video_coding/receiver.cc @@ -30,19 +30,49 @@ enum { kMaxReceiverDelayMs = 10000 }; VCMReceiver::VCMReceiver(VCMTiming* timing, Clock* clock, EventFactory* event_factory) + : VCMReceiver::VCMReceiver(timing, + clock, + event_factory, + nullptr, // NackSender + nullptr) // KeyframeRequestSender +{} + +VCMReceiver::VCMReceiver(VCMTiming* timing, + Clock* clock, + EventFactory* event_factory, + NackSender* nack_sender, + KeyFrameRequestSender* keyframe_request_sender) : VCMReceiver(timing, clock, std::unique_ptr(event_factory->CreateEvent()), - std::unique_ptr(event_factory->CreateEvent())) { -} + std::unique_ptr(event_factory->CreateEvent()), + nack_sender, + keyframe_request_sender) {} VCMReceiver::VCMReceiver(VCMTiming* timing, Clock* clock, std::unique_ptr receiver_event, std::unique_ptr jitter_buffer_event) + : VCMReceiver::VCMReceiver(timing, + clock, + std::move(receiver_event), + std::move(jitter_buffer_event), + nullptr, // NackSender + nullptr) // KeyframeRequestSender +{} + +VCMReceiver::VCMReceiver(VCMTiming* timing, + Clock* clock, + std::unique_ptr receiver_event, + std::unique_ptr jitter_buffer_event, + NackSender* nack_sender, + KeyFrameRequestSender* keyframe_request_sender) : crit_sect_(CriticalSectionWrapper::CreateCriticalSection()), clock_(clock), - jitter_buffer_(clock_, std::move(jitter_buffer_event)), + jitter_buffer_(clock_, + std::move(jitter_buffer_event), + nack_sender, + keyframe_request_sender), timing_(timing), render_wait_event_(std::move(receiver_event)), max_video_delay_ms_(kMaxVideoDelayMs) { @@ -67,6 +97,14 @@ void VCMReceiver::UpdateRtt(int64_t rtt) { jitter_buffer_.UpdateRtt(rtt); } +int64_t VCMReceiver::TimeUntilNextProcess() { + return jitter_buffer_.TimeUntilNextProcess(); +} + +void VCMReceiver::Process() { + jitter_buffer_.Process(); +} + int32_t VCMReceiver::InsertPacket(const VCMPacket& packet, uint16_t frame_width, uint16_t frame_height) { diff --git a/webrtc/modules/video_coding/receiver.h b/webrtc/modules/video_coding/receiver.h index 7b2149ddce..a4c55e967c 100644 --- a/webrtc/modules/video_coding/receiver.h +++ b/webrtc/modules/video_coding/receiver.h @@ -28,17 +28,37 @@ class VCMEncodedFrame; class VCMReceiver { public: + // Constructor for current interface, will be removed when the + // new jitter buffer is in place. VCMReceiver(VCMTiming* timing, Clock* clock, EventFactory* event_factory); + // Create method for the new jitter buffer. + VCMReceiver(VCMTiming* timing, + Clock* clock, + EventFactory* event_factory, + NackSender* nack_sender, + KeyFrameRequestSender* keyframe_request_sender); + // Using this constructor, you can specify a different event factory for the // jitter buffer. Useful for unit tests when you want to simulate incoming // packets, in which case the jitter buffer's wait event is different from // that of VCMReceiver itself. + // + // Constructor for current interface, will be removed when the + // new jitter buffer is in place. VCMReceiver(VCMTiming* timing, Clock* clock, std::unique_ptr receiver_event, std::unique_ptr jitter_buffer_event); + // Create method for the new jitter buffer. + VCMReceiver(VCMTiming* timing, + Clock* clock, + std::unique_ptr receiver_event, + std::unique_ptr jitter_buffer_event, + NackSender* nack_sender, + KeyFrameRequestSender* keyframe_request_sender); + ~VCMReceiver(); void Reset(); @@ -79,6 +99,9 @@ class VCMReceiver { void TriggerDecoderShutdown(); + int64_t TimeUntilNextProcess(); + void Process(); + private: CriticalSectionWrapper* crit_sect_; Clock* const clock_; diff --git a/webrtc/modules/video_coding/video_coding_impl.cc b/webrtc/modules/video_coding/video_coding_impl.cc index 01a7a42ccf..e5f0ee1222 100644 --- a/webrtc/modules/video_coding/video_coding_impl.cc +++ b/webrtc/modules/video_coding/video_coding_impl.cc @@ -75,13 +75,18 @@ class VideoCodingModuleImpl : public VideoCodingModule { EventFactory* event_factory, bool owns_event_factory, VideoEncoderRateObserver* encoder_rate_observer, - VCMQMSettingsCallback* qm_settings_callback) + VCMQMSettingsCallback* qm_settings_callback, + NackSender* nack_sender, + KeyFrameRequestSender* keyframe_request_sender) : VideoCodingModule(), sender_(clock, &post_encode_callback_, encoder_rate_observer, qm_settings_callback), - receiver_(clock, event_factory), + receiver_(clock, + event_factory, + nack_sender, + keyframe_request_sender), own_event_factory_(owns_event_factory ? event_factory : NULL) {} virtual ~VideoCodingModuleImpl() { own_event_factory_.reset(); } @@ -291,20 +296,51 @@ void VideoCodingModule::Codec(VideoCodecType codecType, VideoCodec* codec) { VCMCodecDataBase::Codec(codecType, codec); } +// Create method for current interface, will be removed when the +// new jitter buffer is in place. VideoCodingModule* VideoCodingModule::Create( Clock* clock, VideoEncoderRateObserver* encoder_rate_observer, VCMQMSettingsCallback* qm_settings_callback) { - return new VideoCodingModuleImpl(clock, new EventFactoryImpl, true, - encoder_rate_observer, qm_settings_callback); + return VideoCodingModule::Create(clock, encoder_rate_observer, + qm_settings_callback, + nullptr, // NackSender + nullptr); // KeyframeRequestSender } +// Create method for the new jitter buffer. +VideoCodingModule* VideoCodingModule::Create( + Clock* clock, + VideoEncoderRateObserver* encoder_rate_observer, + VCMQMSettingsCallback* qm_settings_callback, + NackSender* nack_sender, + KeyFrameRequestSender* keyframe_request_sender) { + return new VideoCodingModuleImpl(clock, new EventFactoryImpl, true, + encoder_rate_observer, qm_settings_callback, + nack_sender, + keyframe_request_sender); +} + +// Create method for current interface, will be removed when the +// new jitter buffer is in place. VideoCodingModule* VideoCodingModule::Create(Clock* clock, EventFactory* event_factory) { + return VideoCodingModule::Create(clock, event_factory, + nullptr, // NackSender + nullptr); // KeyframeRequestSender +} + +// Create method for the new jitter buffer. +VideoCodingModule* VideoCodingModule::Create( + Clock* clock, + EventFactory* event_factory, + NackSender* nack_sender, + KeyFrameRequestSender* keyframe_request_sender) { assert(clock); assert(event_factory); return new VideoCodingModuleImpl(clock, event_factory, false, nullptr, - nullptr); + nullptr, nack_sender, + keyframe_request_sender); } } // namespace webrtc diff --git a/webrtc/modules/video_coding/video_coding_impl.h b/webrtc/modules/video_coding/video_coding_impl.h index c01d51716b..f5f9b00206 100644 --- a/webrtc/modules/video_coding/video_coding_impl.h +++ b/webrtc/modules/video_coding/video_coding_impl.h @@ -129,7 +129,10 @@ class VideoReceiver { public: typedef VideoCodingModule::ReceiverRobustness ReceiverRobustness; - VideoReceiver(Clock* clock, EventFactory* event_factory); + VideoReceiver(Clock* clock, + EventFactory* event_factory, + NackSender* nack_sender = nullptr, + KeyFrameRequestSender* keyframe_request_sender = nullptr); ~VideoReceiver(); int32_t RegisterReceiveCodec(const VideoCodec* receiveCodec, diff --git a/webrtc/modules/video_coding/video_receiver.cc b/webrtc/modules/video_coding/video_receiver.cc index e7844d0add..5aadcf91e0 100644 --- a/webrtc/modules/video_coding/video_receiver.cc +++ b/webrtc/modules/video_coding/video_receiver.cc @@ -25,12 +25,19 @@ namespace webrtc { namespace vcm { -VideoReceiver::VideoReceiver(Clock* clock, EventFactory* event_factory) +VideoReceiver::VideoReceiver(Clock* clock, + EventFactory* event_factory, + NackSender* nack_sender, + KeyFrameRequestSender* keyframe_request_sender) : clock_(clock), process_crit_sect_(CriticalSectionWrapper::CreateCriticalSection()), _receiveCritSect(CriticalSectionWrapper::CreateCriticalSection()), _timing(clock_), - _receiver(&_timing, clock_, event_factory), + _receiver(&_timing, + clock_, + event_factory, + nack_sender, + keyframe_request_sender), _decodedFrameCallback(&_timing, clock_), _frameTypeCallback(NULL), _receiveStatsCallback(NULL), @@ -110,6 +117,10 @@ void VideoReceiver::Process() { RequestKeyFrame(); } + if (_receiver.TimeUntilNextProcess() == 0) { + _receiver.Process(); + } + // Packet retransmission requests // TODO(holmer): Add API for changing Process interval and make sure it's // disabled when NACK is off. @@ -150,6 +161,8 @@ int64_t VideoReceiver::TimeUntilNextProcess() { } timeUntilNextProcess = VCM_MIN(timeUntilNextProcess, _keyRequestTimer.TimeUntilProcess()); + timeUntilNextProcess = + VCM_MIN(timeUntilNextProcess, _receiver.TimeUntilNextProcess()); return timeUntilNextProcess; } diff --git a/webrtc/video/video_receive_stream.cc b/webrtc/video/video_receive_stream.cc index 32fb3e5cc8..cd487ac5d3 100644 --- a/webrtc/video/video_receive_stream.cc +++ b/webrtc/video/video_receive_stream.cc @@ -159,7 +159,11 @@ VideoReceiveStream::VideoReceiveStream( congestion_controller_(congestion_controller), call_stats_(call_stats), remb_(remb), - vcm_(VideoCodingModule::Create(clock_, nullptr, nullptr)), + vcm_(VideoCodingModule::Create(clock_, + nullptr, + nullptr, + this, + this)), incoming_video_stream_( 0, config.renderer ? config.renderer->SmoothsRenderedFrames() : false), @@ -424,5 +428,14 @@ void VideoReceiveStream::Decode() { vcm_->Decode(kMaxDecodeWaitTimeMs); } +void VideoReceiveStream::SendNack( + const std::vector& sequence_numbers) { + rtp_rtcp_->SendNack(sequence_numbers); +} + +void VideoReceiveStream::RequestKeyFrame() { + rtp_rtcp_->RequestKeyFrame(); +} + } // namespace internal } // namespace webrtc diff --git a/webrtc/video/video_receive_stream.h b/webrtc/video/video_receive_stream.h index f1061dc130..3aca570f99 100644 --- a/webrtc/video/video_receive_stream.h +++ b/webrtc/video/video_receive_stream.h @@ -40,7 +40,9 @@ namespace internal { class VideoReceiveStream : public webrtc::VideoReceiveStream, public I420FrameCallback, public VideoRenderCallback, - public EncodedImageCallback { + public EncodedImageCallback, + public NackSender, + public KeyFrameRequestSender { public: VideoReceiveStream(int num_cpu_cores, CongestionController* congestion_controller, @@ -79,6 +81,12 @@ class VideoReceiveStream : public webrtc::VideoReceiveStream, void SetSyncChannel(VoiceEngine* voice_engine, int audio_channel_id); + // NackSender + void SendNack(const std::vector& sequence_numbers) override; + + // KeyFrameRequestSender + void RequestKeyFrame() override; + private: static bool DecodeThreadFunction(void* ptr); void Decode();