VCM/JB: FrameForDecoding->IncompleteFrameForDecoding
- Update complete frame for decoding
- Remove FrameForDecodingNack
This CL should only be committed after issue http://webrtc-codereview.appspot.com/1313007/
Review URL: https://webrtc-codereview.appspot.com/1316007
git-svn-id: http://webrtc.googlecode.com/svn/trunk@3901 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/webrtc/modules/video_coding/main/source/jitter_buffer.cc b/webrtc/modules/video_coding/main/source/jitter_buffer.cc
index abcda82..0c11512 100644
--- a/webrtc/modules/video_coding/main/source/jitter_buffer.cc
+++ b/webrtc/modules/video_coding/main/source/jitter_buffer.cc
@@ -438,7 +438,6 @@
if (!running_) {
return NULL;
}
-
CleanUpOldOrEmptyFrames();
if (last_decoded_state_.in_initial_state()) {
@@ -482,15 +481,16 @@
frame_event_->Reset();
}
- if (it == frame_list_.end()) {
+ if (!decode_with_errors_ && it == frame_list_.end()) {
// Even after signaling we're still missing a complete continuous frame.
- // Look for a complete key frame.
+ // Look for a complete key frame if we're not decoding with errors.
it = find_if(frame_list_.begin(), frame_list_.end(),
CompleteKeyFrameCriteria());
- if (it == frame_list_.end()) {
+ }
+
+ if (it == frame_list_.end()) {
crit_sect_->Leave();
return NULL;
- }
}
VCMFrameBuffer* oldest_frame = *it;
@@ -530,15 +530,15 @@
return oldest_frame;
}
-VCMEncodedFrame* VCMJitterBuffer::GetFrameForDecoding() {
- TRACE_EVENT0("webrtc", "JB::GetFrameForDecoding");
+VCMEncodedFrame* VCMJitterBuffer::MaybeGetIncompleteFrameForDecoding() {
+ TRACE_EVENT0("webrtc", "JB::MaybeGetIncompleteFrameForDecoding");
CriticalSectionScoped cs(crit_sect_);
if (!running_) {
return NULL;
}
-
- if (WaitForRetransmissions()) {
- return GetFrameForDecodingNACK();
+ if (!decode_with_errors_) {
+ // No point to continue, as we are not decoding with errors.
+ return NULL;
}
CleanUpOldOrEmptyFrames();
@@ -548,15 +548,13 @@
}
VCMFrameBuffer* oldest_frame = frame_list_.front();
- if (frame_list_.size() <= 1 &&
- oldest_frame->GetState() != kStateComplete) {
+ // If we have only one frame in the buffer, release it only if it is complete.
+ if (frame_list_.size() <= 1 && oldest_frame->GetState() != kStateComplete) {
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
- // turned off NACK this might still happen.
const bool retransmitted = (oldest_frame->GetNackCount() > 0);
if (retransmitted) {
jitter_estimate_.FrameNacked();
@@ -998,57 +996,6 @@
return last_decoded_state_.time_stamp();
}
-VCMEncodedFrame* VCMJitterBuffer::GetFrameForDecodingNACK() {
- TRACE_EVENT0("webrtc", "JB::GetFrameForDecodingNACK");
- CleanUpOldOrEmptyFrames();
- // 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.
- it = find_if(frame_list_.begin(), frame_list_.end(),
- CompleteKeyFrameCriteria());
- if (it == frame_list_.end()) {
- return NULL;
- }
- }
- VCMFrameBuffer* oldest_frame = *it;
- // Update jitter estimate
- const bool retransmitted = (oldest_frame->GetNackCount() > 0);
- if (retransmitted) {
- jitter_estimate_.FrameNacked();
- } else if (oldest_frame->Length() > 0) {
- // Ignore retransmitted and empty frames.
- UpdateJitterEstimate(*oldest_frame, false);
- }
- it = frame_list_.erase(it);
- if (frame_list_.empty()) {
- TRACE_EVENT_INSTANT1("webrtc", "JB::FrameListEmptied",
- "type", "GetFrameForDecodingNACK");
- }
-
- // Look for previous frame loss.
- VerifyAndSetPreviousFrameLost(oldest_frame);
-
- // The state must be changed to decoding before cleaning up zero sized
- // frames to avoid empty frames being cleaned up and then given to the
- // 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());
- return oldest_frame;
-}
-
// Set the frame state to free and remove it from the sorted
// frame list. Must be called from inside the critical section crit_sect_.
void VCMJitterBuffer::ReleaseFrameIfNotDecoding(VCMFrameBuffer* frame) {
diff --git a/webrtc/modules/video_coding/main/source/jitter_buffer.h b/webrtc/modules/video_coding/main/source/jitter_buffer.h
index 669089f..fd637bb 100644
--- a/webrtc/modules/video_coding/main/source/jitter_buffer.h
+++ b/webrtc/modules/video_coding/main/source/jitter_buffer.h
@@ -104,13 +104,16 @@
// or more packets.
bool CompleteSequenceWithNextFrame();
- // TODO(mikhal/stefan): Merge all GetFrameForDecoding into one.
- // Wait |max_wait_time_ms| for a complete frame to arrive. After timeout NULL
- // is returned.
+ // Returns a complete frame ready for decoding. Allows max_wait_time_ms to
+ // wait for such a frame, if one is unavailable.
+ // Always starts with a key frame.
VCMEncodedFrame* GetCompleteFrameForDecoding(uint32_t max_wait_time_ms);
- // Get a frame for decoding (even an incomplete) without delay.
- VCMEncodedFrame* GetFrameForDecoding();
+ // Get next frame for decoding without delay. If decoding with errors is not
+ // enabled, will return NULL. Actual returned frame will be the next one in
+ // the list, either complete or not.
+ // TODO(mikhal): Consider only allowing decodable/complete.
+ VCMEncodedFrame* MaybeGetIncompleteFrameForDecoding();
// Releases a frame returned from the jitter buffer, should be called when
// done with decoding.
@@ -189,11 +192,6 @@
// Drops all packets in the NACK list up until |last_decoded_sequence_number|.
void DropPacketsFromNackList(uint16_t last_decoded_sequence_number);
- // In NACK-only mode this function doesn't return or release non-complete
- // frames unless we have a complete key frame. In hybrid mode, we may release
- // "decodable", incomplete frames.
- VCMEncodedFrame* GetFrameForDecodingNACK();
-
void ReleaseFrameIfNotDecoding(VCMFrameBuffer* frame);
// Gets an empty frame, creating a new frame if necessary (i.e. increases
diff --git a/webrtc/modules/video_coding/main/source/jitter_buffer_unittest.cc b/webrtc/modules/video_coding/main/source/jitter_buffer_unittest.cc
index 22fb425..bf81973 100644
--- a/webrtc/modules/video_coding/main/source/jitter_buffer_unittest.cc
+++ b/webrtc/modules/video_coding/main/source/jitter_buffer_unittest.cc
@@ -241,8 +241,9 @@
return ret;
}
- bool DecodeFrame() {
- VCMEncodedFrame* frame = jitter_buffer_->GetFrameForDecoding();
+ bool DecodeIncompleteFrame() {
+ VCMEncodedFrame* frame =
+ jitter_buffer_->MaybeGetIncompleteFrameForDecoding();
bool ret = (frame != NULL);
jitter_buffer_->ReleaseFrame(frame);
return ret;
@@ -416,7 +417,7 @@
EXPECT_GE(InsertFrame(kVideoFrameDelta), kNoError);
// Waiting for a key frame.
EXPECT_FALSE(DecodeCompleteFrame());
- EXPECT_FALSE(DecodeFrame());
+ EXPECT_FALSE(DecodeIncompleteFrame());
// The next complete continuous frame isn't a key frame, but we're waiting
// for one.
@@ -466,7 +467,7 @@
// The next complete continuous frame isn't a key frame, but we're waiting
// for one.
EXPECT_FALSE(DecodeCompleteFrame());
- EXPECT_FALSE(DecodeFrame());
+ EXPECT_FALSE(DecodeIncompleteFrame());
EXPECT_GE(InsertFrame(kVideoFrameKey), kNoError);
// Skipping ahead to the key frame.
EXPECT_TRUE(DecodeCompleteFrame());
@@ -521,9 +522,10 @@
TEST_F(TestJitterBufferNack, NormalOperation) {
EXPECT_EQ(kNack, jitter_buffer_->nack_mode());
+ jitter_buffer_->DecodeWithErrors(true);
EXPECT_GE(InsertFrame(kVideoFrameKey), kNoError);
- EXPECT_TRUE(DecodeFrame());
+ EXPECT_TRUE(DecodeIncompleteFrame());
// ----------------------------------------------------------------
// | 1 | 2 | .. | 8 | 9 | x | 11 | 12 | .. | 19 | x | 21 | .. | 100 |
@@ -544,7 +546,7 @@
EXPECT_EQ(kIncomplete, InsertPacketAndPop(0));
EXPECT_EQ(0, stream_generator->PacketsRemaining());
EXPECT_FALSE(DecodeCompleteFrame());
- EXPECT_FALSE(DecodeFrame());
+ EXPECT_FALSE(DecodeIncompleteFrame());
uint16_t nack_list_size = 0;
bool request_key_frame = false;
uint16_t* list = jitter_buffer_->GetNackList(&nack_list_size,
diff --git a/webrtc/modules/video_coding/main/source/receiver.cc b/webrtc/modules/video_coding/main/source/receiver.cc
index 2595fb2..fffe609 100644
--- a/webrtc/modules/video_coding/main/source/receiver.cc
+++ b/webrtc/modules/video_coding/main/source/receiver.cc
@@ -254,11 +254,8 @@
!jitter_buffer_.CompleteSequenceWithNextFrame()) {
// Jitter buffer state might get corrupt with this frame.
dual_receiver->CopyJitterBufferStateFromReceiver(*this);
- frame = jitter_buffer_.GetFrameForDecoding();
- assert(frame);
- } else {
- frame = jitter_buffer_.GetFrameForDecoding();
}
+ frame = jitter_buffer_.MaybeGetIncompleteFrameForDecoding();
}
if (frame == NULL) {
// Wait for a complete frame.
@@ -282,7 +279,7 @@
dual_receiver->CopyJitterBufferStateFromReceiver(*this);
}
- frame = jitter_buffer_.GetFrameForDecoding();
+ frame = jitter_buffer_.MaybeGetIncompleteFrameForDecoding();
}
return frame;
}
@@ -320,7 +317,7 @@
dual_receiver->CopyJitterBufferStateFromReceiver(*this);
}
- frame = jitter_buffer_.GetFrameForDecoding();
+ frame = jitter_buffer_.MaybeGetIncompleteFrameForDecoding();
}
return frame;
}
@@ -412,6 +409,16 @@
return state_;
}
+void VCMReceiver::SetDecodeWithErrors(bool enable){
+ CriticalSectionScoped cs(crit_sect_);
+ jitter_buffer_.DecodeWithErrors(enable);
+}
+
+bool VCMReceiver::DecodeWithErrors() const {
+ CriticalSectionScoped cs(crit_sect_);
+ return jitter_buffer_.decode_with_errors();
+}
+
int VCMReceiver::SetMinReceiverDelay(int desired_delay_ms) {
CriticalSectionScoped cs(crit_sect_);
if (desired_delay_ms < 0 || desired_delay_ms > kMaxReceiverDelayMs) {
diff --git a/webrtc/modules/video_coding/main/source/receiver.h b/webrtc/modules/video_coding/main/source/receiver.h
index 88f2fb2..f0cf865 100644
--- a/webrtc/modules/video_coding/main/source/receiver.h
+++ b/webrtc/modules/video_coding/main/source/receiver.h
@@ -76,6 +76,10 @@
// Receiver video delay.
int SetMinReceiverDelay(int desired_delay_ms);
+ // Decoding with errors.
+ void SetDecodeWithErrors(bool enable);
+ bool DecodeWithErrors() const;
+
private:
VCMEncodedFrame* FrameForDecoding(uint16_t max_wait_time_ms,
int64_t nextrender_time_ms,
diff --git a/webrtc/modules/video_coding/main/source/video_coding_impl.cc b/webrtc/modules/video_coding/main/source/video_coding_impl.cc
index 64d00c1..74730ec 100644
--- a/webrtc/modules/video_coding/main/source/video_coding_impl.cc
+++ b/webrtc/modules/video_coding/main/source/video_coding_impl.cc
@@ -535,10 +535,17 @@
}
// Enable or disable a video protection method.
+// Note: This API should be deprecated, as it does not offer a distinction
+// between the protection method and decoding with or without errors. If such a
+// behavior is desired, use the following API: SetReceiverRobustnessMode.
int32_t
VideoCodingModuleImpl::SetVideoProtection(VCMVideoProtection videoProtection,
bool enable)
{
+ // By default, do not decode with errors.
+ _receiver.SetDecodeWithErrors(false);
+ // The dual decoder should always be error free.
+ _dualReceiver.SetDecodeWithErrors(false);
switch (videoProtection)
{
@@ -583,6 +590,7 @@
// Enable NACK and always wait for retransmissions and
// compensate with extra delay.
_dualReceiver.SetNackMode(kNack, -1, -1);
+ _receiver.SetDecodeWithErrors(true);
}
else
{
@@ -597,6 +605,7 @@
if (enable)
{
_keyRequestMode = kKeyOnLoss;
+ _receiver.SetDecodeWithErrors(true);
}
else if (_keyRequestMode == kKeyOnLoss)
{
@@ -640,6 +649,8 @@
_receiver.SetNackMode(kNack,
media_optimization::kLowRttNackMs,
-1);
+ _receiver.SetDecodeWithErrors(false);
+ _receiver.SetDecodeWithErrors(false);
}
else
{
@@ -1021,6 +1032,10 @@
}
int64_t dummyRenderTime;
int32_t decodeCount = 0;
+ // The dual decoder's state is copied from the main decoder, which may
+ // decode with errors. Make sure that the dual decoder does not introduce
+ // error.
+ _dualReceiver.SetDecodeWithErrors(false);
VCMEncodedFrame* dualFrame = _dualReceiver.FrameForDecoding(
maxWaitTimeMs,
dummyRenderTime);
@@ -1417,6 +1432,9 @@
_dualReceiver.SetNackMode(kNoNack, -1, -1);
break;
}
+ _receiver.SetDecodeWithErrors(errorMode == kAllowDecodeErrors);
+ // The dual decoder should never decode with errors.
+ _dualReceiver.SetDecodeWithErrors(false);
return VCM_OK;
}
diff --git a/webrtc/modules/video_coding/main/source/video_coding_robustness_unittest.cc b/webrtc/modules/video_coding/main/source/video_coding_robustness_unittest.cc
index d7e0723..b06e296 100644
--- a/webrtc/modules/video_coding/main/source/video_coding_robustness_unittest.cc
+++ b/webrtc/modules/video_coding/main/source/video_coding_robustness_unittest.cc
@@ -329,73 +329,4 @@
InsertPacket(9000, 11, false, true, kVideoFrameDelta);
EXPECT_EQ(VCM_OK, vcm_->Decode(0)); // Decode timestamp 9000 complete.
}
-
-TEST_F(VCMRobustnessTest, TestModeNoneWithoutErrors) {
- Sequence s1;
- EXPECT_CALL(decoder_, InitDecode(_, _)).Times(1);
- EXPECT_CALL(decoder_, Release()).Times(1);
- EXPECT_CALL(request_callback_, ResendPackets(_, 1))
- .With(Args<0, 1>(ElementsAre(4)))
- .Times(0);
-
- EXPECT_CALL(decoder_, Copy())
- .Times(0);
- EXPECT_CALL(decoderCopy_, Copy())
- .Times(0);
-
- // Decode operations
- EXPECT_CALL(decoder_, Decode(AllOf(Field(&EncodedImage::_timeStamp, 0),
- Field(&EncodedImage::_completeFrame,
- true)),
- false, _, _, _))
- .Times(1)
- .InSequence(s1);
- EXPECT_CALL(decoder_, Decode(AllOf(Field(&EncodedImage::_timeStamp, 3000),
- Field(&EncodedImage::_completeFrame,
- false)),
- false, _, _, _))
- .Times(1)
- .InSequence(s1);
- EXPECT_CALL(decoder_, Decode(AllOf(Field(&EncodedImage::_timeStamp, 6000),
- Field(&EncodedImage::_completeFrame,
- true)),
- false, _, _, _))
- .Times(1)
- .InSequence(s1);
- EXPECT_CALL(frame_type_callback_, RequestKeyFrame())
- .Times(1);
-
- ASSERT_EQ(VCM_OK, vcm_->SetReceiverRobustnessMode(
- VideoCodingModule::kNone,
- VideoCodingModule::kNoDecodeErrors));
-
- InsertPacket(0, 0, true, false, kVideoFrameKey);
- InsertPacket(0, 1, false, false, kVideoFrameKey);
- InsertPacket(0, 2, false, true, kVideoFrameKey);
- EXPECT_EQ(VCM_OK, vcm_->Decode(0)); // Decode timestamp 0.
- EXPECT_EQ(VCM_OK, vcm_->Process()); // Expect no NACK list.
-
- clock_->AdvanceTimeMilliseconds(33);
- InsertPacket(3000, 3, true, false, kVideoFrameDelta);
- // Packet 4 missing
- InsertPacket(3000, 5, false, true, kVideoFrameDelta);
- EXPECT_EQ(VCM_FRAME_NOT_READY, vcm_->Decode(0));
- EXPECT_EQ(VCM_OK, vcm_->Process()); // Expect no NACK list.
-
- clock_->AdvanceTimeMilliseconds(33);
- InsertPacket(6000, 6, true, false, kVideoFrameDelta);
- InsertPacket(6000, 7, false, false, kVideoFrameDelta);
- InsertPacket(6000, 8, false, true, kVideoFrameDelta);
- EXPECT_EQ(VCM_OK, vcm_->Decode(0)); // Decode timestamp 3000 incomplete.
- // Schedule key frame request.
- EXPECT_EQ(VCM_OK, vcm_->Process()); // Expect no NACK list.
-
- clock_->AdvanceTimeMilliseconds(10);
- EXPECT_EQ(VCM_OK, vcm_->Decode(0)); // Decode timestamp 6000 complete.
- EXPECT_EQ(VCM_OK, vcm_->Process()); // Expect no NACK list.
-
- // Wait for the key request timer to set.
- clock_->AdvanceTimeMilliseconds(500);
- EXPECT_EQ(VCM_OK, vcm_->Process()); // Expect key frame request.
-}
} // namespace webrtc
diff --git a/webrtc/modules/video_coding/main/test/jitter_buffer_test.cc b/webrtc/modules/video_coding/main/test/jitter_buffer_test.cc
index 975468c..d78b7e3 100644
--- a/webrtc/modules/video_coding/main/test/jitter_buffer_test.cc
+++ b/webrtc/modules/video_coding/main/test/jitter_buffer_test.cc
@@ -136,11 +136,14 @@
TEST(0 == jb.GetFrame(packet));
TEST(-1 == jb.NextTimestamp(10, &incomingFrameType, &renderTimeMs));
TEST(0 == jb.GetCompleteFrameForDecoding(10));
- TEST(0 == jb.GetFrameForDecoding());
+ TEST(0 == jb.MaybeGetIncompleteFrameForDecoding());
// Start
jb.Start();
+ // Allow decoding with errors.
+ jb.DecodeWithErrors(true);
+
// Get frame to use for this timestamp
VCMEncodedFrame* frameIn = jb.GetFrame(packet);
TEST(frameIn != 0);
@@ -808,7 +811,7 @@
TEST(kIncomplete == jb.InsertPacket(frameIn, packet));
// Get the frame
- frameOut = jb.GetFrameForDecoding();
+ frameOut = jb.MaybeGetIncompleteFrameForDecoding();
// One of the packets has been discarded by the jitter buffer.
// Last frame can't be extracted yet.
@@ -1665,7 +1668,7 @@
packet.seqNum = seqNum;
packet.timestamp = timeStamp;
packet.frameType = kFrameEmpty;
- VCMEncodedFrame* testFrame = jb.GetFrameForDecoding();
+ VCMEncodedFrame* testFrame = jb.MaybeGetIncompleteFrameForDecoding();
// timestamp should bever be the last TS inserted
if (testFrame != NULL)
{
@@ -1744,7 +1747,7 @@
// Get packet notification
TEST(timeStamp == jb.NextTimestamp(10, &incomingFrameType,
&renderTimeMs));
- frameOut = jb.GetFrameForDecoding();
+ frameOut = jb.MaybeGetIncompleteFrameForDecoding();
// We can decode everything from a NALU until a packet has been lost.
// Thus we can decode the first packet of the first NALU and the second NALU
@@ -1810,7 +1813,7 @@
// This packet should not be decoded because it is an incomplete NAL if it
// is the last
- frameOut = jb.GetFrameForDecoding();
+ frameOut = jb.MaybeGetIncompleteFrameForDecoding();
// Only last NALU is complete
TEST(CheckOutFrame(frameOut, insertedLength, false) == 0);
jb.ReleaseFrame(frameOut);
@@ -1834,7 +1837,7 @@
// Will be sent to the decoder, as a packet belonging to a subsequent frame
// has arrived.
- frameOut = jb.GetFrameForDecoding();
+ frameOut = jb.MaybeGetIncompleteFrameForDecoding();
// Test that a frame can include an empty packet.
@@ -1880,7 +1883,7 @@
TEST(frameIn = jb.GetFrame(packet));
TEST(kFirstPacket == jb.InsertPacket(frameIn, packet));
- frameOut = jb.GetFrameForDecoding();
+ frameOut = jb.MaybeGetIncompleteFrameForDecoding();
TEST(frameOut == NULL);
packet.seqNum += 2;
@@ -1889,7 +1892,7 @@
TEST(frameIn = jb.GetFrame(packet));
TEST(kFirstPacket == jb.InsertPacket(frameIn, packet));
- frameOut = jb.GetFrameForDecoding();
+ frameOut = jb.MaybeGetIncompleteFrameForDecoding();
TEST(frameOut != NULL);
TEST(CheckOutFrame(frameOut, packet.sizeBytes, false) == 0);
