From baf6db5ead76ee9591964b9f15ed3847b3a2055a Mon Sep 17 00:00:00 2001 From: "henrik.lundin@webrtc.org" Date: Wed, 2 Nov 2011 18:58:39 +0000 Subject: [PATCH] Making dual decoder work again in VCM Changing the assignment operator in VCMJitterBuffer to a named function (CopyFrom) instead, since it is not a straight assignment. Also fixing two bugs in the jitter copy function. Bug fix in VCMEncodedFrame: The copy constructor did not copy _length. In VCM codec database, make sure that the callback object is preserved when copying back from secondary to primary decoder. In VP8 wrapper, adding code to copy the _decodedImage to the Copy() method. Bugfix in video_coding_test rtp_player: The retransmissions where made in reverse order. Now new items are appended to the end of the LostPackets list, which makes the order correct when retransmitting. Handling the case when cloning an unused decoder state: When the decoder has not successfully decoded a frame yet, it cannot be cloned. A NULL pointer will be returned all the way out to VideoCodingModuleImpl::Decode(). When this happens, the VCM will call Reset() for the dual receiver, in order to reset the state to kPassive. Review URL: http://webrtc-codereview.appspot.com/239010 git-svn-id: http://webrtc.googlecode.com/svn/trunk@873 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../video_coding/codecs/vp8/main/source/vp8.cc | 10 ++++++++-- .../video_coding/main/source/codec_database.cc | 12 +++++++++++- .../video_coding/main/source/encoded_frame.cc | 1 + .../video_coding/main/source/jitter_buffer.cc | 10 ++++++---- .../video_coding/main/source/jitter_buffer.h | 4 +++- src/modules/video_coding/main/source/receiver.cc | 16 +++++++++++++--- src/modules/video_coding/main/source/receiver.h | 1 + .../main/source/video_coding_impl.cc | 4 ++++ src/modules/video_coding/main/test/rtp_player.cc | 2 +- 9 files changed, 48 insertions(+), 12 deletions(-) diff --git a/src/modules/video_coding/codecs/vp8/main/source/vp8.cc b/src/modules/video_coding/codecs/vp8/main/source/vp8.cc index 19fc83eb5e..f0a42d7af8 100644 --- a/src/modules/video_coding/codecs/vp8/main/source/vp8.cc +++ b/src/modules/video_coding/codecs/vp8/main/source/vp8.cc @@ -1162,13 +1162,11 @@ VP8Decoder::Copy() if (_decodedImage._buffer == NULL) { // Nothing has been decoded before; cannot clone. - assert(false); return NULL; } if (_lastKeyFrame._buffer == NULL) { // Cannot clone if we have no key frame to start with. - assert(false); return NULL; } @@ -1242,6 +1240,14 @@ VP8Decoder::Copy() memcpy(copyTo->_lastKeyFrame._buffer, _lastKeyFrame._buffer, _lastKeyFrame._length); + // Initialize _decodedImage. + copyTo->_decodedImage = _decodedImage; // Shallow copy + copyTo->_decodedImage._buffer = NULL; + if (_decodedImage._size) + { + copyTo->_decodedImage._buffer = new WebRtc_UWord8[_decodedImage._size]; + } + return static_cast(copyTo); } diff --git a/src/modules/video_coding/main/source/codec_database.cc b/src/modules/video_coding/main/source/codec_database.cc index 4583a0bb67..8aa9dd7814 100644 --- a/src/modules/video_coding/main/source/codec_database.cc +++ b/src/modules/video_coding/main/source/codec_database.cc @@ -9,6 +9,9 @@ */ #include "codec_database.h" + +#include + #include "../../../../engine_configurations.h" #include "internal_defines.h" #include "trace.h" @@ -694,8 +697,15 @@ VCMCodecDataBase::CopyDecoder(const VCMGenericDecoder& decoder) VideoDecoder* decoderCopy = decoder._decoder.Copy(); if (decoderCopy != NULL) { + VCMDecodedFrameCallback* cb = _ptrDecoder->_callback; ReleaseDecoder(_ptrDecoder); - _ptrDecoder = new VCMGenericDecoder(*decoderCopy, _id, decoder.External()); + _ptrDecoder = new VCMGenericDecoder(*decoderCopy, _id, + decoder.External()); + if (cb) + { + WebRtc_Word32 ret = _ptrDecoder->RegisterDecodeCompleteCallback(cb); + assert(ret == WEBRTC_VIDEO_CODEC_OK); + } } } diff --git a/src/modules/video_coding/main/source/encoded_frame.cc b/src/modules/video_coding/main/source/encoded_frame.cc index 6ed2855f8f..8913addaba 100644 --- a/src/modules/video_coding/main/source/encoded_frame.cc +++ b/src/modules/video_coding/main/source/encoded_frame.cc @@ -63,6 +63,7 @@ VCMEncodedFrame::VCMEncodedFrame(const VCMEncodedFrame& rhs) { VerifyAndAllocate(rhs._size); memcpy(_buffer, rhs._buffer, rhs._length); + _length = rhs._length; } // Deep operator= _fragmentation = rhs._fragmentation; diff --git a/src/modules/video_coding/main/source/jitter_buffer.cc b/src/modules/video_coding/main/source/jitter_buffer.cc index b855fcba00..aa894a70ff 100644 --- a/src/modules/video_coding/main/source/jitter_buffer.cc +++ b/src/modules/video_coding/main/source/jitter_buffer.cc @@ -115,8 +115,8 @@ VCMJitterBuffer::~VCMJitterBuffer() delete &_critSect; } -VCMJitterBuffer& -VCMJitterBuffer::operator=(const VCMJitterBuffer& rhs) +void +VCMJitterBuffer::CopyFrom(const VCMJitterBuffer& rhs) { if (this != &rhs) { @@ -138,7 +138,6 @@ VCMJitterBuffer::operator=(const VCMJitterBuffer& rhs) _jitterEstimate = rhs._jitterEstimate; _delayEstimate = rhs._delayEstimate; _waitingForCompletion = rhs._waitingForCompletion; - _nackMode = rhs._nackMode; _rttMs = rhs._rttMs; _NACKSeqNumLength = rhs._NACKSeqNumLength; _waitingForKeyFrame = rhs._waitingForKeyFrame; @@ -173,7 +172,6 @@ VCMJitterBuffer::operator=(const VCMJitterBuffer& rhs) rhs._critSect.Leave(); _critSect.Leave(); } - return *this; } // Start jitter buffer @@ -971,6 +969,10 @@ VCMJitterBuffer::CompleteSequenceWithNextFrame() // Frame not ready to be decoded. return true; } + if (!oldestFrame->Complete()) + { + return false; + } // See if we have lost a frame before this one. if (_lastDecodedSeqNum == -1) diff --git a/src/modules/video_coding/main/source/jitter_buffer.h b/src/modules/video_coding/main/source/jitter_buffer.h index 7e97e6bfeb..225e76a320 100644 --- a/src/modules/video_coding/main/source/jitter_buffer.h +++ b/src/modules/video_coding/main/source/jitter_buffer.h @@ -53,7 +53,7 @@ public: bool master = true); virtual ~VCMJitterBuffer(); - VCMJitterBuffer& operator=(const VCMJitterBuffer& rhs); + void CopyFrom(const VCMJitterBuffer& rhs); // We need a start and stop to break out of the wait event // used in GetCompleteFrameForDecoding @@ -241,6 +241,8 @@ private: bool _waitingForKeyFrame; bool _firstPacket; + + DISALLOW_COPY_AND_ASSIGN(VCMJitterBuffer); }; } // namespace webrtc diff --git a/src/modules/video_coding/main/source/receiver.cc b/src/modules/video_coding/main/source/receiver.cc index 10227f053f..9137c8912d 100644 --- a/src/modules/video_coding/main/source/receiver.cc +++ b/src/modules/video_coding/main/source/receiver.cc @@ -42,8 +42,8 @@ VCMReceiver::~VCMReceiver() delete &_critSect; } -WebRtc_Word32 -VCMReceiver::Initialize() +void +VCMReceiver::Reset() { CriticalSectionScoped cs(_critSect); if (!_jitterBuffer.Running()) @@ -62,6 +62,16 @@ VCMReceiver::Initialize() else { _state = kPassive; + } +} + +WebRtc_Word32 +VCMReceiver::Initialize() +{ + CriticalSectionScoped cs(_critSect); + Reset(); + if (!_master) + { SetNackMode(kNoNack); } return VCM_OK; @@ -416,7 +426,7 @@ VCMReceiver::DualDecoderCaughtUp(VCMEncodedFrame* dualFrame, VCMReceiver& dualRe void VCMReceiver::CopyJitterBufferStateFromReceiver(const VCMReceiver& receiver) { - _jitterBuffer = receiver._jitterBuffer; + _jitterBuffer.CopyFrom(receiver._jitterBuffer); } VCMReceiverState diff --git a/src/modules/video_coding/main/source/receiver.h b/src/modules/video_coding/main/source/receiver.h index 6373e64664..3f8480af4c 100644 --- a/src/modules/video_coding/main/source/receiver.h +++ b/src/modules/video_coding/main/source/receiver.h @@ -45,6 +45,7 @@ public: bool master = true); ~VCMReceiver(); + void Reset(); WebRtc_Word32 Initialize(); void UpdateRtt(WebRtc_UWord32 rtt); WebRtc_Word32 InsertPacket(const VCMPacket& packet, diff --git a/src/modules/video_coding/main/source/video_coding_impl.cc b/src/modules/video_coding/main/source/video_coding_impl.cc index fb8184c16f..c642730607 100644 --- a/src/modules/video_coding/main/source/video_coding_impl.cc +++ b/src/modules/video_coding/main/source/video_coding_impl.cc @@ -1071,6 +1071,10 @@ VideoCodingModuleImpl::Decode(WebRtc_UWord16 maxWaitTimeMs) _dualDecoder->RegisterDecodeCompleteCallback( &_dualDecodedFrameCallback); } + else + { + _dualReceiver.Reset(); + } } if (frame == NULL) diff --git a/src/modules/video_coding/main/test/rtp_player.cc b/src/modules/video_coding/main/test/rtp_player.cc index fb4ea36b69..797e272d65 100644 --- a/src/modules/video_coding/main/test/rtp_player.cc +++ b/src/modules/video_coding/main/test/rtp_player.cc @@ -70,7 +70,7 @@ WebRtc_UWord32 LostPackets::AddPacket(WebRtc_UWord8* rtpData, WebRtc_UWord16 rtp CriticalSectionScoped cs(_critSect); RawRtpPacket* packet = new RawRtpPacket(rtpData, rtpLen); ListItem* newItem = new ListItem(packet); - InsertBefore(First(), newItem); + Insert(Last(), newItem); const WebRtc_UWord16 seqNo = (rtpData[2] << 8) + rtpData[3]; if (_debugFile != NULL) {