diff --git a/webrtc/modules/video_coding/main/interface/mock/mock_vcm_callbacks.h b/webrtc/modules/video_coding/main/interface/mock/mock_vcm_callbacks.h index 4d6567ea5..c84d5e71c 100644 --- a/webrtc/modules/video_coding/main/interface/mock/mock_vcm_callbacks.h +++ b/webrtc/modules/video_coding/main/interface/mock/mock_vcm_callbacks.h @@ -12,8 +12,7 @@ #define WEBRTC_MODULES_VIDEO_CODING_MAIN_INTERFACE_MOCK_MOCK_VCM_CALLBACKS_H_ #include "gmock/gmock.h" -#include "webrtc/modules/video_coding/main/interface/video_coding_defines.h" -#include "webrtc/typedefs.h" +#include "typedefs.h" namespace webrtc { diff --git a/webrtc/modules/video_coding/main/source/decoding_state.cc b/webrtc/modules/video_coding/main/source/decoding_state.cc index c58ef9288..e347a2c76 100644 --- a/webrtc/modules/video_coding/main/source/decoding_state.cc +++ b/webrtc/modules/video_coding/main/source/decoding_state.cc @@ -94,13 +94,6 @@ void VCMDecodingState::SetStateOneBack(const VCMFrameBuffer* frame) { init_ = false; } -void VCMDecodingState::UpdateEmptyFrame(const VCMFrameBuffer* frame) { - if (ContinuousFrame(frame) && frame->GetState() == kStateEmpty) { - time_stamp_ = frame->TimeStamp(); - sequence_num_ = frame->GetHighSeqNum(); - } -} - void VCMDecodingState::UpdateOldPacket(const VCMPacket* packet) { assert(packet != NULL); if (packet->timestamp == time_stamp_) { diff --git a/webrtc/modules/video_coding/main/source/decoding_state.h b/webrtc/modules/video_coding/main/source/decoding_state.h index cdfcf9fff..136bae6b6 100644 --- a/webrtc/modules/video_coding/main/source/decoding_state.h +++ b/webrtc/modules/video_coding/main/source/decoding_state.h @@ -33,7 +33,6 @@ class VCMDecodingState { void SetState(const VCMFrameBuffer* frame); // Set the decoding state one frame back. void SetStateOneBack(const VCMFrameBuffer* frame); - void UpdateEmptyFrame(const VCMFrameBuffer* frame); // Update the sequence number if the timestamp matches current state and the // sequence number is higher than the current one. This accounts for packets // arriving late. diff --git a/webrtc/modules/video_coding/main/source/generic_decoder.cc b/webrtc/modules/video_coding/main/source/generic_decoder.cc index 3139ba883..edb6a64d8 100644 --- a/webrtc/modules/video_coding/main/source/generic_decoder.cc +++ b/webrtc/modules/video_coding/main/source/generic_decoder.cc @@ -185,8 +185,7 @@ WebRtc_Word32 VCMGenericDecoder::Decode(const VCMEncodedFrame& frame, _callback->Pop(frame.TimeStamp()); } // Update the key frame decoded variable so that we know whether or not we've decoded a key frame since reset. - _keyFrameDecoded = (_keyFrameDecoded || - frame.FrameType() == kVideoFrameKey); + _keyFrameDecoded = (frame.FrameType() == kVideoFrameKey || frame.FrameType() == kVideoFrameGolden); return ret; } diff --git a/webrtc/modules/video_coding/main/source/jitter_buffer.cc b/webrtc/modules/video_coding/main/source/jitter_buffer.cc index c80b84a07..0af298446 100644 --- a/webrtc/modules/video_coding/main/source/jitter_buffer.cc +++ b/webrtc/modules/video_coding/main/source/jitter_buffer.cc @@ -933,7 +933,8 @@ uint16_t* VCMJitterBuffer::CreateNackList(uint16_t* nack_list_size, // We don't need to check if frame is decoding since low_seq_num is based // on the last decoded sequence number. VCMFrameBufferStateEnum state = frame_buffers_[i]->GetState(); - if (kStateFree != state) { + if ((kStateFree != state) && + (kStateEmpty != state)) { // Reaching thus far means we are going to update the NACK list // When in hybrid mode, we use the soft NACKing feature. if (nack_mode_ == kNackHybrid) { @@ -1270,11 +1271,11 @@ FrameList::iterator VCMJitterBuffer::FindOldestCompleteContinuousFrame( void VCMJitterBuffer::CleanUpOldFrames() { while (frame_list_.size() > 0) { VCMFrameBuffer* oldest_frame = frame_list_.front(); - if (oldest_frame->GetState() == kStateEmpty && frame_list_.size() > 1) { - // This frame is empty, mark it as decoded, thereby making it old. - last_decoded_state_.UpdateEmptyFrame(oldest_frame); - } - if (last_decoded_state_.IsOldFrame(oldest_frame)) { + bool next_frame_empty = + (last_decoded_state_.ContinuousFrame(oldest_frame) && + oldest_frame->GetState() == kStateEmpty); + if (last_decoded_state_.IsOldFrame(oldest_frame) || + (next_frame_empty && frame_list_.size() > 1)) { ReleaseFrameIfNotDecoding(frame_list_.front()); frame_list_.erase(frame_list_.begin()); } else { diff --git a/webrtc/modules/video_coding/main/source/session_info.cc b/webrtc/modules/video_coding/main/source/session_info.cc index 4d41f0f23..726140386 100644 --- a/webrtc/modules/video_coding/main/source/session_info.cc +++ b/webrtc/modules/video_coding/main/source/session_info.cc @@ -357,44 +357,38 @@ int VCMSessionInfo::BuildHardNackList(int* seq_num_list, if (NULL == seq_num_list || seq_num_list_length < 1) { return -1; } - if (packets_.empty() && empty_seq_num_low_ == -1) { + if (packets_.empty()) { return 0; } // Find end point (index of entry equals the sequence number of the first // packet). int index = 0; - int low_seq_num = (packets_.empty()) ? empty_seq_num_low_: - packets_.front().seqNum; for (; index < seq_num_list_length; ++index) { - if (seq_num_list[index] == low_seq_num) { + if (seq_num_list[index] == packets_.front().seqNum) { seq_num_list[index] = -1; ++index; break; } } - if (!packets_.empty()) { - // Zero out between the first entry and the end point. - PacketIterator it = packets_.begin(); - PacketIterator prev_it = it; - ++it; - while (it != packets_.end() && index < seq_num_list_length) { - if (!InSequence(it, prev_it)) { - // Found a sequence number gap due to packet loss. - index += PacketsMissing(it, prev_it); - session_nack_ = true; - } - seq_num_list[index] = -1; - ++index; - prev_it = it; - ++it; + // Zero out between the first entry and the end point. + PacketIterator it = packets_.begin(); + PacketIterator prev_it = it; + ++it; + while (it != packets_.end() && index < seq_num_list_length) { + if (!InSequence(it, prev_it)) { + // Found a sequence number gap due to packet loss. + index += PacketsMissing(it, prev_it); + session_nack_ = true; } - if (!packets_.front().isFirstPacket) - session_nack_ = true; + seq_num_list[index] = -1; + ++index; + prev_it = it; + ++it; } - index = ClearOutEmptyPacketSequenceNumbers(seq_num_list, seq_num_list_length, - index); + if (!packets_.front().isFirstPacket) + session_nack_ = true; return 0; } @@ -491,17 +485,6 @@ int VCMSessionInfo::BuildSoftNackList(int* seq_num_list, } } - index = ClearOutEmptyPacketSequenceNumbers(seq_num_list, seq_num_list_length, - index); - - session_nack_ = allow_nack; - return 0; -} - -int VCMSessionInfo::ClearOutEmptyPacketSequenceNumbers( - int* seq_num_list, - int seq_num_list_length, - int index) const { // Empty packets follow the data packets, and therefore have a higher // sequence number. We do not want to NACK empty packets. if ((empty_seq_num_low_ != -1) && (empty_seq_num_high_ != -1) && @@ -514,14 +497,15 @@ int VCMSessionInfo::ClearOutEmptyPacketSequenceNumbers( } // Mark empty packets. - while (seq_num_list[index] >= 0 && - seq_num_list[index] <= empty_seq_num_high_ && + while (seq_num_list[index] <= empty_seq_num_high_ && index < seq_num_list_length) { seq_num_list[index] = -2; ++index; } } - return index; + + session_nack_ = allow_nack; + return 0; } int VCMSessionInfo::PacketsMissing(const PacketIterator& packet_it, diff --git a/webrtc/modules/video_coding/main/source/session_info.h b/webrtc/modules/video_coding/main/source/session_info.h index bf7e543fa..27533cee0 100644 --- a/webrtc/modules/video_coding/main/source/session_info.h +++ b/webrtc/modules/video_coding/main/source/session_info.h @@ -115,14 +115,6 @@ class VCMSessionInfo { // would be sent to the decoder. void UpdateDecodableSession(int rtt_ms); - // Clears the sequence numbers in |seq_num_list| of any empty packets received - // in this session. |index| is an index in the list at which we start looking - // for the sequence numbers. When done this function returns the index of the - // next element in the list. - int ClearOutEmptyPacketSequenceNumbers(int* seq_num_list, - int seq_num_list_length, - int index) const; - // If this session has been NACKed by the jitter buffer. bool session_nack_; bool complete_; diff --git a/webrtc/modules/video_coding/main/source/video_coding_impl_unittest.cc b/webrtc/modules/video_coding/main/source/video_coding_impl_unittest.cc index 4f89d3ee7..27297ac4f 100644 --- a/webrtc/modules/video_coding/main/source/video_coding_impl_unittest.cc +++ b/webrtc/modules/video_coding/main/source/video_coding_impl_unittest.cc @@ -10,11 +10,9 @@ #include -#include "webrtc/modules/video_coding/codecs/interface/mock/mock_video_codec_interface.h" -#include "webrtc/modules/video_coding/main/interface/mock/mock_vcm_callbacks.h" -#include "webrtc/modules/video_coding/main/interface/video_coding.h" -#include "webrtc/system_wrappers/interface/scoped_ptr.h" -#include "webrtc/system_wrappers/interface/tick_util.h" +#include "modules/video_coding/codecs/interface/mock/mock_video_codec_interface.h" +#include "modules/video_coding/main/interface/video_coding.h" +#include "system_wrappers/interface/scoped_ptr.h" #include "gtest/gtest.h" @@ -38,14 +36,9 @@ class TestVideoCodingModule : public ::testing::Test { static const int kUnusedPayloadType = 10; virtual void SetUp() { - TickTime::UseFakeClock(0); vcm_ = VideoCodingModule::Create(0); - EXPECT_EQ(0, vcm_->InitializeReceiver()); - EXPECT_EQ(0, vcm_->InitializeSender()); EXPECT_EQ(0, vcm_->RegisterExternalEncoder(&encoder_, kUnusedPayloadType, false)); - EXPECT_EQ(0, vcm_->RegisterExternalDecoder(&decoder_, kUnusedPayloadType, - true)); memset(&settings_, 0, sizeof(settings_)); EXPECT_EQ(0, vcm_->Codec(kVideoCodecVP8, &settings_)); settings_.numberOfSimulcastStreams = kNumberOfStreams; @@ -57,7 +50,6 @@ class TestVideoCodingModule : public ::testing::Test { &settings_.simulcastStream[2]); settings_.plType = kUnusedPayloadType; // Use the mocked encoder. EXPECT_EQ(0, vcm_->RegisterSendCodec(&settings_, 1, 1200)); - EXPECT_EQ(0, vcm_->RegisterReceiveCodec(&settings_, 1, true)); } virtual void TearDown() { @@ -93,41 +85,10 @@ class TestVideoCodingModule : public ::testing::Test { stream->qpMax = 45; } - void InsertAndVerifyPaddingFrame(const uint8_t* payload, int length, - WebRtcRTPHeader* header) { - ASSERT_TRUE(header != NULL); - for (int j = 0; j < 5; ++j) { - // Padding only packets are passed to the VCM with payload size 0. - EXPECT_EQ(0, vcm_->IncomingPacket(payload, 0, *header)); - ++header->header.sequenceNumber; - } - EXPECT_CALL(packet_request_callback_, ResendPackets(_, _)) - .Times(0); - EXPECT_EQ(0, vcm_->Process()); - EXPECT_CALL(decoder_, Decode(_, _, _, _, _)) - .Times(0); - EXPECT_EQ(VCM_FRAME_NOT_READY, vcm_->Decode(0)); - } - - void InsertAndVerifyDecodableFrame(const uint8_t* payload, int length, - WebRtcRTPHeader* header) { - ASSERT_TRUE(header != NULL); - EXPECT_EQ(0, vcm_->IncomingPacket(payload, length, *header)); - ++header->header.sequenceNumber; - EXPECT_CALL(packet_request_callback_, ResendPackets(_, _)) - .Times(0); - EXPECT_EQ(0, vcm_->Process()); - EXPECT_CALL(decoder_, Decode(_, _, _, _, _)) - .Times(1); - EXPECT_EQ(0, vcm_->Decode(0)); - } - VideoCodingModule* vcm_; - NiceMock decoder_; NiceMock encoder_; I420VideoFrame input_frame_; VideoCodec settings_; - NiceMock packet_request_callback_; }; TEST_F(TestVideoCodingModule, TestIntraRequests) { @@ -176,112 +137,4 @@ TEST_F(TestVideoCodingModule, TestIntraRequestsInternalCapture) { EXPECT_EQ(-1, vcm_->IntraFrameRequest(-1)); } -TEST_F(TestVideoCodingModule, PaddingOnlyFrames) { - EXPECT_EQ(0, vcm_->SetVideoProtection(kProtectionNack, true)); - EXPECT_EQ(0, vcm_->RegisterPacketRequestCallback(&packet_request_callback_)); - const unsigned int kPaddingSize = 220; - const uint8_t payload[kPaddingSize] = {0}; - WebRtcRTPHeader header; - memset(&header, 0, sizeof(header)); - header.frameType = kFrameEmpty; - header.header.markerBit = false; - header.header.paddingLength = kPaddingSize; - header.header.payloadType = kUnusedPayloadType; - header.header.ssrc = 1; - header.header.headerLength = 12; - header.type.Video.codec = kRTPVideoVP8; - for (int i = 0; i < 10; ++i) { - InsertAndVerifyPaddingFrame(payload, 0, &header); - TickTime::AdvanceFakeClock(33); - header.header.timestamp += 3000; - } -} - -TEST_F(TestVideoCodingModule, PaddingOnlyFramesWithLosses) { - EXPECT_EQ(0, vcm_->SetVideoProtection(kProtectionNack, true)); - EXPECT_EQ(0, vcm_->RegisterPacketRequestCallback(&packet_request_callback_)); - const unsigned int kFrameSize = 1200; - const unsigned int kPaddingSize = 220; - const uint8_t payload[kPaddingSize] = {0}; - WebRtcRTPHeader header; - memset(&header, 0, sizeof(header)); - header.frameType = kFrameEmpty; - header.header.markerBit = false; - header.header.paddingLength = kPaddingSize; - header.header.payloadType = kUnusedPayloadType; - header.header.ssrc = 1; - header.header.headerLength = 12; - header.type.Video.codec = kRTPVideoVP8; - // Insert one video frame to get one frame decoded. - header.frameType = kVideoFrameKey; - header.type.Video.isFirstPacket = true; - header.header.markerBit = true; - InsertAndVerifyDecodableFrame(payload, kFrameSize, &header); - TickTime::AdvanceFakeClock(33); - header.header.timestamp += 3000; - - header.frameType = kFrameEmpty; - header.type.Video.isFirstPacket = false; - header.header.markerBit = false; - // Insert padding frames. - for (int i = 0; i < 10; ++i) { - // Lose the 4th frame. - if (i == 3) { - header.header.sequenceNumber += 5; - ++i; - } - // Lose one packet from the 6th frame. - if (i == 5) { - ++header.header.sequenceNumber; - } - InsertAndVerifyPaddingFrame(payload, 0, &header); - TickTime::AdvanceFakeClock(33); - header.header.timestamp += 3000; - } -} - -TEST_F(TestVideoCodingModule, PaddingOnlyAndVideo) { - EXPECT_EQ(0, vcm_->SetVideoProtection(kProtectionNack, true)); - EXPECT_EQ(0, vcm_->RegisterPacketRequestCallback(&packet_request_callback_)); - const unsigned int kFrameSize = 1200; - const unsigned int kPaddingSize = 220; - const uint8_t payload[kPaddingSize] = {0}; - WebRtcRTPHeader header; - memset(&header, 0, sizeof(header)); - header.frameType = kFrameEmpty; - header.type.Video.isFirstPacket = false; - header.header.markerBit = false; - header.header.paddingLength = kPaddingSize; - header.header.payloadType = kUnusedPayloadType; - header.header.ssrc = 1; - header.header.headerLength = 12; - header.type.Video.codec = kRTPVideoVP8; - header.type.Video.codecHeader.VP8.pictureId = -1; - header.type.Video.codecHeader.VP8.tl0PicIdx = -1; - for (int i = 0; i < 3; ++i) { - // Insert 2 video frames. - for (int j = 0; j < 2; ++j) { - if (i == 0 && j == 0) // First frame should be a key frame. - header.frameType = kVideoFrameKey; - else - header.frameType = kVideoFrameDelta; - header.type.Video.isFirstPacket = true; - header.header.markerBit = true; - InsertAndVerifyDecodableFrame(payload, kFrameSize, &header); - TickTime::AdvanceFakeClock(33); - header.header.timestamp += 3000; - } - - // Insert 2 padding only frames. - header.frameType = kFrameEmpty; - header.type.Video.isFirstPacket = false; - header.header.markerBit = false; - for (int j = 0; j < 2; ++j) { - InsertAndVerifyPaddingFrame(payload, 0, &header); - TickTime::AdvanceFakeClock(33); - header.header.timestamp += 3000; - } - } -} - } // namespace webrtc