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
This commit is contained in:
stefan@webrtc.org 2011-10-25 11:52:48 +00:00
parent bdb55c806f
commit d855c1a4e8
4 changed files with 22 additions and 25 deletions

View File

@ -54,8 +54,7 @@ VCMJitterBuffer::CompleteDecodableKeyFrameCriteria(VCMFrameBuffer* frame,
const VCMFrameBufferStateEnum state = frame->GetState(); const VCMFrameBufferStateEnum state = frame->GetState();
// We can decode key frame or decodable/complete frames. // We can decode key frame or decodable/complete frames.
return (frame->FrameType() == kVideoFrameKey) && return (frame->FrameType() == kVideoFrameKey) &&
((state == kStateComplete) (state == kStateComplete || state == kStateDecodable);
|| (state == kStateDecodable));
} }
// Constructor // Constructor
@ -889,7 +888,9 @@ VCMJitterBuffer::GetNextTimeStamp(WebRtc_UWord32 maxWaitTimeMS,
if (oldestFrame == NULL) if (oldestFrame == NULL)
{ {
_packetEvent.Reset();
_critSect.Leave(); _critSect.Leave();
if (_packetEvent.Wait(maxWaitTimeMS) == kEventSignaled) if (_packetEvent.Wait(maxWaitTimeMS) == kEventSignaled)
{ {
// are we closing down the Jitter buffer // are we closing down the Jitter buffer
@ -908,7 +909,6 @@ VCMJitterBuffer::GetNextTimeStamp(WebRtc_UWord32 maxWaitTimeMS,
_critSect.Enter(); _critSect.Enter();
} }
} }
_packetEvent.Reset();
if (oldestFrame == NULL) if (oldestFrame == NULL)
{ {

View File

@ -241,38 +241,46 @@ VCMReceiver::FrameForDecoding(WebRtc_UWord16 maxWaitTimeMs,
{ {
// How long can we wait until we must decode the next frame // How long can we wait until we must decode the next frame
WebRtc_UWord32 waitTimeMs = _timing.MaxWaitingTime(nextRenderTimeMs, WebRtc_UWord32 waitTimeMs = _timing.MaxWaitingTime(nextRenderTimeMs,
VCMTickTime::MillisecondTimestamp()); VCMTickTime::MillisecondTimestamp());
// Try to get a complete frame from the jitter buffer // Try to get a complete frame from the jitter buffer
VCMEncodedFrame* frame = _jitterBuffer.GetCompleteFrameForDecoding(0); VCMEncodedFrame* frame = _jitterBuffer.GetCompleteFrameForDecoding(0);
if (frame == NULL && maxWaitTimeMs == 0 && waitTimeMs > 0) if (frame == NULL && maxWaitTimeMs == 0 && waitTimeMs > 0)
{ {
// If we're not allowed to wait for frames to get complete we must calculate if // If we're not allowed to wait for frames to get complete we must
// it's time to decode, and if it's not we will just return for now. // calculate if it's time to decode, and if it's not we will just return
// for now.
return NULL; 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) if (frame == NULL)
{ {
// Wait for a complete frame // Wait for a complete frame
waitTimeMs = VCM_MIN(waitTimeMs, maxWaitTimeMs); frame = _jitterBuffer.GetCompleteFrameForDecoding(maxWaitTimeMs);
frame = _jitterBuffer.GetCompleteFrameForDecoding(waitTimeMs);
} }
if (frame == NULL) if (frame == NULL)
{ {
// Get an incomplete frame // 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 // Still time to wait for a complete frame
return NULL; return NULL;
} }
// No time left to wait, we must decode this frame now. // No time left to wait, we must decode this frame now.
const bool dualReceiverEnabledAndPassive = dualReceiver != NULL && const bool dualReceiverEnabledAndPassive = (dualReceiver != NULL &&
dualReceiver->State() == kPassive && dualReceiver->State() == kPassive &&
dualReceiver->NackMode() == kNackInfinite; dualReceiver->NackMode() == kNackInfinite);
if (dualReceiverEnabledAndPassive && !_jitterBuffer.CompleteSequenceWithNextFrame()) if (dualReceiverEnabledAndPassive &&
!_jitterBuffer.CompleteSequenceWithNextFrame())
{ {
// Jitter buffer state might get corrupt with this frame. // Jitter buffer state might get corrupt with this frame.
dualReceiver->CopyJitterBufferStateFromReceiver(*this); dualReceiver->CopyJitterBufferStateFromReceiver(*this);

View File

@ -15,7 +15,6 @@
#include "vie_channel.h" #include "vie_channel.h"
#include "vie_defines.h" #include "vie_defines.h"
#include "critical_section_wrapper.h" #include "critical_section_wrapper.h"
#include "event_wrapper.h"
#include "rtp_rtcp.h" #include "rtp_rtcp.h"
#include "udp_transport.h" #include "udp_transport.h"
#include "video_coding.h" #include "video_coding.h"
@ -37,7 +36,6 @@
namespace webrtc namespace webrtc
{ {
static const int kDecoderThreadWaitPeriod = 3; // wait 3 ms
// ---------------------------------------------------------------------------- // ----------------------------------------------------------------------------
// Constructor // Constructor
// ---------------------------------------------------------------------------- // ----------------------------------------------------------------------------
@ -77,7 +75,6 @@ ViEChannel::ViEChannel(WebRtc_Word32 channelId, WebRtc_Word32 engineId,
_decoderReset(true), _decoderReset(true),
_waitForKeyFrame(false), _waitForKeyFrame(false),
_ptrDecodeThread(NULL), _ptrDecodeThread(NULL),
_vieDecodeEvent(*EventWrapper::Create()),
_ptrSrtpModuleEncryption(NULL), _ptrSrtpModuleEncryption(NULL),
_ptrSrtpModuleDecryption(NULL), _ptrSrtpModuleDecryption(NULL),
_ptrExternalEncryption(NULL), _ptrExternalEncryption(NULL),
@ -262,13 +259,11 @@ ViEChannel::~ViEChannel()
RtpRtcp::DestroyRtpRtcp(rtpRtcp); RtpRtcp::DestroyRtpRtcp(rtpRtcp);
_simulcastRtpRtcp.erase(it); _simulcastRtpRtcp.erase(it);
} }
_vieDecodeEvent.Set();
if (_ptrDecodeThread) if (_ptrDecodeThread)
{ {
StopDecodeThread(); StopDecodeThread();
} }
delete &_vieDecodeEvent;
delete &_vieReceiver; delete &_vieReceiver;
delete &_vieSender; delete &_vieSender;
delete &_vieSync; delete &_vieSync;
@ -3095,11 +3090,7 @@ bool ViEChannel::ChannelDecodeThreadFunction(void* obj)
bool ViEChannel::ChannelDecodeProcess() bool ViEChannel::ChannelDecodeProcess()
{ {
// Decode is blocking, but sleep some time anyway to not get a spin // Decode is blocking, but sleep some time anyway to not get a spin
int ret = _vcm.Decode(50); _vcm.Decode(50);
if (ret == VCM_FRAME_NOT_READY) {
// TODO (mikhal): Add trigger by incoming packet.
_vieDecodeEvent.Wait(kDecoderThreadWaitPeriod);
}
if ((TickTime::Now() - _vcmRTTReported).Milliseconds() > 1000) if ((TickTime::Now() - _vcmRTTReported).Milliseconds() > 1000)
{ {

View File

@ -42,7 +42,6 @@ namespace webrtc
{ {
class CriticalSectionWrapper; class CriticalSectionWrapper;
class Encryption; class Encryption;
class EventWrapper;
class ProcessThread; class ProcessThread;
class RtpRtcp; class RtpRtcp;
class ThreadWrapper; class ThreadWrapper;
@ -472,7 +471,6 @@ private:
// Decoder // Decoder
ThreadWrapper* _ptrDecodeThread; ThreadWrapper* _ptrDecodeThread;
EventWrapper& _vieDecodeEvent;
//SRTP - using seperate pointers for encryption and decryption to support //SRTP - using seperate pointers for encryption and decryption to support
// simultaneous operations. // simultaneous operations.