From d855c1a4e8d9e54330df76811725348b47cf7aa0 Mon Sep 17 00:00:00 2001 From: "stefan@webrtc.org" Date: Tue, 25 Oct 2011 11:52:48 +0000 Subject: [PATCH] Reverts r807 and fixes the real issue in the VCM. This fixes an issue in the VCM where we don't wait for a packet to arrive if the jitter buffer is empty. This also fixes an issue where an old packet can trigger a packet event signal for a future frame. BUG= TEST= Review URL: http://webrtc-codereview.appspot.com/248001 git-svn-id: http://webrtc.googlecode.com/svn/trunk@814 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../video_coding/main/source/jitter_buffer.cc | 6 ++-- .../video_coding/main/source/receiver.cc | 28 ++++++++++++------- src/video_engine/main/source/vie_channel.cc | 11 +------- src/video_engine/main/source/vie_channel.h | 2 -- 4 files changed, 22 insertions(+), 25 deletions(-) diff --git a/src/modules/video_coding/main/source/jitter_buffer.cc b/src/modules/video_coding/main/source/jitter_buffer.cc index 24a1b2e731..6b63edbaad 100644 --- a/src/modules/video_coding/main/source/jitter_buffer.cc +++ b/src/modules/video_coding/main/source/jitter_buffer.cc @@ -54,8 +54,7 @@ VCMJitterBuffer::CompleteDecodableKeyFrameCriteria(VCMFrameBuffer* frame, const VCMFrameBufferStateEnum state = frame->GetState(); // We can decode key frame or decodable/complete frames. return (frame->FrameType() == kVideoFrameKey) && - ((state == kStateComplete) - || (state == kStateDecodable)); + (state == kStateComplete || state == kStateDecodable); } // Constructor @@ -889,7 +888,9 @@ VCMJitterBuffer::GetNextTimeStamp(WebRtc_UWord32 maxWaitTimeMS, if (oldestFrame == NULL) { + _packetEvent.Reset(); _critSect.Leave(); + if (_packetEvent.Wait(maxWaitTimeMS) == kEventSignaled) { // are we closing down the Jitter buffer @@ -908,7 +909,6 @@ VCMJitterBuffer::GetNextTimeStamp(WebRtc_UWord32 maxWaitTimeMS, _critSect.Enter(); } } - _packetEvent.Reset(); if (oldestFrame == NULL) { diff --git a/src/modules/video_coding/main/source/receiver.cc b/src/modules/video_coding/main/source/receiver.cc index 69a49a4077..10227f053f 100644 --- a/src/modules/video_coding/main/source/receiver.cc +++ b/src/modules/video_coding/main/source/receiver.cc @@ -241,38 +241,46 @@ VCMReceiver::FrameForDecoding(WebRtc_UWord16 maxWaitTimeMs, { // How long can we wait until we must decode the next frame WebRtc_UWord32 waitTimeMs = _timing.MaxWaitingTime(nextRenderTimeMs, - VCMTickTime::MillisecondTimestamp()); + VCMTickTime::MillisecondTimestamp()); // Try to get a complete frame from the jitter buffer VCMEncodedFrame* frame = _jitterBuffer.GetCompleteFrameForDecoding(0); if (frame == NULL && maxWaitTimeMs == 0 && waitTimeMs > 0) { - // If we're not allowed to wait for frames to get complete we must calculate if - // it's time to decode, and if it's not we will just return for now. + // If we're not allowed to wait for frames to get complete we must + // calculate if it's time to decode, and if it's not we will just return + // for now. return NULL; } + if (frame == NULL && VCM_MIN(waitTimeMs, maxWaitTimeMs) == 0) + { + // No time to wait for a complete frame, + // check if we have an incomplete + frame = _jitterBuffer.GetFrameForDecoding(); + } if (frame == NULL) { // Wait for a complete frame - waitTimeMs = VCM_MIN(waitTimeMs, maxWaitTimeMs); - frame = _jitterBuffer.GetCompleteFrameForDecoding(waitTimeMs); + frame = _jitterBuffer.GetCompleteFrameForDecoding(maxWaitTimeMs); } if (frame == NULL) { // Get an incomplete frame - if (_timing.MaxWaitingTime(nextRenderTimeMs, VCMTickTime::MillisecondTimestamp()) > 0) + if (_timing.MaxWaitingTime(nextRenderTimeMs, + VCMTickTime::MillisecondTimestamp()) > 0) { // Still time to wait for a complete frame return NULL; } // No time left to wait, we must decode this frame now. - const bool dualReceiverEnabledAndPassive = dualReceiver != NULL && - dualReceiver->State() == kPassive && - dualReceiver->NackMode() == kNackInfinite; - if (dualReceiverEnabledAndPassive && !_jitterBuffer.CompleteSequenceWithNextFrame()) + const bool dualReceiverEnabledAndPassive = (dualReceiver != NULL && + dualReceiver->State() == kPassive && + dualReceiver->NackMode() == kNackInfinite); + if (dualReceiverEnabledAndPassive && + !_jitterBuffer.CompleteSequenceWithNextFrame()) { // Jitter buffer state might get corrupt with this frame. dualReceiver->CopyJitterBufferStateFromReceiver(*this); diff --git a/src/video_engine/main/source/vie_channel.cc b/src/video_engine/main/source/vie_channel.cc index 7c30c2e4b2..118e840831 100644 --- a/src/video_engine/main/source/vie_channel.cc +++ b/src/video_engine/main/source/vie_channel.cc @@ -15,7 +15,6 @@ #include "vie_channel.h" #include "vie_defines.h" #include "critical_section_wrapper.h" -#include "event_wrapper.h" #include "rtp_rtcp.h" #include "udp_transport.h" #include "video_coding.h" @@ -37,7 +36,6 @@ namespace webrtc { -static const int kDecoderThreadWaitPeriod = 3; // wait 3 ms // ---------------------------------------------------------------------------- // Constructor // ---------------------------------------------------------------------------- @@ -77,7 +75,6 @@ ViEChannel::ViEChannel(WebRtc_Word32 channelId, WebRtc_Word32 engineId, _decoderReset(true), _waitForKeyFrame(false), _ptrDecodeThread(NULL), - _vieDecodeEvent(*EventWrapper::Create()), _ptrSrtpModuleEncryption(NULL), _ptrSrtpModuleDecryption(NULL), _ptrExternalEncryption(NULL), @@ -262,13 +259,11 @@ ViEChannel::~ViEChannel() RtpRtcp::DestroyRtpRtcp(rtpRtcp); _simulcastRtpRtcp.erase(it); } - _vieDecodeEvent.Set(); if (_ptrDecodeThread) { StopDecodeThread(); } - delete &_vieDecodeEvent; delete &_vieReceiver; delete &_vieSender; delete &_vieSync; @@ -3095,11 +3090,7 @@ bool ViEChannel::ChannelDecodeThreadFunction(void* obj) bool ViEChannel::ChannelDecodeProcess() { // Decode is blocking, but sleep some time anyway to not get a spin - int ret = _vcm.Decode(50); - if (ret == VCM_FRAME_NOT_READY) { - // TODO (mikhal): Add trigger by incoming packet. - _vieDecodeEvent.Wait(kDecoderThreadWaitPeriod); - } + _vcm.Decode(50); if ((TickTime::Now() - _vcmRTTReported).Milliseconds() > 1000) { diff --git a/src/video_engine/main/source/vie_channel.h b/src/video_engine/main/source/vie_channel.h index 63d41e1286..a68e85ed38 100644 --- a/src/video_engine/main/source/vie_channel.h +++ b/src/video_engine/main/source/vie_channel.h @@ -42,7 +42,6 @@ namespace webrtc { class CriticalSectionWrapper; class Encryption; -class EventWrapper; class ProcessThread; class RtpRtcp; class ThreadWrapper; @@ -472,7 +471,6 @@ private: // Decoder ThreadWrapper* _ptrDecodeThread; - EventWrapper& _vieDecodeEvent; //SRTP - using seperate pointers for encryption and decryption to support // simultaneous operations.