From 1248d4effcb01b3919b89dbe22d54161836c56e2 Mon Sep 17 00:00:00 2001 From: "mikhal@webrtc.org" Date: Tue, 23 Apr 2013 20:57:06 +0000 Subject: [PATCH] VCM/JB: Using last decoded state for waiting for key Review URL: https://webrtc-codereview.appspot.com/1323006 git-svn-id: http://webrtc.googlecode.com/svn/trunk@3892 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../main/source/decoding_state.cc | 7 +++- .../main/source/decoding_state_unittest.cc | 12 ++++-- .../video_coding/main/source/jitter_buffer.cc | 42 ++++--------------- .../video_coding/main/source/jitter_buffer.h | 1 - 4 files changed, 22 insertions(+), 40 deletions(-) diff --git a/webrtc/modules/video_coding/main/source/decoding_state.cc b/webrtc/modules/video_coding/main/source/decoding_state.cc index 81478971b..eb03705ec 100644 --- a/webrtc/modules/video_coding/main/source/decoding_state.cc +++ b/webrtc/modules/video_coding/main/source/decoding_state.cc @@ -149,8 +149,11 @@ bool VCMDecodingState::ContinuousFrame(const VCMFrameBuffer* frame) const { // Return true when in initial state. // Note that when a method is not applicable it will return false. assert(frame != NULL); - if (in_initial_state_) - return true; + if (in_initial_state_) { + // Always start with a key frame. + if (frame->FrameType() == kVideoFrameKey) return true; + return false; + } if (!ContinuousLayer(frame->TemporalId(), frame->Tl0PicId())) { // Base layers are not continuous or temporal layers are inactive. diff --git a/webrtc/modules/video_coding/main/source/decoding_state_unittest.cc b/webrtc/modules/video_coding/main/source/decoding_state_unittest.cc index 53a26ad18..c5ad5e6df 100644 --- a/webrtc/modules/video_coding/main/source/decoding_state_unittest.cc +++ b/webrtc/modules/video_coding/main/source/decoding_state_unittest.cc @@ -30,8 +30,9 @@ TEST(TestDecodingState, Sanity) { TEST(TestDecodingState, FrameContinuity) { VCMDecodingState dec_state; // Check that makes decision based on correct method. - VCMFrameBuffer frame; + VCMFrameBuffer frame, frame_key; frame.SetState(kStateEmpty); + frame_key.SetState(kStateEmpty); VCMPacket* packet = new VCMPacket(); packet->isFirstPacket = 1; packet->timestamp = 1; @@ -40,11 +41,15 @@ TEST(TestDecodingState, FrameContinuity) { packet->codecSpecificHeader.codec = kRTPVideoVP8; packet->codecSpecificHeader.codecHeader.VP8.pictureId = 0x007F; frame.InsertPacket(*packet, 0, false, 0); - // Should return true on init. + // Always start with a key frame. dec_state.Reset(); - EXPECT_TRUE(dec_state.ContinuousFrame(&frame)); + EXPECT_FALSE(dec_state.ContinuousFrame(&frame)); + packet->frameType = kVideoFrameKey; + frame_key.InsertPacket(*packet, 0, false, 0); + EXPECT_TRUE(dec_state.ContinuousFrame(&frame_key)); dec_state.SetState(&frame); frame.Reset(); + packet->frameType = kVideoFrameDelta; // Use pictureId packet->codecSpecificHeader.codecHeader.VP8.pictureId = 0x0002; frame.InsertPacket(*packet, 0, false, 0); @@ -458,5 +463,4 @@ TEST(TestDecodingState, OldInput) { delete packet; } - } // namespace webrtc diff --git a/webrtc/modules/video_coding/main/source/jitter_buffer.cc b/webrtc/modules/video_coding/main/source/jitter_buffer.cc index abcda8202..ecb015af5 100644 --- a/webrtc/modules/video_coding/main/source/jitter_buffer.cc +++ b/webrtc/modules/video_coding/main/source/jitter_buffer.cc @@ -105,7 +105,6 @@ VCMJitterBuffer::VCMJitterBuffer(Clock* clock, nack_seq_nums_(), max_nack_list_size_(0), max_packet_age_to_nack_(0), - waiting_for_key_frame_(false), decode_with_errors_(false) { memset(frame_buffers_, 0, sizeof(frame_buffers_)); memset(receive_statistics_, 0, sizeof(receive_statistics_)); @@ -147,7 +146,6 @@ void VCMJitterBuffer::CopyFrom(const VCMJitterBuffer& rhs) { inter_frame_delay_ = rhs.inter_frame_delay_; waiting_for_completion_ = rhs.waiting_for_completion_; rtt_ms_ = rhs.rtt_ms_; - waiting_for_key_frame_ = rhs.waiting_for_key_frame_; first_packet_ = rhs.first_packet_; last_decoded_state_ = rhs.last_decoded_state_; num_not_decodable_packets_ = rhs.num_not_decodable_packets_; @@ -201,9 +199,9 @@ void VCMJitterBuffer::Start() { waiting_for_completion_.timestamp = 0; waiting_for_completion_.latest_packet_time = -1; first_packet_ = true; - waiting_for_key_frame_ = false; rtt_ms_ = kDefaultRtt; num_not_decodable_packets_ = 0; + last_decoded_state_.Reset(); WEBRTC_TRACE(webrtc::kTraceDebug, webrtc::kTraceVideoCoding, VCMId(vcm_id_, receiver_id_), "JB(0x%x): Jitter buffer: start", @@ -441,10 +439,6 @@ VCMEncodedFrame* VCMJitterBuffer::GetCompleteFrameForDecoding( CleanUpOldOrEmptyFrames(); - if (last_decoded_state_.in_initial_state()) { - waiting_for_key_frame_ = true; - } - FrameList::iterator it = FindOldestCompleteContinuousFrame(); if (it == frame_list_.end()) { const int64_t end_wait_time_ms = clock_->TimeInMilliseconds() + @@ -495,12 +489,6 @@ VCMEncodedFrame* VCMJitterBuffer::GetCompleteFrameForDecoding( VCMFrameBuffer* oldest_frame = *it; - // Are we waiting for a key frame? - if (waiting_for_key_frame_ && oldest_frame->FrameType() != kVideoFrameKey) { - crit_sect_->Leave(); - return NULL; - } - it = frame_list_.erase(it); if (frame_list_.empty()) { TRACE_EVENT_INSTANT1("webrtc", "JB::FrameListEmptied", @@ -518,10 +506,6 @@ VCMEncodedFrame* VCMJitterBuffer::GetCompleteFrameForDecoding( oldest_frame->SetState(kStateDecoding); - if (oldest_frame->FrameType() == kVideoFrameKey) { - waiting_for_key_frame_ = false; - } - // We have a frame - update decoded state with frame info. last_decoded_state_.SetState(oldest_frame); DropPacketsFromNackList(last_decoded_state_.sequence_num()); @@ -553,6 +537,12 @@ VCMEncodedFrame* VCMJitterBuffer::GetFrameForDecoding() { return NULL; } + // Always start with a key frame. + if (last_decoded_state_.in_initial_state() && + oldest_frame->FrameType() != kVideoFrameKey) { + return NULL; + } + // Incomplete frame pulled out from jitter buffer, // update the jitter estimate with what we currently know. // This frame shouldn't have been retransmitted, but if we recently @@ -587,10 +577,6 @@ VCMEncodedFrame* VCMJitterBuffer::GetFrameForDecoding() { // Set as decoding. Propagates the missing_frame bit. oldest_frame->SetState(kStateDecoding); - if (oldest_frame->FrameType() == kVideoFrameKey) { - waiting_for_key_frame_ = false; - } - num_not_decodable_packets_ += oldest_frame->NotDecodablePackets(); // We have a frame - update decoded state with frame info. @@ -1004,9 +990,6 @@ VCMEncodedFrame* VCMJitterBuffer::GetFrameForDecodingNACK() { // First look for a complete continuous frame. // When waiting for nack, wait for a key frame, if a continuous frame cannot // be determined (i.e. initial decoding state). - if (last_decoded_state_.in_initial_state()) { - waiting_for_key_frame_ = true; - } FrameList::iterator it = FindOldestCompleteContinuousFrame(); if (it == frame_list_.end()) { // If we didn't find one we're good with a complete key frame. @@ -1017,6 +1000,7 @@ VCMEncodedFrame* VCMJitterBuffer::GetFrameForDecodingNACK() { } } VCMFrameBuffer* oldest_frame = *it; + // Update jitter estimate const bool retransmitted = (oldest_frame->GetNackCount() > 0); if (retransmitted) { @@ -1039,10 +1023,6 @@ VCMEncodedFrame* VCMJitterBuffer::GetFrameForDecodingNACK() { // decoder. oldest_frame->SetState(kStateDecoding); - if (oldest_frame->FrameType() == kVideoFrameKey) { - waiting_for_key_frame_ = false; - } - // We have a frame - update decoded state with frame info. last_decoded_state_.SetState(oldest_frame); DropPacketsFromNackList(last_decoded_state_.sequence_num()); @@ -1120,7 +1100,6 @@ bool VCMJitterBuffer::RecycleFramesUntilKeyFrame() { TRACE_EVENT_INSTANT1("webrtc", "JB::FrameListEmptied", "type", "RecycleFramesUntilKeyFrame"); } - waiting_for_key_frame_ = true; last_decoded_state_.Reset(); // TODO(mikhal): No sync. missing_sequence_numbers_.clear(); return false; @@ -1262,11 +1241,8 @@ FrameList::iterator VCMJitterBuffer::FindOldestCompleteContinuousFrame() { if (oldest_frame == NULL) { // No complete frame no point to continue. return frame_list_.end(); - } else if (waiting_for_key_frame_ && - oldest_frame->FrameType() != kVideoFrameKey) { - // We are waiting for a key frame. - return frame_list_.end(); } + // We have a complete continuous frame. return it; } diff --git a/webrtc/modules/video_coding/main/source/jitter_buffer.h b/webrtc/modules/video_coding/main/source/jitter_buffer.h index 669089fde..13dcf834c 100644 --- a/webrtc/modules/video_coding/main/source/jitter_buffer.h +++ b/webrtc/modules/video_coding/main/source/jitter_buffer.h @@ -294,7 +294,6 @@ class VCMJitterBuffer { std::vector nack_seq_nums_; size_t max_nack_list_size_; int max_packet_age_to_nack_; // Measured in sequence numbers. - bool waiting_for_key_frame_; bool decode_with_errors_; DISALLOW_COPY_AND_ASSIGN(VCMJitterBuffer);