From 14f28ebb0e988e508784065b4666e6b8ca997321 Mon Sep 17 00:00:00 2001 From: "kwiberg@webrtc.org" Date: Tue, 4 Nov 2014 13:23:36 +0000 Subject: Remove the useless dummy state parameter to WebRtcG711_* R=henrik.lundin@webrtc.org Review URL: https://webrtc-codereview.appspot.com/27029004 git-svn-id: http://webrtc.googlecode.com/svn/trunk/webrtc@7609 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../audio_coding/codecs/g711/audio_encoder_pcm.cc | 6 ++--- modules/audio_coding/codecs/g711/g711_interface.c | 28 ++++------------------ .../codecs/g711/include/g711_interface.h | 25 ++++--------------- modules/audio_coding/codecs/g711/test/testG711.cc | 9 ++++--- modules/audio_coding/main/acm2/acm_pcma.cc | 2 +- modules/audio_coding/main/acm2/acm_pcmu.cc | 2 +- modules/audio_coding/neteq/audio_decoder_impl.cc | 4 ++-- modules/audio_coding/neteq/test/RTPencode.cc | 7 ++---- 8 files changed, 22 insertions(+), 61 deletions(-) diff --git a/modules/audio_coding/codecs/g711/audio_encoder_pcm.cc b/modules/audio_coding/codecs/g711/audio_encoder_pcm.cc index 097e11f1..6454e93c 100644 --- a/modules/audio_coding/codecs/g711/audio_encoder_pcm.cc +++ b/modules/audio_coding/codecs/g711/audio_encoder_pcm.cc @@ -82,8 +82,7 @@ bool AudioEncoderPcm::Encode(uint32_t timestamp, int16_t AudioEncoderPcmA::EncodeCall(const int16_t* audio, size_t input_len, uint8_t* encoded) { - return WebRtcG711_EncodeA(NULL, - const_cast(audio), + return WebRtcG711_EncodeA(const_cast(audio), static_cast(input_len), reinterpret_cast(encoded)); } @@ -91,8 +90,7 @@ int16_t AudioEncoderPcmA::EncodeCall(const int16_t* audio, int16_t AudioEncoderPcmU::EncodeCall(const int16_t* audio, size_t input_len, uint8_t* encoded) { - return WebRtcG711_EncodeU(NULL, - const_cast(audio), + return WebRtcG711_EncodeU(const_cast(audio), static_cast(input_len), reinterpret_cast(encoded)); } diff --git a/modules/audio_coding/codecs/g711/g711_interface.c b/modules/audio_coding/codecs/g711/g711_interface.c index 134c1e4e..ec726c51 100644 --- a/modules/audio_coding/codecs/g711/g711_interface.c +++ b/modules/audio_coding/codecs/g711/g711_interface.c @@ -12,16 +12,12 @@ #include "g711_interface.h" #include "webrtc/typedefs.h" -int16_t WebRtcG711_EncodeA(void* state, - int16_t* speechIn, +int16_t WebRtcG711_EncodeA(int16_t* speechIn, int16_t len, int16_t* encoded) { int n; uint16_t tempVal, tempVal2; - // Set and discard to avoid getting warnings - (void)(state = NULL); - // Sanity check of input length if (len < 0) { return (-1); @@ -50,16 +46,12 @@ int16_t WebRtcG711_EncodeA(void* state, return (len); } -int16_t WebRtcG711_EncodeU(void* state, - int16_t* speechIn, +int16_t WebRtcG711_EncodeU(int16_t* speechIn, int16_t len, int16_t* encoded) { int n; uint16_t tempVal; - // Set and discard to avoid getting warnings - (void)(state = NULL); - // Sanity check of input length if (len < 0) { return (-1); @@ -86,17 +78,13 @@ int16_t WebRtcG711_EncodeU(void* state, return (len); } -int16_t WebRtcG711_DecodeA(void* state, - int16_t* encoded, +int16_t WebRtcG711_DecodeA(int16_t* encoded, int16_t len, int16_t* decoded, int16_t* speechType) { int n; uint16_t tempVal; - // Set and discard to avoid getting warnings - (void)(state = NULL); - // Sanity check of input length if (len < 0) { return (-1); @@ -123,17 +111,13 @@ int16_t WebRtcG711_DecodeA(void* state, return (len); } -int16_t WebRtcG711_DecodeU(void* state, - int16_t* encoded, +int16_t WebRtcG711_DecodeU(int16_t* encoded, int16_t len, int16_t* decoded, int16_t* speechType) { int n; uint16_t tempVal; - // Set and discard to avoid getting warnings - (void)(state = NULL); - // Sanity check of input length if (len < 0) { return (-1); @@ -160,10 +144,8 @@ int16_t WebRtcG711_DecodeU(void* state, return (len); } -int WebRtcG711_DurationEst(void* state, - const uint8_t* payload, +int WebRtcG711_DurationEst(const uint8_t* payload, int payload_length_bytes) { - (void) state; (void) payload; /* G.711 is one byte per sample, so we can just return the number of bytes. */ return payload_length_bytes; diff --git a/modules/audio_coding/codecs/g711/include/g711_interface.h b/modules/audio_coding/codecs/g711/include/g711_interface.h index 83357e47..545ca3e8 100644 --- a/modules/audio_coding/codecs/g711/include/g711_interface.h +++ b/modules/audio_coding/codecs/g711/include/g711_interface.h @@ -28,8 +28,6 @@ extern "C" { * Input speech length has be of any length. * * Input: - * - state : Dummy state to make this codec look more like - * other codecs * - speechIn : Input speech vector * - len : Samples in speechIn * @@ -40,8 +38,7 @@ extern "C" { * -1 - Error */ -int16_t WebRtcG711_EncodeA(void* state, - int16_t* speechIn, +int16_t WebRtcG711_EncodeA(int16_t* speechIn, int16_t len, int16_t* encoded); @@ -52,8 +49,6 @@ int16_t WebRtcG711_EncodeA(void* state, * Input speech length has be of any length. * * Input: - * - state : Dummy state to make this codec look more like - * other codecs * - speechIn : Input speech vector * - len : Samples in speechIn * @@ -64,8 +59,7 @@ int16_t WebRtcG711_EncodeA(void* state, * -1 - Error */ -int16_t WebRtcG711_EncodeU(void* state, - int16_t* speechIn, +int16_t WebRtcG711_EncodeU(int16_t* speechIn, int16_t len, int16_t* encoded); @@ -75,8 +69,6 @@ int16_t WebRtcG711_EncodeU(void* state, * This function decodes a packet G711 A-law frame. * * Input: - * - state : Dummy state to make this codec look more like - * other codecs * - encoded : Encoded data * - len : Bytes in encoded vector * @@ -90,8 +82,7 @@ int16_t WebRtcG711_EncodeU(void* state, * -1 - Error */ -int16_t WebRtcG711_DecodeA(void* state, - int16_t* encoded, +int16_t WebRtcG711_DecodeA(int16_t* encoded, int16_t len, int16_t* decoded, int16_t* speechType); @@ -102,8 +93,6 @@ int16_t WebRtcG711_DecodeA(void* state, * This function decodes a packet G711 U-law frame. * * Input: - * - state : Dummy state to make this codec look more like - * other codecs * - encoded : Encoded data * - len : Bytes in encoded vector * @@ -117,8 +106,7 @@ int16_t WebRtcG711_DecodeA(void* state, * -1 - Error */ -int16_t WebRtcG711_DecodeU(void* state, - int16_t* encoded, +int16_t WebRtcG711_DecodeU(int16_t* encoded, int16_t len, int16_t* decoded, int16_t* speechType); @@ -129,8 +117,6 @@ int16_t WebRtcG711_DecodeU(void* state, * This function estimates the duration of a G711 packet in samples. * * Input: - * - state : Dummy state to make this codec look more like - * other codecs * - payload : Encoded data * - payloadLengthBytes : Bytes in encoded vector * @@ -139,8 +125,7 @@ int16_t WebRtcG711_DecodeU(void* state, * byte per sample. */ -int WebRtcG711_DurationEst(void* state, - const uint8_t* payload, +int WebRtcG711_DurationEst(const uint8_t* payload, int payload_length_bytes); /********************************************************************** diff --git a/modules/audio_coding/codecs/g711/test/testG711.cc b/modules/audio_coding/codecs/g711/test/testG711.cc index 95a02469..76950fa3 100644 --- a/modules/audio_coding/codecs/g711/test/testG711.cc +++ b/modules/audio_coding/codecs/g711/test/testG711.cc @@ -127,7 +127,7 @@ int main(int argc, char* argv[]) { /* G.711 encoding */ if (!strcmp(law, "A")) { /* A-law encoding */ - stream_len = WebRtcG711_EncodeA(NULL, shortdata, framelength, streamdata); + stream_len = WebRtcG711_EncodeA(shortdata, framelength, streamdata); if (argc == 6) { /* Write bits to file */ if (fwrite(streamdata, sizeof(unsigned char), stream_len, bitp) != @@ -135,11 +135,11 @@ int main(int argc, char* argv[]) { return -1; } } - err = WebRtcG711_DecodeA(NULL, streamdata, stream_len, decoded, + err = WebRtcG711_DecodeA(streamdata, stream_len, decoded, speechType); } else if (!strcmp(law, "u")) { /* u-law encoding */ - stream_len = WebRtcG711_EncodeU(NULL, shortdata, framelength, streamdata); + stream_len = WebRtcG711_EncodeU(shortdata, framelength, streamdata); if (argc == 6) { /* Write bits to file */ if (fwrite(streamdata, sizeof(unsigned char), stream_len, bitp) != @@ -147,8 +147,7 @@ int main(int argc, char* argv[]) { return -1; } } - err = WebRtcG711_DecodeU(NULL, streamdata, stream_len, decoded, - speechType); + err = WebRtcG711_DecodeU(streamdata, stream_len, decoded, speechType); } else { printf("Wrong law mode\n"); exit(1); diff --git a/modules/audio_coding/main/acm2/acm_pcma.cc b/modules/audio_coding/main/acm2/acm_pcma.cc index 548e8fda..41d4d085 100644 --- a/modules/audio_coding/main/acm2/acm_pcma.cc +++ b/modules/audio_coding/main/acm2/acm_pcma.cc @@ -27,7 +27,7 @@ ACMPCMA::~ACMPCMA() { return; } int16_t ACMPCMA::InternalEncode(uint8_t* bitstream, int16_t* bitstream_len_byte) { *bitstream_len_byte = WebRtcG711_EncodeA( - NULL, &in_audio_[in_audio_ix_read_], frame_len_smpl_ * num_channels_, + &in_audio_[in_audio_ix_read_], frame_len_smpl_ * num_channels_, reinterpret_cast(bitstream)); // Increment the read index this tell the caller that how far // we have gone forward in reading the audio buffer. diff --git a/modules/audio_coding/main/acm2/acm_pcmu.cc b/modules/audio_coding/main/acm2/acm_pcmu.cc index 5c032363..4f16062f 100644 --- a/modules/audio_coding/main/acm2/acm_pcmu.cc +++ b/modules/audio_coding/main/acm2/acm_pcmu.cc @@ -27,7 +27,7 @@ ACMPCMU::~ACMPCMU() {} int16_t ACMPCMU::InternalEncode(uint8_t* bitstream, int16_t* bitstream_len_byte) { *bitstream_len_byte = WebRtcG711_EncodeU( - NULL, &in_audio_[in_audio_ix_read_], frame_len_smpl_ * num_channels_, + &in_audio_[in_audio_ix_read_], frame_len_smpl_ * num_channels_, reinterpret_cast(bitstream)); // Increment the read index this tell the caller that how far diff --git a/modules/audio_coding/neteq/audio_decoder_impl.cc b/modules/audio_coding/neteq/audio_decoder_impl.cc index 0215f36d..c3f2f0b1 100644 --- a/modules/audio_coding/neteq/audio_decoder_impl.cc +++ b/modules/audio_coding/neteq/audio_decoder_impl.cc @@ -45,7 +45,7 @@ int AudioDecoderPcmU::Decode(const uint8_t* encoded, size_t encoded_len, int16_t* decoded, SpeechType* speech_type) { int16_t temp_type = 1; // Default is speech. int16_t ret = WebRtcG711_DecodeU( - state_, reinterpret_cast(const_cast(encoded)), + reinterpret_cast(const_cast(encoded)), static_cast(encoded_len), decoded, &temp_type); *speech_type = ConvertSpeechType(temp_type); return ret; @@ -62,7 +62,7 @@ int AudioDecoderPcmA::Decode(const uint8_t* encoded, size_t encoded_len, int16_t* decoded, SpeechType* speech_type) { int16_t temp_type = 1; // Default is speech. int16_t ret = WebRtcG711_DecodeA( - state_, reinterpret_cast(const_cast(encoded)), + reinterpret_cast(const_cast(encoded)), static_cast(encoded_len), decoded, &temp_type); *speech_type = ConvertSpeechType(temp_type); return ret; diff --git a/modules/audio_coding/neteq/test/RTPencode.cc b/modules/audio_coding/neteq/test/RTPencode.cc index ab338a73..b73e70e5 100644 --- a/modules/audio_coding/neteq/test/RTPencode.cc +++ b/modules/audio_coding/neteq/test/RTPencode.cc @@ -235,9 +235,6 @@ WebRtcVadInst *VAD_inst[2]; #ifdef CODEC_CELT_32 CELT_encinst_t *CELT32enc_inst[2]; #endif -#ifdef CODEC_G711 - void *G711state[2]={NULL, NULL}; -#endif int main(int argc, char* argv[]) @@ -1602,12 +1599,12 @@ int NetEQTest_encode(int coder, int16_t *indata, int frameLen, unsigned char * e /* Encode with the selected coder type */ if (coder==webrtc::kDecoderPCMu) { /*g711 u-law */ #ifdef CODEC_G711 - cdlen = WebRtcG711_EncodeU(G711state[k], indata, frameLen, (int16_t*) encoded); + cdlen = WebRtcG711_EncodeU(indata, frameLen, (int16_t*) encoded); #endif } else if (coder==webrtc::kDecoderPCMa) { /*g711 A-law */ #ifdef CODEC_G711 - cdlen = WebRtcG711_EncodeA(G711state[k], indata, frameLen, (int16_t*) encoded); + cdlen = WebRtcG711_EncodeA(indata, frameLen, (int16_t*) encoded); } #endif #ifdef CODEC_PCM16B -- cgit v1.2.3 From ab22837337733a450afbb8352c337bd5607c948e Mon Sep 17 00:00:00 2001 From: "kwiberg@webrtc.org" Date: Tue, 4 Nov 2014 13:29:24 +0000 Subject: Remove the useless dummy state parameter to WebRtcPcm16b_DecodeW16 R=henrik.lundin@webrtc.org Review URL: https://webrtc-codereview.appspot.com/26039004 git-svn-id: http://webrtc.googlecode.com/svn/trunk/webrtc@7610 4adac7df-926f-26a2-2b94-8c16560cd09d --- modules/audio_coding/codecs/pcm16b/include/pcm16b.h | 3 +-- modules/audio_coding/codecs/pcm16b/pcm16b.c | 6 +----- modules/audio_coding/neteq/audio_decoder_impl.cc | 2 +- modules/audio_coding/neteq/mock/mock_external_decoder_pcm16b.h | 2 +- 4 files changed, 4 insertions(+), 9 deletions(-) diff --git a/modules/audio_coding/codecs/pcm16b/include/pcm16b.h b/modules/audio_coding/codecs/pcm16b/include/pcm16b.h index 9c96b830..86b32fe2 100644 --- a/modules/audio_coding/codecs/pcm16b/include/pcm16b.h +++ b/modules/audio_coding/codecs/pcm16b/include/pcm16b.h @@ -73,8 +73,7 @@ int16_t WebRtcPcm16b_Encode(int16_t *speech16b, * Returned value : Samples in speechOut16b */ -int16_t WebRtcPcm16b_DecodeW16(void *inst, - int16_t *speechIn16b, +int16_t WebRtcPcm16b_DecodeW16(int16_t *speechIn16b, int16_t length_bytes, int16_t *speechOut16b, int16_t* speechType); diff --git a/modules/audio_coding/codecs/pcm16b/pcm16b.c b/modules/audio_coding/codecs/pcm16b/pcm16b.c index af6720f6..2c6bea6a 100644 --- a/modules/audio_coding/codecs/pcm16b/pcm16b.c +++ b/modules/audio_coding/codecs/pcm16b/pcm16b.c @@ -61,8 +61,7 @@ int16_t WebRtcPcm16b_Encode(int16_t *speech16b, /* Decoder with int16_t Input instead of char when the int16_t Encoder is used */ -int16_t WebRtcPcm16b_DecodeW16(void *inst, - int16_t *speechIn16b, +int16_t WebRtcPcm16b_DecodeW16(int16_t *speechIn16b, int16_t length_bytes, int16_t *speechOut16b, int16_t* speechType) @@ -80,9 +79,6 @@ int16_t WebRtcPcm16b_DecodeW16(void *inst, *speechType=1; - // Avoid warning. - (void)(inst = NULL); - return length_bytes >> 1; } diff --git a/modules/audio_coding/neteq/audio_decoder_impl.cc b/modules/audio_coding/neteq/audio_decoder_impl.cc index c3f2f0b1..07b1b4be 100644 --- a/modules/audio_coding/neteq/audio_decoder_impl.cc +++ b/modules/audio_coding/neteq/audio_decoder_impl.cc @@ -82,7 +82,7 @@ int AudioDecoderPcm16B::Decode(const uint8_t* encoded, size_t encoded_len, int16_t* decoded, SpeechType* speech_type) { int16_t temp_type = 1; // Default is speech. int16_t ret = WebRtcPcm16b_DecodeW16( - state_, reinterpret_cast(const_cast(encoded)), + reinterpret_cast(const_cast(encoded)), static_cast(encoded_len), decoded, &temp_type); *speech_type = ConvertSpeechType(temp_type); return ret; diff --git a/modules/audio_coding/neteq/mock/mock_external_decoder_pcm16b.h b/modules/audio_coding/neteq/mock/mock_external_decoder_pcm16b.h index c15fa1ac..400c0b03 100644 --- a/modules/audio_coding/neteq/mock/mock_external_decoder_pcm16b.h +++ b/modules/audio_coding/neteq/mock/mock_external_decoder_pcm16b.h @@ -33,7 +33,7 @@ class ExternalPcm16B : public AudioDecoder { int16_t* decoded, SpeechType* speech_type) { int16_t temp_type; int16_t ret = WebRtcPcm16b_DecodeW16( - state_, reinterpret_cast(const_cast(encoded)), + reinterpret_cast(const_cast(encoded)), static_cast(encoded_len), decoded, &temp_type); *speech_type = ConvertSpeechType(temp_type); return ret; -- cgit v1.2.3 From e765eef08a12e6e7808a76d8c1d5b1e58c9b29f5 Mon Sep 17 00:00:00 2001 From: "pbos@webrtc.org" Date: Tue, 4 Nov 2014 13:48:15 +0000 Subject: Delete VideoReceiveStream channels in destructor. R=stefan@webrtc.org BUG=1667 Review URL: https://webrtc-codereview.appspot.com/31909004 git-svn-id: http://webrtc.googlecode.com/svn/trunk/webrtc@7611 4adac7df-926f-26a2-2b94-8c16560cd09d --- video/end_to_end_tests.cc | 35 +++++++++++++++++++++++++++++++++++ video/video_receive_stream.cc | 3 ++- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/video/end_to_end_tests.cc b/video/end_to_end_tests.cc index 06cb187c..96249c3c 100644 --- a/video/end_to_end_tests.cc +++ b/video/end_to_end_tests.cc @@ -2140,4 +2140,39 @@ TEST_F(EndToEndTest, NewReceiveStreamsRespectNetworkDown) { DestroyStreams(); } + +// TODO(pbos): Remove this regression test when VideoEngine is no longer used as +// a backend. This is to test that we hand channels back properly. +TEST_F(EndToEndTest, CanCreateAndDestroyManyVideoStreams) { + test::NullTransport transport; + scoped_ptr call(Call::Create(Call::Config(&transport))); + test::FakeDecoder fake_decoder; + test::FakeEncoder fake_encoder(Clock::GetRealTimeClock()); + for (size_t i = 0; i < 100; ++i) { + VideoSendStream::Config send_config; + send_config.encoder_settings.encoder = &fake_encoder; + send_config.encoder_settings.payload_name = "FAKE"; + send_config.encoder_settings.payload_type = 123; + + VideoEncoderConfig encoder_config; + encoder_config.streams = test::CreateVideoStreams(1); + send_config.rtp.ssrcs.push_back(1); + VideoSendStream* send_stream = + call->CreateVideoSendStream(send_config, encoder_config); + call->DestroyVideoSendStream(send_stream); + + VideoReceiveStream::Config receive_config; + receive_config.rtp.remote_ssrc = 1; + receive_config.rtp.local_ssrc = kReceiverLocalSsrc; + VideoReceiveStream::Decoder decoder; + decoder.decoder = &fake_decoder; + decoder.payload_type = 123; + decoder.payload_name = "FAKE"; + receive_config.decoders.push_back(decoder); + VideoReceiveStream* receive_stream = + call->CreateVideoReceiveStream(receive_config); + call->DestroyVideoReceiveStream(receive_stream); + } +} + } // namespace webrtc diff --git a/video/video_receive_stream.cc b/video/video_receive_stream.cc index b1822402..5b085bb8 100644 --- a/video/video_receive_stream.cc +++ b/video/video_receive_stream.cc @@ -222,7 +222,6 @@ VideoReceiveStream::~VideoReceiveStream() { video_engine_base_->SetVoiceEngine(NULL); image_process_->Release(); - video_engine_base_->Release(); external_codec_->Release(); codec_->DeregisterDecoderObserver(channel_); rtp_rtcp_->DeregisterReceiveChannelRtpStatisticsCallback(channel_, @@ -233,6 +232,8 @@ VideoReceiveStream::~VideoReceiveStream() { network_->Release(); render_->Release(); rtp_rtcp_->Release(); + video_engine_base_->DeleteChannel(channel_); + video_engine_base_->Release(); } void VideoReceiveStream::Start() { -- cgit v1.2.3 From 95e0f61a2bf27af6156ee639fbcad1f9794a3594 Mon Sep 17 00:00:00 2001 From: "henrik.lundin@webrtc.org" Date: Tue, 4 Nov 2014 14:03:58 +0000 Subject: Fix problem with late packets in NetEq Since r7255, it could happen that an old packet would block the decoding process until enough packet was received for the buffer to flush. This CL fixes that by: - Partially reverting r7255; - Remove recent old packets before taking a decision for GetAudio; - Remove all old packets after a packet has been extracted for decoding; - Adding tests for reordered packets. BUG=chrome:423985 R=tina.legrand@webrtc.org Review URL: https://webrtc-codereview.appspot.com/25079004 git-svn-id: http://webrtc.googlecode.com/svn/trunk/webrtc@7612 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../audio_coding/neteq/decision_logic_normal.cc | 10 +-- .../audio_coding/neteq/mock/mock_packet_buffer.h | 4 +- .../neteq/neteq_external_decoder_unittest.cc | 13 ++- modules/audio_coding/neteq/neteq_impl.cc | 8 +- modules/audio_coding/neteq/neteq_impl_unittest.cc | 93 ++++++++++++++++++++++ modules/audio_coding/neteq/packet_buffer.cc | 12 +-- modules/audio_coding/neteq/packet_buffer.h | 28 ++++++- .../audio_coding/neteq/packet_buffer_unittest.cc | 59 +++++++++++++- 8 files changed, 207 insertions(+), 20 deletions(-) diff --git a/modules/audio_coding/neteq/decision_logic_normal.cc b/modules/audio_coding/neteq/decision_logic_normal.cc index 9e422041..f2382845 100644 --- a/modules/audio_coding/neteq/decision_logic_normal.cc +++ b/modules/audio_coding/neteq/decision_logic_normal.cc @@ -67,19 +67,19 @@ Operations DecisionLogicNormal::GetDecisionSpecialized( return kNormal; } + const uint32_t five_seconds_samples = 5 * 8000 * fs_mult_; // Check if the required packet is available. if (target_timestamp == available_timestamp) { return ExpectedPacketAvailable(prev_mode, play_dtmf); - } else if (IsNewerTimestamp(available_timestamp, target_timestamp)) { + } else if (!PacketBuffer::IsObsoleteTimestamp( + available_timestamp, target_timestamp, five_seconds_samples)) { return FuturePacketAvailable(sync_buffer, expand, decoder_frame_length, prev_mode, target_timestamp, available_timestamp, play_dtmf); } else { // This implies that available_timestamp < target_timestamp, which can - // happen when a new stream or codec is received. Do Expand instead, and - // wait for a newer packet to arrive, or for the buffer to flush (resulting - // in a master reset). - return kExpand; + // happen when a new stream or codec is received. Signal for a reset. + return kUndefined; } } diff --git a/modules/audio_coding/neteq/mock/mock_packet_buffer.h b/modules/audio_coding/neteq/mock/mock_packet_buffer.h index 74eea6f0..0eb7edc9 100644 --- a/modules/audio_coding/neteq/mock/mock_packet_buffer.h +++ b/modules/audio_coding/neteq/mock/mock_packet_buffer.h @@ -44,7 +44,9 @@ class MockPacketBuffer : public PacketBuffer { Packet*(int* discard_count)); MOCK_METHOD0(DiscardNextPacket, int()); - MOCK_METHOD1(DiscardOldPackets, + MOCK_METHOD2(DiscardOldPackets, + int(uint32_t timestamp_limit, uint32_t horizon_samples)); + MOCK_METHOD1(DiscardAllOldPackets, int(uint32_t timestamp_limit)); MOCK_CONST_METHOD0(NumPacketsInBuffer, int()); diff --git a/modules/audio_coding/neteq/neteq_external_decoder_unittest.cc b/modules/audio_coding/neteq/neteq_external_decoder_unittest.cc index 2e07b490..d41bc543 100644 --- a/modules/audio_coding/neteq/neteq_external_decoder_unittest.cc +++ b/modules/audio_coding/neteq/neteq_external_decoder_unittest.cc @@ -308,6 +308,8 @@ class LargeTimestampJumpTest : public NetEqExternalDecoderTest { case kExpandPhase: { if (output_type == kOutputPLCtoCNG) { test_state_ = kFadedExpandPhase; + } else if (output_type == kOutputNormal) { + test_state_ = kRecovered; } break; } @@ -337,9 +339,14 @@ class LargeTimestampJumpTest : public NetEqExternalDecoderTest { } int NumExpectedDecodeCalls(int num_loops) const OVERRIDE { - // Some packets won't be decoded because of the buffer being flushed after - // the timestamp jump. - return num_loops - (config_.max_packets_in_buffer + 1); + // Some packets at the end of the stream won't be decoded. When the jump in + // timestamp happens, NetEq will do Expand during one GetAudio call. In the + // next call it will decode the packet after the jump, but the net result is + // that the delay increased by 1 packet. In another call, a Pre-emptive + // Expand operation is performed, leading to delay increase by 1 packet. In + // total, the test will end with a 2-packet delay, which results in the 2 + // last packets not being decoded. + return num_loops - 2; } TestStates test_state_; diff --git a/modules/audio_coding/neteq/neteq_impl.cc b/modules/audio_coding/neteq/neteq_impl.cc index 44faa22a..f3d1a4f6 100644 --- a/modules/audio_coding/neteq/neteq_impl.cc +++ b/modules/audio_coding/neteq/neteq_impl.cc @@ -868,6 +868,10 @@ int NetEqImpl::GetDecision(Operations* operation, assert(sync_buffer_.get()); uint32_t end_timestamp = sync_buffer_->end_timestamp(); + if (!new_codec_) { + const uint32_t five_seconds_samples = 5 * fs_hz_; + packet_buffer_->DiscardOldPackets(end_timestamp, five_seconds_samples); + } const RTPHeader* header = packet_buffer_->NextRtpHeader(); if (decision_logic_->CngRfc3389On() || last_mode_ == kModeRfc3389Cng) { @@ -884,7 +888,7 @@ int NetEqImpl::GetDecision(Operations* operation, } // Check buffer again. if (!new_codec_) { - packet_buffer_->DiscardOldPackets(end_timestamp); + packet_buffer_->DiscardOldPackets(end_timestamp, 5 * fs_hz_); } header = packet_buffer_->NextRtpHeader(); } @@ -1823,7 +1827,7 @@ int NetEqImpl::ExtractPackets(int required_samples, PacketList* packet_list) { // we could end up in the situation where we never decode anything, since // all incoming packets are considered too old but the buffer will also // never be flooded and flushed. - packet_buffer_->DiscardOldPackets(timestamp_); + packet_buffer_->DiscardAllOldPackets(timestamp_); } return extracted_samples; diff --git a/modules/audio_coding/neteq/neteq_impl_unittest.cc b/modules/audio_coding/neteq/neteq_impl_unittest.cc index b3bd69ba..8047fe20 100644 --- a/modules/audio_coding/neteq/neteq_impl_unittest.cc +++ b/modules/audio_coding/neteq/neteq_impl_unittest.cc @@ -32,9 +32,11 @@ using ::testing::Return; using ::testing::ReturnNull; using ::testing::_; using ::testing::SetArgPointee; +using ::testing::SetArrayArgument; using ::testing::InSequence; using ::testing::Invoke; using ::testing::WithArg; +using ::testing::Pointee; namespace webrtc { @@ -494,4 +496,95 @@ TEST_F(NetEqImplTest, VerifyTimestampPropagation) { static_cast(sync_buffer->FutureLength())); } +TEST_F(NetEqImplTest, ReorderedPacket) { + UseNoMocks(); + CreateInstance(); + + const uint8_t kPayloadType = 17; // Just an arbitrary number. + const uint32_t kReceiveTime = 17; // Value doesn't matter for this test. + const int kSampleRateHz = 8000; + const int kPayloadLengthSamples = 10 * kSampleRateHz / 1000; // 10 ms. + const size_t kPayloadLengthBytes = kPayloadLengthSamples; + uint8_t payload[kPayloadLengthBytes] = {0}; + WebRtcRTPHeader rtp_header; + rtp_header.header.payloadType = kPayloadType; + rtp_header.header.sequenceNumber = 0x1234; + rtp_header.header.timestamp = 0x12345678; + rtp_header.header.ssrc = 0x87654321; + + // Create a mock decoder object. + MockAudioDecoder mock_decoder; + EXPECT_CALL(mock_decoder, Init()).WillRepeatedly(Return(0)); + EXPECT_CALL(mock_decoder, IncomingPacket(_, kPayloadLengthBytes, _, _, _)) + .WillRepeatedly(Return(0)); + int16_t dummy_output[kPayloadLengthSamples] = {0}; + // The below expectation will make the mock decoder write + // |kPayloadLengthSamples| zeros to the output array, and mark it as speech. + EXPECT_CALL(mock_decoder, Decode(Pointee(0), kPayloadLengthBytes, _, _)) + .WillOnce(DoAll(SetArrayArgument<2>(dummy_output, + dummy_output + kPayloadLengthSamples), + SetArgPointee<3>(AudioDecoder::kSpeech), + Return(kPayloadLengthSamples))); + EXPECT_EQ(NetEq::kOK, + neteq_->RegisterExternalDecoder( + &mock_decoder, kDecoderPCM16B, kPayloadType)); + + // Insert one packet. + EXPECT_EQ(NetEq::kOK, + neteq_->InsertPacket( + rtp_header, payload, kPayloadLengthBytes, kReceiveTime)); + + // Pull audio once. + const int kMaxOutputSize = 10 * kSampleRateHz / 1000; + int16_t output[kMaxOutputSize]; + int samples_per_channel; + int num_channels; + NetEqOutputType type; + EXPECT_EQ( + NetEq::kOK, + neteq_->GetAudio( + kMaxOutputSize, output, &samples_per_channel, &num_channels, &type)); + ASSERT_EQ(kMaxOutputSize, samples_per_channel); + EXPECT_EQ(1, num_channels); + EXPECT_EQ(kOutputNormal, type); + + // Insert two more packets. The first one is out of order, and is already too + // old, the second one is the expected next packet. + rtp_header.header.sequenceNumber -= 1; + rtp_header.header.timestamp -= kPayloadLengthSamples; + payload[0] = 1; + EXPECT_EQ(NetEq::kOK, + neteq_->InsertPacket( + rtp_header, payload, kPayloadLengthBytes, kReceiveTime)); + rtp_header.header.sequenceNumber += 2; + rtp_header.header.timestamp += 2 * kPayloadLengthSamples; + payload[0] = 2; + EXPECT_EQ(NetEq::kOK, + neteq_->InsertPacket( + rtp_header, payload, kPayloadLengthBytes, kReceiveTime)); + + // Expect only the second packet to be decoded (the one with "2" as the first + // payload byte). + EXPECT_CALL(mock_decoder, Decode(Pointee(2), kPayloadLengthBytes, _, _)) + .WillOnce(DoAll(SetArrayArgument<2>(dummy_output, + dummy_output + kPayloadLengthSamples), + SetArgPointee<3>(AudioDecoder::kSpeech), + Return(kPayloadLengthSamples))); + + // Pull audio once. + EXPECT_EQ( + NetEq::kOK, + neteq_->GetAudio( + kMaxOutputSize, output, &samples_per_channel, &num_channels, &type)); + ASSERT_EQ(kMaxOutputSize, samples_per_channel); + EXPECT_EQ(1, num_channels); + EXPECT_EQ(kOutputNormal, type); + + // Now check the packet buffer, and make sure it is empty, since the + // out-of-order packet should have been discarded. + EXPECT_TRUE(packet_buffer_->Empty()); + + EXPECT_CALL(mock_decoder, Die()); +} + } // namespace webrtc diff --git a/modules/audio_coding/neteq/packet_buffer.cc b/modules/audio_coding/neteq/packet_buffer.cc index 4c484185..816713d8 100644 --- a/modules/audio_coding/neteq/packet_buffer.cc +++ b/modules/audio_coding/neteq/packet_buffer.cc @@ -216,12 +216,12 @@ int PacketBuffer::DiscardNextPacket() { return kOK; } -int PacketBuffer::DiscardOldPackets(uint32_t timestamp_limit) { - while (!Empty() && - timestamp_limit != buffer_.front()->header.timestamp && - static_cast(timestamp_limit - - buffer_.front()->header.timestamp) < - 0xFFFFFFFF / 2) { +int PacketBuffer::DiscardOldPackets(uint32_t timestamp_limit, + uint32_t horizon_samples) { + while (!Empty() && timestamp_limit != buffer_.front()->header.timestamp && + IsObsoleteTimestamp(buffer_.front()->header.timestamp, + timestamp_limit, + horizon_samples)) { if (DiscardNextPacket() != kOK) { assert(false); // Must be ok by design. } diff --git a/modules/audio_coding/neteq/packet_buffer.h b/modules/audio_coding/neteq/packet_buffer.h index 76c4ddd1..b9a16189 100644 --- a/modules/audio_coding/neteq/packet_buffer.h +++ b/modules/audio_coding/neteq/packet_buffer.h @@ -95,9 +95,19 @@ class PacketBuffer { // PacketBuffer::kOK otherwise. virtual int DiscardNextPacket(); - // Discards all packets that are (strictly) older than |timestamp_limit|. + // Discards all packets that are (strictly) older than timestamp_limit, + // but newer than timestamp_limit - horizon_samples. Setting horizon_samples + // to zero implies that the horizon is set to half the timestamp range. That + // is, if a packet is more than 2^31 timestamps into the future compared with + // timestamp_limit (including wrap-around), it is considered old. // Returns number of packets discarded. - virtual int DiscardOldPackets(uint32_t timestamp_limit); + virtual int DiscardOldPackets(uint32_t timestamp_limit, + uint32_t horizon_samples); + + // Discards all packets that are (strictly) older than timestamp_limit. + virtual int DiscardAllOldPackets(uint32_t timestamp_limit) { + return DiscardOldPackets(timestamp_limit, 0); + } // Returns the number of packets in the buffer, including duplicates and // redundant packets. @@ -125,6 +135,20 @@ class PacketBuffer { // in |packet_list|. static void DeleteAllPackets(PacketList* packet_list); + // Static method returning true if |timestamp| is older than |timestamp_limit| + // but less than |horizon_samples| behind |timestamp_limit|. For instance, + // with timestamp_limit = 100 and horizon_samples = 10, a timestamp in the + // range (90, 100) is considered obsolete, and will yield true. + // Setting |horizon_samples| to 0 is the same as setting it to 2^31, i.e., + // half the 32-bit timestamp range. + static bool IsObsoleteTimestamp(uint32_t timestamp, + uint32_t timestamp_limit, + uint32_t horizon_samples) { + return IsNewerTimestamp(timestamp_limit, timestamp) && + (horizon_samples == 0 || + IsNewerTimestamp(timestamp, timestamp_limit - horizon_samples)); + } + private: size_t max_number_of_packets_; PacketList buffer_; diff --git a/modules/audio_coding/neteq/packet_buffer_unittest.cc b/modules/audio_coding/neteq/packet_buffer_unittest.cc index 478328cb..dc8b68c3 100644 --- a/modules/audio_coding/neteq/packet_buffer_unittest.cc +++ b/modules/audio_coding/neteq/packet_buffer_unittest.cc @@ -391,7 +391,7 @@ TEST(PacketBuffer, Failures) { EXPECT_EQ(NULL, buffer->NextRtpHeader()); EXPECT_EQ(NULL, buffer->GetNextPacket(NULL)); EXPECT_EQ(PacketBuffer::kBufferEmpty, buffer->DiscardNextPacket()); - EXPECT_EQ(0, buffer->DiscardOldPackets(0)); // 0 packets discarded. + EXPECT_EQ(0, buffer->DiscardAllOldPackets(0)); // 0 packets discarded. // Insert one packet to make the buffer non-empty. packet = gen.NextPacket(payload_len); @@ -513,4 +513,61 @@ TEST(PacketBuffer, DeleteAllPackets) { EXPECT_FALSE(PacketBuffer::DeleteFirstPacket(&list)); } +namespace { +void TestIsObsoleteTimestamp(uint32_t limit_timestamp) { + // Check with zero horizon, which implies that the horizon is at 2^31, i.e., + // half the timestamp range. + static const uint32_t kZeroHorizon = 0; + static const uint32_t k2Pow31Minus1 = 0x7FFFFFFF; + // Timestamp on the limit is not old. + EXPECT_FALSE(PacketBuffer::IsObsoleteTimestamp( + limit_timestamp, limit_timestamp, kZeroHorizon)); + // 1 sample behind is old. + EXPECT_TRUE(PacketBuffer::IsObsoleteTimestamp( + limit_timestamp - 1, limit_timestamp, kZeroHorizon)); + // 2^31 - 1 samples behind is old. + EXPECT_TRUE(PacketBuffer::IsObsoleteTimestamp( + limit_timestamp - k2Pow31Minus1, limit_timestamp, kZeroHorizon)); + // 1 sample ahead is not old. + EXPECT_FALSE(PacketBuffer::IsObsoleteTimestamp( + limit_timestamp + 1, limit_timestamp, kZeroHorizon)); + // 2^31 samples ahead is not old. + EXPECT_FALSE(PacketBuffer::IsObsoleteTimestamp( + limit_timestamp + (1 << 31), limit_timestamp, kZeroHorizon)); + + // Fixed horizon at 10 samples. + static const uint32_t kHorizon = 10; + // Timestamp on the limit is not old. + EXPECT_FALSE(PacketBuffer::IsObsoleteTimestamp( + limit_timestamp, limit_timestamp, kHorizon)); + // 1 sample behind is old. + EXPECT_TRUE(PacketBuffer::IsObsoleteTimestamp( + limit_timestamp - 1, limit_timestamp, kHorizon)); + // 9 samples behind is old. + EXPECT_TRUE(PacketBuffer::IsObsoleteTimestamp( + limit_timestamp - 9, limit_timestamp, kHorizon)); + // 10 samples behind is not old. + EXPECT_FALSE(PacketBuffer::IsObsoleteTimestamp( + limit_timestamp - 10, limit_timestamp, kHorizon)); + // 2^31 - 1 samples behind is not old. + EXPECT_FALSE(PacketBuffer::IsObsoleteTimestamp( + limit_timestamp - k2Pow31Minus1, limit_timestamp, kHorizon)); + // 1 sample ahead is not old. + EXPECT_FALSE(PacketBuffer::IsObsoleteTimestamp( + limit_timestamp + 1, limit_timestamp, kHorizon)); + // 2^31 samples ahead is not old. + EXPECT_FALSE(PacketBuffer::IsObsoleteTimestamp( + limit_timestamp + (1 << 31), limit_timestamp, kHorizon)); +} +} // namespace + +// Test the IsObsoleteTimestamp method with different limit timestamps. +TEST(PacketBuffer, IsObsoleteTimestamp) { + TestIsObsoleteTimestamp(0); + TestIsObsoleteTimestamp(1); + TestIsObsoleteTimestamp(0xFFFFFFFF); // -1 in uint32_t. + TestIsObsoleteTimestamp(0x80000000); // 2^31. + TestIsObsoleteTimestamp(0x80000001); // 2^31 + 1. + TestIsObsoleteTimestamp(0x7FFFFFFF); // 2^31 - 1. +} } // namespace webrtc -- cgit v1.2.3 From 8d28158fe5a9526c3b8138fc3c27bcea768eb6f5 Mon Sep 17 00:00:00 2001 From: "stefan@webrtc.org" Date: Tue, 4 Nov 2014 14:57:14 +0000 Subject: Adds support for finch experiments to video_loopback. Adds support for logging to stderr via -logs. Enables abs-send-time by default. R=mflodman@webrtc.org, pbos@webrtc.org Review URL: https://webrtc-codereview.appspot.com/23329004 git-svn-id: http://webrtc.googlecode.com/svn/trunk/webrtc@7613 4adac7df-926f-26a2-2b94-8c16560cd09d --- video/loopback.cc | 26 +++++++++++++++++++++++++- webrtc_tests.gypi | 1 + 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/video/loopback.cc b/video/loopback.cc index 4d3393e7..4b49c31e 100644 --- a/video/loopback.cc +++ b/video/loopback.cc @@ -22,13 +22,18 @@ #include "webrtc/test/direct_transport.h" #include "webrtc/test/encoder_settings.h" #include "webrtc/test/fake_encoder.h" +#include "webrtc/test/field_trial.h" #include "webrtc/test/run_loop.h" #include "webrtc/test/run_test.h" +#include "webrtc/test/testsupport/trace_to_stderr.h" #include "webrtc/test/video_capturer.h" #include "webrtc/test/video_renderer.h" #include "webrtc/typedefs.h" namespace webrtc { + +static const int kAbsSendTimeExtensionId = 7; + namespace flags { DEFINE_int32(width, 640, "Video width."); @@ -82,6 +87,16 @@ DEFINE_int32(std_propagation_delay_ms, int StdPropagationDelayMs() { return static_cast(FLAGS_std_propagation_delay_ms); } + +DEFINE_bool(logs, false, "print logs to stderr"); + +DEFINE_string( + force_fieldtrials, + "", + "Field trials control experimental feature code which can be forced. " + "E.g. running with --force_fieldtrials=WebRTC-FooFeature/Enable/" + " will assign the group Enable to field trial WebRTC-FooFeature. Multiple " + "trials are separated by \"/\""); } // namespace flags static const uint32_t kSendSsrc = 0x654321; @@ -91,6 +106,10 @@ static const uint32_t kReceiverLocalSsrc = 0x123456; static const uint8_t kRtxPayloadType = 96; void Loopback() { + scoped_ptr trace_to_stderr_; + if (webrtc::flags::FLAGS_logs) + trace_to_stderr_.reset(new test::TraceToStderr); + scoped_ptr local_preview(test::VideoRenderer::Create( "Local Preview", flags::Width(), flags::Height())); scoped_ptr loopback_video(test::VideoRenderer::Create( @@ -116,6 +135,8 @@ void Loopback() { send_config.rtp.rtx.ssrcs.push_back(kSendRtxSsrc); send_config.rtp.rtx.payload_type = kRtxPayloadType; send_config.rtp.nack.rtp_history_ms = 1000; + send_config.rtp.extensions.push_back( + RtpExtension(RtpExtension::kAbsSendTime, kAbsSendTimeExtensionId)); send_config.local_renderer = local_preview.get(); scoped_ptr encoder; @@ -158,6 +179,8 @@ void Loopback() { receive_config.rtp.nack.rtp_history_ms = 1000; receive_config.rtp.rtx[kRtxPayloadType].ssrc = kSendRtxSsrc; receive_config.rtp.rtx[kRtxPayloadType].payload_type = kRtxPayloadType; + receive_config.rtp.extensions.push_back( + RtpExtension(RtpExtension::kAbsSendTime, kAbsSendTimeExtensionId)); receive_config.renderer = loopback_video.get(); VideoReceiveStream::Decoder decoder = test::CreateMatchingDecoder(send_config.encoder_settings); @@ -188,7 +211,8 @@ void Loopback() { int main(int argc, char* argv[]) { ::testing::InitGoogleTest(&argc, argv); google::ParseCommandLineFlags(&argc, &argv, true); - + webrtc::test::InitFieldTrialsFromString( + webrtc::flags::FLAGS_force_fieldtrials); webrtc::test::RunTest(webrtc::Loopback); return 0; } diff --git a/webrtc_tests.gypi b/webrtc_tests.gypi index 33e509fa..dc17e707 100644 --- a/webrtc_tests.gypi +++ b/webrtc_tests.gypi @@ -59,6 +59,7 @@ '<(webrtc_root)/modules/modules.gyp:video_capture_module_internal_impl', '<(webrtc_root)/modules/modules.gyp:video_render_module_impl', '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:system_wrappers_default', + 'test/test.gyp:test_main', 'webrtc', ], }, -- cgit v1.2.3 From 9a8c28f10f329c5ce91e77057933e60224000627 Mon Sep 17 00:00:00 2001 From: "sprang@webrtc.org" Date: Tue, 4 Nov 2014 16:27:16 +0000 Subject: Reworked paced sender queue Packet queue in the paced sender is now based on a priority queue rather than having a separate fifo-queue per priority level. This allows more flexible sorting and cleaner usage. Packets with earlier capture times are now prioritized higher. In situations with high packet loss, the queue might contain packets from several subsequent frames. Retransmit packets from the earlier frames first, since the later ones will probably be dependent on these. Also, don't force sending of packets after a certain time of inactivity or when packets grow too old, since this was causing consistent overuse on poor connections. Instead, drop frames in vie encoder if pacer queue is too long. BUG= R=mflodman@webrtc.org, stefan@webrtc.org Review URL: https://webrtc-codereview.appspot.com/27869004 git-svn-id: http://webrtc.googlecode.com/svn/trunk/webrtc@7617 4adac7df-926f-26a2-2b94-8c16560cd09d --- modules/pacing/include/paced_sender.h | 40 +--- modules/pacing/paced_sender.cc | 354 +++++++++++++++----------------- modules/pacing/paced_sender_unittest.cc | 124 +++++++++-- modules/rtp_rtcp/source/rtp_sender.cc | 36 ++-- modules/rtp_rtcp/source/rtp_sender.h | 2 + video_engine/vie_encoder.cc | 37 ++-- video_engine/vie_encoder.h | 2 + 7 files changed, 323 insertions(+), 272 deletions(-) diff --git a/modules/pacing/include/paced_sender.h b/modules/pacing/include/paced_sender.h index d3034466..d7efb8ea 100644 --- a/modules/pacing/include/paced_sender.h +++ b/modules/pacing/include/paced_sender.h @@ -27,7 +27,7 @@ class CriticalSectionWrapper; namespace paced_sender { class IntervalBudget; struct Packet; -class PacketList; +class PacketQueue; } // namespace paced_sender class PacedSender : public Module { @@ -105,15 +105,15 @@ class PacedSender : public Module { int bytes, bool retransmission); - // Sets the max length of the pacer queue in milliseconds. - // A negative queue size is interpreted as infinite. - virtual void set_max_queue_length_ms(int max_queue_length_ms); - // Returns the time since the oldest queued packet was enqueued. virtual int QueueInMs() const; virtual size_t QueueSizePackets() const; + // Returns the number of milliseconds it will take to send the current + // packets in the queue, given the current size and bitrate, ignoring prio. + virtual int ExpectedQueueTimeMs() const; + // Returns the number of milliseconds until the module want a worker thread // to call Process. virtual int32_t TimeUntilNextProcess() OVERRIDE; @@ -125,24 +125,13 @@ class PacedSender : public Module { virtual bool ProbingExperimentIsEnabled() const; private: - // Return true if next packet in line should be transmitted. - // Return packet list that contains the next packet. - bool ShouldSendNextPacket(paced_sender::PacketList** packet_list, bool probe) - EXCLUSIVE_LOCKS_REQUIRED(critsect_); - - // Local helper function to GetNextPacket. - paced_sender::Packet GetNextPacketFromList(paced_sender::PacketList* packets) - EXCLUSIVE_LOCKS_REQUIRED(critsect_); - - bool SendPacketFromList(paced_sender::PacketList* packet_list) - EXCLUSIVE_LOCKS_REQUIRED(critsect_); - // Updates the number of bytes that can be sent for the next time interval. void UpdateBytesPerInterval(uint32_t delta_time_in_ms) EXCLUSIVE_LOCKS_REQUIRED(critsect_); - // Updates the buffers with the number of bytes that we sent. - void UpdateMediaBytesSent(int num_bytes) EXCLUSIVE_LOCKS_REQUIRED(critsect_); + bool SendPacket(const paced_sender::Packet& packet) + EXCLUSIVE_LOCKS_REQUIRED(critsect_); + void SendPadding(int padding_needed) EXCLUSIVE_LOCKS_REQUIRED(critsect_); Clock* const clock_; Callback* const callback_; @@ -150,7 +139,6 @@ class PacedSender : public Module { scoped_ptr critsect_; bool enabled_ GUARDED_BY(critsect_); bool paused_ GUARDED_BY(critsect_); - int max_queue_length_ms_ GUARDED_BY(critsect_); // This is the media budget, keeping track of how many bits of media // we can pace out during the current interval. scoped_ptr media_budget_ GUARDED_BY(critsect_); @@ -164,17 +152,9 @@ class PacedSender : public Module { int bitrate_bps_ GUARDED_BY(critsect_); int64_t time_last_update_us_ GUARDED_BY(critsect_); - // Only accessed via process thread. - int64_t time_last_media_send_us_; - int64_t capture_time_ms_last_queued_ GUARDED_BY(critsect_); - int64_t capture_time_ms_last_sent_ GUARDED_BY(critsect_); - scoped_ptr high_priority_packets_ - GUARDED_BY(critsect_); - scoped_ptr normal_priority_packets_ - GUARDED_BY(critsect_); - scoped_ptr low_priority_packets_ - GUARDED_BY(critsect_); + scoped_ptr packets_ GUARDED_BY(critsect_); + uint64_t packet_counter_ GUARDED_BY(critsect_); }; } // namespace webrtc #endif // WEBRTC_MODULES_PACING_INCLUDE_PACED_SENDER_H_ diff --git a/modules/pacing/paced_sender.cc b/modules/pacing/paced_sender.cc index 64b3eb1e..a071ffcc 100644 --- a/modules/pacing/paced_sender.cc +++ b/modules/pacing/paced_sender.cc @@ -13,6 +13,7 @@ #include #include +#include #include #include "webrtc/modules/interface/module_common_types.h" @@ -31,80 +32,140 @@ const int kMinPacketLimitMs = 5; // time. const int kMaxIntervalTimeMs = 30; -// Max time that the first packet in the queue can sit in the queue if no -// packets are sent, regardless of buffer state. In practice only in effect at -// low bitrates (less than 320 kbits/s). -const int kMaxQueueTimeWithoutSendingUs = 30000; - } // namespace namespace webrtc { namespace paced_sender { struct Packet { - Packet(uint32_t ssrc, + Packet(PacedSender::Priority priority, + uint32_t ssrc, uint16_t seq_number, int64_t capture_time_ms, int64_t enqueue_time_ms, int length_in_bytes, - bool retransmission) - : ssrc(ssrc), + bool retransmission, + uint64_t enqueue_order) + : priority(priority), + ssrc(ssrc), sequence_number(seq_number), capture_time_ms(capture_time_ms), enqueue_time_ms(enqueue_time_ms), bytes(length_in_bytes), - retransmission(retransmission) {} + retransmission(retransmission), + enqueue_order(enqueue_order) {} + + PacedSender::Priority priority; uint32_t ssrc; uint16_t sequence_number; int64_t capture_time_ms; int64_t enqueue_time_ms; int bytes; bool retransmission; + uint64_t enqueue_order; + std::list::iterator this_it; +}; + +// Used by priority queue to sort packets. +struct Comparator { + bool operator()(const Packet* first, const Packet* second) { + // Highest prio = 0. + if (first->priority != second->priority) + return first->priority > second->priority; + + // Retransmissions go first. + if (second->retransmission && !first->retransmission) + return true; + + // Older frames have higher prio. + if (first->capture_time_ms != second->capture_time_ms) + return first->capture_time_ms > second->capture_time_ms; + + return first->enqueue_order > second->enqueue_order; + } }; -// STL list style class which prevents duplicates in the list. -class PacketList { +// Class encapsulating a priority queue with some extensions. +class PacketQueue { public: - PacketList() {}; + PacketQueue() : bytes_(0) {} + virtual ~PacketQueue() {} + + void Push(const Packet& packet) { + if (!AddToDupeSet(packet)) + return; + + // Store packet in list, use pointers in priority queue for cheaper moves. + // Packets have a handle to its own iterator in the list, for easy removal + // when popping from queue. + packet_list_.push_front(packet); + std::list::iterator it = packet_list_.begin(); + it->this_it = it; // Handle for direct removal from list. + prio_queue_.push(&(*it)); // Pointer into list. + bytes_ += packet.bytes; + } - bool empty() const { - return packet_list_.empty(); + const Packet& BeginPop() { + const Packet& packet = *prio_queue_.top(); + prio_queue_.pop(); + return packet; } - Packet front() const { - return packet_list_.front(); + void CancelPop(const Packet& packet) { prio_queue_.push(&(*packet.this_it)); } + + void FinalizePop(const Packet& packet) { + RemoveFromDupeSet(packet); + bytes_ -= packet.bytes; + packet_list_.erase(packet.this_it); } - size_t size() const { - size_t sum = 0; - for (std::map >::const_iterator it = - sequence_number_set_.begin(); - it != sequence_number_set_.end(); - ++it) { - sum += it->second.size(); - } - return sum; + bool Empty() const { return prio_queue_.empty(); } + + size_t SizeInPackets() const { return prio_queue_.size(); } + + uint32_t SizeInBytes() const { return bytes_; } + + int64_t OldestEnqueueTime() const { + std::list::const_reverse_iterator it = packet_list_.rbegin(); + if (it == packet_list_.rend()) + return 0; + return it->enqueue_time_ms; } - void pop_front() { - Packet& packet = packet_list_.front(); - uint16_t sequence_number = packet.sequence_number; - uint32_t ssrc = packet.ssrc; - packet_list_.pop_front(); - sequence_number_set_[ssrc].erase(sequence_number); + private: + // Try to add a packet to the set of ssrc/seqno identifiers currently in the + // queue. Return true if inserted, false if this is a duplicate. + bool AddToDupeSet(const Packet& packet) { + SsrcSeqNoMap::iterator it = dupe_map_.find(packet.ssrc); + if (it == dupe_map_.end()) { + // First for this ssrc, just insert. + dupe_map_[packet.ssrc].insert(packet.sequence_number); + return true; + } + + // Insert returns a pair, where second is a bool set to true if new element. + return it->second.insert(packet.sequence_number).second; } - void push_back(const Packet& packet) { - if (sequence_number_set_[packet.ssrc].find(packet.sequence_number) == - sequence_number_set_[packet.ssrc].end()) { - // Don't insert duplicates. - packet_list_.push_back(packet); - sequence_number_set_[packet.ssrc].insert(packet.sequence_number); + void RemoveFromDupeSet(const Packet& packet) { + SsrcSeqNoMap::iterator it = dupe_map_.find(packet.ssrc); + assert(it != dupe_map_.end()); + it->second.erase(packet.sequence_number); + if (it->second.empty()) { + dupe_map_.erase(it); } } - private: + // List of packets, in the order the were enqueued. Since dequeueing may + // occur out of order, use list instead of vector. std::list packet_list_; - std::map > sequence_number_set_; + // Priority queue of the packets, sorted according to Comparator. + // Use pointers into list, to avoid moving whole struct within heap. + std::priority_queue, Comparator> prio_queue_; + // Total number of bytes in the queue. + uint64_t bytes_; + // Map >, for checking duplicates. + typedef std::map > SsrcSeqNoMap; + SsrcSeqNoMap dupe_map_; }; class IntervalBudget { @@ -135,6 +196,8 @@ class IntervalBudget { int bytes_remaining() const { return bytes_remaining_; } + int target_rate_kbps() const { return target_rate_kbps_; } + private: int target_rate_kbps_; int bytes_remaining_; @@ -153,18 +216,13 @@ PacedSender::PacedSender(Clock* clock, critsect_(CriticalSectionWrapper::CreateCriticalSection()), enabled_(true), paused_(false), - max_queue_length_ms_(kDefaultMaxQueueLengthMs), media_budget_(new paced_sender::IntervalBudget(max_bitrate_kbps)), padding_budget_(new paced_sender::IntervalBudget(min_bitrate_kbps)), prober_(new BitrateProber()), bitrate_bps_(1000 * bitrate_kbps), time_last_update_us_(clock->TimeInMicroseconds()), - time_last_media_send_us_(-1), - capture_time_ms_last_queued_(0), - capture_time_ms_last_sent_(0), - high_priority_packets_(new paced_sender::PacketList), - normal_priority_packets_(new paced_sender::PacketList), - low_priority_packets_(new paced_sender::PacketList) { + packets_(new paced_sender::PacketQueue()), + packet_counter_(0) { UpdateBytesPerInterval(kMinPacketLimitMs); } @@ -216,64 +274,33 @@ bool PacedSender::SendPacket(Priority priority, uint32_t ssrc, if (capture_time_ms < 0) { capture_time_ms = clock_->TimeInMilliseconds(); } - if (priority != kHighPriority && - capture_time_ms > capture_time_ms_last_queued_) { - capture_time_ms_last_queued_ = capture_time_ms; - TRACE_EVENT_ASYNC_BEGIN1("webrtc_rtp", "PacedSend", capture_time_ms, - "capture_time_ms", capture_time_ms); - } - paced_sender::PacketList* packet_list = NULL; - switch (priority) { - case kHighPriority: - packet_list = high_priority_packets_.get(); - break; - case kNormalPriority: - packet_list = normal_priority_packets_.get(); - break; - case kLowPriority: - packet_list = low_priority_packets_.get(); - break; - } - packet_list->push_back(paced_sender::Packet(ssrc, - sequence_number, - capture_time_ms, - clock_->TimeInMilliseconds(), - bytes, - retransmission)); + + packets_->Push(paced_sender::Packet( + priority, ssrc, sequence_number, capture_time_ms, + clock_->TimeInMilliseconds(), bytes, retransmission, packet_counter_++)); return false; } -void PacedSender::set_max_queue_length_ms(int max_queue_length_ms) { +int PacedSender::ExpectedQueueTimeMs() const { CriticalSectionScoped cs(critsect_.get()); - max_queue_length_ms_ = max_queue_length_ms; + int target_rate = media_budget_->target_rate_kbps(); + assert(target_rate > 0); + return packets_->SizeInBytes() * 8 / target_rate; } -int PacedSender::QueueInMs() const { +size_t PacedSender::QueueSizePackets() const { CriticalSectionScoped cs(critsect_.get()); - int64_t now_ms = clock_->TimeInMilliseconds(); - int64_t oldest_packet_enqueue_time = now_ms; - if (!high_priority_packets_->empty()) { - oldest_packet_enqueue_time = - std::min(oldest_packet_enqueue_time, - high_priority_packets_->front().enqueue_time_ms); - } - if (!normal_priority_packets_->empty()) { - oldest_packet_enqueue_time = - std::min(oldest_packet_enqueue_time, - normal_priority_packets_->front().enqueue_time_ms); - } - if (!low_priority_packets_->empty()) { - oldest_packet_enqueue_time = - std::min(oldest_packet_enqueue_time, - low_priority_packets_->front().enqueue_time_ms); - } - return now_ms - oldest_packet_enqueue_time; + return packets_->SizeInPackets(); } -size_t PacedSender::QueueSizePackets() const { +int PacedSender::QueueInMs() const { CriticalSectionScoped cs(critsect_.get()); - return low_priority_packets_->size() + normal_priority_packets_->size() + - high_priority_packets_->size(); + + int64_t oldest_packet = packets_->OldestEnqueueTime(); + if (oldest_packet == 0) + return 0; + + return clock_->TimeInMilliseconds() - oldest_packet; } int32_t PacedSender::TimeUntilNextProcess() { @@ -303,127 +330,66 @@ int32_t PacedSender::Process() { uint32_t delta_time_ms = std::min(kMaxIntervalTimeMs, elapsed_time_ms); UpdateBytesPerInterval(delta_time_ms); } - paced_sender::PacketList* packet_list; - while (ShouldSendNextPacket(&packet_list, prober_->IsProbing())) { - if (!SendPacketFromList(packet_list)) + + while (!packets_->Empty()) { + if (media_budget_->bytes_remaining() <= 0 && !prober_->IsProbing()) return 0; - // Send one packet per Process() call when probing, so that we have - // better control over the delta between packets. - if (prober_->IsProbing()) + + // Since we need to release the lock in order to send, we first pop the + // element from the priority queue but keep it in storage, so that we can + // reinsert it if send fails. + const paced_sender::Packet& packet = packets_->BeginPop(); + if (SendPacket(packet)) { + // Send succeeded, remove it from the queue. + packets_->FinalizePop(packet); + if (prober_->IsProbing()) + return 0; + } else { + // Send failed, put it back into the queue. + packets_->CancelPop(packet); return 0; + } } - if (high_priority_packets_->empty() && normal_priority_packets_->empty() && - low_priority_packets_->empty() && - padding_budget_->bytes_remaining() > 0) { - int padding_needed = padding_budget_->bytes_remaining(); - critsect_->Leave(); - int bytes_sent = callback_->TimeToSendPadding(padding_needed); - critsect_->Enter(); - media_budget_->UseBudget(bytes_sent); - padding_budget_->UseBudget(bytes_sent); + + int padding_needed = padding_budget_->bytes_remaining(); + if (padding_needed > 0) { + SendPadding(padding_needed); } } return 0; } -bool PacedSender::SendPacketFromList(paced_sender::PacketList* packet_list) - EXCLUSIVE_LOCKS_REQUIRED(critsect_.get()) { - paced_sender::Packet packet = GetNextPacketFromList(packet_list); +bool PacedSender::SendPacket(const paced_sender::Packet& packet) { critsect_->Leave(); - const bool success = callback_->TimeToSendPacket(packet.ssrc, packet.sequence_number, packet.capture_time_ms, packet.retransmission); critsect_->Enter(); - // If packet cannot be sent then keep it in packet list and exit early. - // There's no need to send more packets. - if (!success) { - return false; - } - packet_list->pop_front(); - const bool last_packet = - packet_list->empty() || - packet_list->front().capture_time_ms > packet.capture_time_ms; - if (packet_list != high_priority_packets_.get()) { - if (packet.capture_time_ms > capture_time_ms_last_sent_) { - capture_time_ms_last_sent_ = packet.capture_time_ms; - } else if (packet.capture_time_ms == capture_time_ms_last_sent_ && - last_packet) { - TRACE_EVENT_ASYNC_END0("webrtc_rtp", "PacedSend", packet.capture_time_ms); - } + + if (success) { + // Update media bytes sent. + prober_->PacketSent(clock_->TimeInMilliseconds(), packet.bytes); + media_budget_->UseBudget(packet.bytes); + padding_budget_->UseBudget(packet.bytes); } - return true; -} -void PacedSender::UpdateBytesPerInterval(uint32_t delta_time_ms) { - media_budget_->IncreaseBudget(delta_time_ms); - padding_budget_->IncreaseBudget(delta_time_ms); + return success; } -bool PacedSender::ShouldSendNextPacket(paced_sender::PacketList** packet_list, - bool probe) { - *packet_list = NULL; - if (!probe && media_budget_->bytes_remaining() <= 0) { - // All bytes consumed for this interval. - // Check if we have not sent in a too long time. - if (clock_->TimeInMicroseconds() - time_last_media_send_us_ > - kMaxQueueTimeWithoutSendingUs) { - if (!high_priority_packets_->empty()) { - *packet_list = high_priority_packets_.get(); - return true; - } - if (!normal_priority_packets_->empty()) { - *packet_list = normal_priority_packets_.get(); - return true; - } - } - // Send any old packets to avoid queuing for too long. - if (max_queue_length_ms_ >= 0 && QueueInMs() > max_queue_length_ms_) { - int64_t high_priority_capture_time = -1; - if (!high_priority_packets_->empty()) { - high_priority_capture_time = - high_priority_packets_->front().capture_time_ms; - *packet_list = high_priority_packets_.get(); - } - if (!normal_priority_packets_->empty() && - (high_priority_capture_time == -1 || - high_priority_capture_time > - normal_priority_packets_->front().capture_time_ms)) { - *packet_list = normal_priority_packets_.get(); - } - if (*packet_list) - return true; - } - return false; - } - if (!high_priority_packets_->empty()) { - *packet_list = high_priority_packets_.get(); - return true; - } - if (!normal_priority_packets_->empty()) { - *packet_list = normal_priority_packets_.get(); - return true; - } - if (!low_priority_packets_->empty()) { - *packet_list = low_priority_packets_.get(); - return true; - } - return false; -} +void PacedSender::SendPadding(int padding_needed) { + critsect_->Leave(); + int bytes_sent = callback_->TimeToSendPadding(padding_needed); + critsect_->Enter(); -paced_sender::Packet PacedSender::GetNextPacketFromList( - paced_sender::PacketList* packets) { - paced_sender::Packet packet = packets->front(); - UpdateMediaBytesSent(packet.bytes); - return packet; + // Update padding bytes sent. + media_budget_->UseBudget(bytes_sent); + padding_budget_->UseBudget(bytes_sent); } -void PacedSender::UpdateMediaBytesSent(int num_bytes) { - prober_->PacketSent(clock_->TimeInMilliseconds(), num_bytes); - time_last_media_send_us_ = clock_->TimeInMicroseconds(); - media_budget_->UseBudget(num_bytes); - padding_budget_->UseBudget(num_bytes); +void PacedSender::UpdateBytesPerInterval(uint32_t delta_time_ms) { + media_budget_->IncreaseBudget(delta_time_ms); + padding_budget_->IncreaseBudget(delta_time_ms); } bool PacedSender::ProbingExperimentIsEnabled() const { diff --git a/modules/pacing/paced_sender_unittest.cc b/modules/pacing/paced_sender_unittest.cc index f8028a91..34787d16 100644 --- a/modules/pacing/paced_sender_unittest.cc +++ b/modules/pacing/paced_sender_unittest.cc @@ -639,34 +639,40 @@ TEST_F(PacedSenderTest, ResendPacket) { EXPECT_EQ(0, send_bucket_->QueueInMs()); } -TEST_F(PacedSenderTest, MaxQueueLength) { +TEST_F(PacedSenderTest, ExpectedQueueTimeMs) { uint32_t ssrc = 12346; uint16_t sequence_number = 1234; - EXPECT_EQ(0, send_bucket_->QueueInMs()); + const int32_t kNumPackets = 60; + const int32_t kPacketSize = 1200; + const int32_t kMaxBitrate = kPaceMultiplier * 30; + EXPECT_EQ(0, send_bucket_->ExpectedQueueTimeMs()); - send_bucket_->UpdateBitrate(30, kPaceMultiplier * 30, 0); - for (int i = 0; i < 30; ++i) { - SendAndExpectPacket(PacedSender::kNormalPriority, - ssrc, - sequence_number++, - clock_.TimeInMilliseconds(), - 1200, - false); + send_bucket_->UpdateBitrate(30, kMaxBitrate, 0); + for (int i = 0; i < kNumPackets; ++i) { + SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number++, + clock_.TimeInMilliseconds(), kPacketSize, false); } - clock_.AdvanceTimeMilliseconds(2001); - SendAndExpectPacket(PacedSender::kNormalPriority, - ssrc, - sequence_number++, - clock_.TimeInMilliseconds(), - 1200, - false); - EXPECT_EQ(2001, send_bucket_->QueueInMs()); - send_bucket_->Process(); - EXPECT_EQ(0, send_bucket_->QueueInMs()); - clock_.AdvanceTimeMilliseconds(31); + // Queue in ms = 1000 * (bytes in queue) / (kbit per second * 1000 / 8) + int32_t queue_in_ms = kNumPackets * kPacketSize * 8 / kMaxBitrate; + EXPECT_EQ(queue_in_ms, send_bucket_->ExpectedQueueTimeMs()); - send_bucket_->Process(); + int64_t time_start = clock_.TimeInMilliseconds(); + while (send_bucket_->QueueSizePackets() > 0) { + int time_until_process = send_bucket_->TimeUntilNextProcess(); + if (time_until_process <= 0) { + send_bucket_->Process(); + } else { + clock_.AdvanceTimeMilliseconds(time_until_process); + } + } + int64_t duration = clock_.TimeInMilliseconds() - time_start; + + EXPECT_EQ(0, send_bucket_->ExpectedQueueTimeMs()); + + // Allow for aliasing, duration should be in [expected(n - 1), expected(n)]. + EXPECT_LE(duration, queue_in_ms); + EXPECT_GE(duration, queue_in_ms - (kPacketSize * 8 / kMaxBitrate)); } TEST_F(PacedSenderTest, QueueTimeGrowsOverTime) { @@ -738,5 +744,79 @@ TEST_F(PacedSenderTest, ProbingWithInitialFrame) { } } } + +TEST_F(PacedSenderTest, PriorityInversion) { + uint32_t ssrc = 12346; + uint16_t sequence_number = 1234; + const int32_t kPacketSize = 1200; + + EXPECT_FALSE(send_bucket_->SendPacket( + PacedSender::kHighPriority, ssrc, sequence_number + 3, + clock_.TimeInMilliseconds() + 33, kPacketSize, true)); + + EXPECT_FALSE(send_bucket_->SendPacket( + PacedSender::kHighPriority, ssrc, sequence_number + 2, + clock_.TimeInMilliseconds() + 33, kPacketSize, true)); + + EXPECT_FALSE(send_bucket_->SendPacket( + PacedSender::kHighPriority, ssrc, sequence_number, + clock_.TimeInMilliseconds(), kPacketSize, true)); + + EXPECT_FALSE(send_bucket_->SendPacket( + PacedSender::kHighPriority, ssrc, sequence_number + 1, + clock_.TimeInMilliseconds(), kPacketSize, true)); + + // Packets from earlier frames should be sent first. + { + ::testing::InSequence sequence; + EXPECT_CALL(callback_, TimeToSendPacket(ssrc, sequence_number, + clock_.TimeInMilliseconds(), true)) + .WillOnce(Return(true)); + EXPECT_CALL(callback_, TimeToSendPacket(ssrc, sequence_number + 1, + clock_.TimeInMilliseconds(), true)) + .WillOnce(Return(true)); + EXPECT_CALL(callback_, TimeToSendPacket(ssrc, sequence_number + 3, + clock_.TimeInMilliseconds() + 33, + true)).WillOnce(Return(true)); + EXPECT_CALL(callback_, TimeToSendPacket(ssrc, sequence_number + 2, + clock_.TimeInMilliseconds() + 33, + true)).WillOnce(Return(true)); + + while (send_bucket_->QueueSizePackets() > 0) { + int time_until_process = send_bucket_->TimeUntilNextProcess(); + if (time_until_process <= 0) { + send_bucket_->Process(); + } else { + clock_.AdvanceTimeMilliseconds(time_until_process); + } + } + } +} + +TEST_F(PacedSenderTest, PaddingOveruse) { + uint32_t ssrc = 12346; + uint16_t sequence_number = 1234; + const int32_t kPacketSize = 1200; + + // Min bitrate 0 => no padding, padding budget will stay at 0. + send_bucket_->UpdateBitrate(60, 90, 0); + SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number++, + clock_.TimeInMilliseconds(), kPacketSize, false); + send_bucket_->Process(); + + // Add 30kbit padding. When increasing budget, media budget will increase from + // negative (overuse) while padding budget will increase form 0. + clock_.AdvanceTimeMilliseconds(5); + send_bucket_->UpdateBitrate(60, 90, 30); + + EXPECT_FALSE(send_bucket_->SendPacket( + PacedSender::kHighPriority, ssrc, sequence_number++, + clock_.TimeInMilliseconds(), kPacketSize, false)); + + // Don't send padding if queue is non-empty, even if padding budget > 0. + EXPECT_CALL(callback_, TimeToSendPadding(_)).Times(0); + send_bucket_->Process(); +} + } // namespace test } // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc index d5412668..0438b9f7 100644 --- a/modules/rtp_rtcp/source/rtp_sender.cc +++ b/modules/rtp_rtcp/source/rtp_sender.cc @@ -50,12 +50,17 @@ RTPSender::RTPSender(const int32_t id, FrameCountObserver* frame_count_observer, SendSideDelayObserver* send_side_delay_observer) : clock_(clock), + // TODO(holmer): Remove this conversion when we remove the use of + // TickTime. + clock_delta_ms_(clock_->TimeInMilliseconds() - + TickTime::MillisecondTimestamp()), bitrate_sent_(clock, this), id_(id), audio_configured_(audio), audio_(NULL), video_(NULL), paced_sender_(paced_sender), + last_capture_time_ms_sent_(0), send_critsect_(CriticalSectionWrapper::CreateCriticalSection()), transport_(transport), sending_media_(true), // Default to sending media. @@ -622,15 +627,10 @@ int32_t RTPSender::ReSendPacket(uint16_t packet_id, uint32_t min_resend_time) { } // Convert from TickTime to Clock since capture_time_ms is based on // TickTime. - // TODO(holmer): Remove this conversion when we remove the use of TickTime. - int64_t clock_delta_ms = clock_->TimeInMilliseconds() - - TickTime::MillisecondTimestamp(); - if (!paced_sender_->SendPacket(PacedSender::kHighPriority, - header.ssrc, - header.sequenceNumber, - capture_time_ms + clock_delta_ms, - length - header.headerLength, - true)) { + int64_t corrected_capture_tims_ms = capture_time_ms + clock_delta_ms_; + if (!paced_sender_->SendPacket( + PacedSender::kHighPriority, header.ssrc, header.sequenceNumber, + corrected_capture_tims_ms, length - header.headerLength, true)) { // We can't send the packet right now. // We will be called when it is time. return length; @@ -819,6 +819,10 @@ bool RTPSender::PrepareAndSendPacket(uint8_t* buffer, RtpUtility::RtpHeaderParser rtp_parser(buffer, length); RTPHeader rtp_header; rtp_parser.Parse(rtp_header); + if (!is_retransmit && rtp_header.markerBit) { + TRACE_EVENT_ASYNC_END0("webrtc_rtp", "PacedSend", capture_time_ms); + } + TRACE_EVENT_INSTANT2("webrtc_rtp", "PrepareAndSendPacket", "timestamp", rtp_header.timestamp, "seqnum", rtp_header.sequenceNumber); @@ -937,12 +941,18 @@ int32_t RTPSender::SendToNetwork( } if (paced_sender_ && storage != kDontStore) { - int64_t clock_delta_ms = clock_->TimeInMilliseconds() - - TickTime::MillisecondTimestamp(); + // Correct offset between implementations of millisecond time stamps in + // TickTime and Clock. + int64_t corrected_time_ms = capture_time_ms + clock_delta_ms_; if (!paced_sender_->SendPacket(priority, rtp_header.ssrc, - rtp_header.sequenceNumber, - capture_time_ms + clock_delta_ms, + rtp_header.sequenceNumber, corrected_time_ms, payload_length, false)) { + if (last_capture_time_ms_sent_ == 0 || + corrected_time_ms > last_capture_time_ms_sent_) { + last_capture_time_ms_sent_ = corrected_time_ms; + TRACE_EVENT_ASYNC_BEGIN1("webrtc_rtp", "PacedSend", corrected_time_ms, + "capture_time_ms", corrected_time_ms); + } // We can't send the packet right now. // We will be called when it is time. return 0; diff --git a/modules/rtp_rtcp/source/rtp_sender.h b/modules/rtp_rtcp/source/rtp_sender.h index b2f2e0c4..780baa1f 100644 --- a/modules/rtp_rtcp/source/rtp_sender.h +++ b/modules/rtp_rtcp/source/rtp_sender.h @@ -336,6 +336,7 @@ class RTPSender : public RTPSenderInterface, public Bitrate::Observer { bool IsFecPacket(const uint8_t* buffer, const RTPHeader& header) const; Clock* clock_; + int64_t clock_delta_ms_; Bitrate bitrate_sent_; int32_t id_; @@ -344,6 +345,7 @@ class RTPSender : public RTPSenderInterface, public Bitrate::Observer { RTPSenderVideo *video_; PacedSender *paced_sender_; + int64_t last_capture_time_ms_sent_; CriticalSectionWrapper *send_critsect_; Transport *transport_; diff --git a/video_engine/vie_encoder.cc b/video_engine/vie_encoder.cc index 9d6da977..3cb0ae70 100644 --- a/video_engine/vie_encoder.cc +++ b/video_engine/vie_encoder.cc @@ -470,9 +470,31 @@ bool ViEEncoder::EncoderPaused() const { std::max(static_cast(target_delay_ms_ * kEncoderPausePacerMargin), kMinPacingDelayMs); } + if (paced_sender_->ExpectedQueueTimeMs() > + PacedSender::kDefaultMaxQueueLengthMs) { + // Too much data in pacer queue, drop frame. + return true; + } return !network_is_transmitting_; } +void ViEEncoder::TraceFrameDropStart() { + // Start trace event only on the first frame after encoder is paused. + if (!encoder_paused_and_dropped_frame_) { + TRACE_EVENT_ASYNC_BEGIN0("webrtc", "EncoderPaused", this); + } + encoder_paused_and_dropped_frame_ = true; + return; +} + +void ViEEncoder::TraceFrameDropEnd() { + // End trace event on first frame after encoder resumes, if frame was dropped. + if (encoder_paused_and_dropped_frame_) { + TRACE_EVENT_ASYNC_END0("webrtc", "EncoderPaused", this); + } + encoder_paused_and_dropped_frame_ = false; +} + RtpRtcp* ViEEncoder::SendRtpRtcpModule() { return default_rtp_rtcp_.get(); } @@ -489,16 +511,10 @@ void ViEEncoder::DeliverFrame(int id, CriticalSectionScoped cs(data_cs_.get()); time_of_last_incoming_frame_ms_ = TickTime::MillisecondTimestamp(); if (EncoderPaused()) { - if (!encoder_paused_and_dropped_frame_) { - TRACE_EVENT_ASYNC_BEGIN0("webrtc", "EncoderPaused", this); - } - encoder_paused_and_dropped_frame_ = true; + TraceFrameDropStart(); return; } - if (encoder_paused_and_dropped_frame_) { - TRACE_EVENT_ASYNC_END0("webrtc", "EncoderPaused", this); - } - encoder_paused_and_dropped_frame_ = false; + TraceFrameDropEnd(); } // Convert render time, in ms, to RTP timestamp. @@ -702,15 +718,10 @@ void ViEEncoder::SetSenderBufferingMode(int target_delay_ms) { // Disable external frame-droppers. vcm_.EnableFrameDropper(false); vpm_.EnableTemporalDecimation(false); - // We don't put any limits on the pacer queue when running in buffered mode - // since the encoder will be paused if the queue grow too large. - paced_sender_->set_max_queue_length_ms(-1); } else { // Real-time mode - enable frame droppers. vpm_.EnableTemporalDecimation(true); vcm_.EnableFrameDropper(true); - paced_sender_->set_max_queue_length_ms( - PacedSender::kDefaultMaxQueueLengthMs); } } diff --git a/video_engine/vie_encoder.h b/video_engine/vie_encoder.h index 36f87faa..1e358def 100644 --- a/video_engine/vie_encoder.h +++ b/video_engine/vie_encoder.h @@ -192,6 +192,8 @@ class ViEEncoder int TimeToSendPadding(int bytes); private: bool EncoderPaused() const EXCLUSIVE_LOCKS_REQUIRED(data_cs_); + void TraceFrameDropStart() EXCLUSIVE_LOCKS_REQUIRED(data_cs_); + void TraceFrameDropEnd() EXCLUSIVE_LOCKS_REQUIRED(data_cs_); void UpdateHistograms(); -- cgit v1.2.3