diff --git a/webrtc/modules/audio_coding/acm2/acm_receiver.cc b/webrtc/modules/audio_coding/acm2/acm_receiver.cc index f6f1786f5e..2eacefdb9d 100644 --- a/webrtc/modules/audio_coding/acm2/acm_receiver.cc +++ b/webrtc/modules/audio_coding/acm2/acm_receiver.cc @@ -77,6 +77,11 @@ int AcmReceiver::InsertPacket(const WebRtcRTPHeader& rtp_header, uint32_t receive_timestamp = 0; const RTPHeader* header = &rtp_header.header; // Just a shorthand. + if (incoming_payload.empty()) { + neteq_->InsertEmptyPacket(rtp_header.header); + return 0; + } + { rtc::CritScope lock(&crit_sect_); diff --git a/webrtc/modules/audio_coding/acm2/audio_coding_module.cc b/webrtc/modules/audio_coding/acm2/audio_coding_module.cc index e22eb5a591..551ae057b4 100644 --- a/webrtc/modules/audio_coding/acm2/audio_coding_module.cc +++ b/webrtc/modules/audio_coding/acm2/audio_coding_module.cc @@ -1079,6 +1079,7 @@ rtc::Optional AudioCodingModuleImpl::ReceiveFormat() const { int AudioCodingModuleImpl::IncomingPacket(const uint8_t* incoming_payload, const size_t payload_length, const WebRtcRTPHeader& rtp_header) { + RTC_DCHECK_EQ(payload_length == 0, incoming_payload == nullptr); return receiver_.InsertPacket( rtp_header, rtc::ArrayView(incoming_payload, payload_length)); diff --git a/webrtc/modules/audio_coding/neteq/delay_manager.cc b/webrtc/modules/audio_coding/neteq/delay_manager.cc index 1f6cb0543c..2941126c02 100644 --- a/webrtc/modules/audio_coding/neteq/delay_manager.cc +++ b/webrtc/modules/audio_coding/neteq/delay_manager.cc @@ -374,6 +374,10 @@ void DelayManager::LastDecodedWasCngOrDtmf(bool it_was) { } } +void DelayManager::RegisterEmptyPacket() { + ++last_seq_no_; +} + bool DelayManager::SetMinimumDelay(int delay_ms) { // Minimum delay shouldn't be more than maximum delay, if any maximum is set. // Also, if possible check |delay| to less than 75% of diff --git a/webrtc/modules/audio_coding/neteq/delay_manager.h b/webrtc/modules/audio_coding/neteq/delay_manager.h index 78715720b8..f91c1dec33 100644 --- a/webrtc/modules/audio_coding/neteq/delay_manager.h +++ b/webrtc/modules/audio_coding/neteq/delay_manager.h @@ -95,6 +95,11 @@ class DelayManager { // speech. virtual void LastDecodedWasCngOrDtmf(bool it_was); + // Notify the delay manager that empty packets have been received. These are + // packets that are part of the sequence number series, so that an empty + // packet will shift the sequence numbers for the following packets. + virtual void RegisterEmptyPacket(); + // Accessors and mutators. // Assuming |delay| is in valid range. virtual bool SetMinimumDelay(int delay_ms); diff --git a/webrtc/modules/audio_coding/neteq/delay_manager_unittest.cc b/webrtc/modules/audio_coding/neteq/delay_manager_unittest.cc index 84ceb8871d..a6ee58d8e1 100644 --- a/webrtc/modules/audio_coding/neteq/delay_manager_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/delay_manager_unittest.cc @@ -266,6 +266,58 @@ TEST_F(DelayManagerTest, MinAndRequiredDelay) { EXPECT_EQ(kMinDelayPackets << 8, dm_->TargetLevel()); } +// Tests that skipped sequence numbers (simulating empty packets) are handled +// correctly. +TEST_F(DelayManagerTest, EmptyPacketsReported) { + SetPacketAudioLength(kFrameSizeMs); + // First packet arrival. + InsertNextPacket(); + + // Advance time by one frame size. + IncreaseTime(kFrameSizeMs); + + // Advance the sequence number by 5, simulating that 5 empty packets were + // received, but never inserted. + seq_no_ += 10; + for (int j = 0; j < 10; ++j) { + dm_->RegisterEmptyPacket(); + } + + // Second packet arrival. + // Expect detector update method to be called once with inter-arrival time + // equal to 1 packet, and (base) target level equal to 1 as well. + // Return false to indicate no peaks found. + EXPECT_CALL(detector_, Update(1, 1)).WillOnce(Return(false)); + InsertNextPacket(); + + EXPECT_EQ(1 << 8, dm_->TargetLevel()); // In Q8. +} + +// Same as above, but do not call RegisterEmptyPacket. Observe the target level +// increase dramatically. +TEST_F(DelayManagerTest, EmptyPacketsNotReported) { + SetPacketAudioLength(kFrameSizeMs); + // First packet arrival. + InsertNextPacket(); + + // Advance time by one frame size. + IncreaseTime(kFrameSizeMs); + + // Advance the sequence number by 5, simulating that 5 empty packets were + // received, but never inserted. + seq_no_ += 10; + + // Second packet arrival. + // Expect detector update method to be called once with inter-arrival time + // equal to 1 packet, and (base) target level equal to 1 as well. + // Return false to indicate no peaks found. + EXPECT_CALL(detector_, Update(10, 10)).WillOnce(Return(false)); + InsertNextPacket(); + + // Note 10 times higher target value. + EXPECT_EQ(10 * 1 << 8, dm_->TargetLevel()); // In Q8. +} + TEST_F(DelayManagerTest, Failures) { // Wrong sample rate. EXPECT_EQ(-1, dm_->Update(0, 0, -1)); diff --git a/webrtc/modules/audio_coding/neteq/include/neteq.h b/webrtc/modules/audio_coding/neteq/include/neteq.h index f03458092e..45aae11048 100644 --- a/webrtc/modules/audio_coding/neteq/include/neteq.h +++ b/webrtc/modules/audio_coding/neteq/include/neteq.h @@ -145,6 +145,12 @@ class NetEq { rtc::ArrayView payload, uint32_t receive_timestamp) = 0; + // Lets NetEq know that a packet arrived with an empty payload. This typically + // happens when empty packets are used for probing the network channel, and + // these packets use RTP sequence numbers from the same series as the actual + // audio packets. + virtual void InsertEmptyPacket(const RTPHeader& rtp_header) = 0; + // Instructs NetEq to deliver 10 ms of audio data. The data is written to // |audio_frame|. All data in |audio_frame| is wiped; |data_|, |speech_type_|, // |num_channels_|, |sample_rate_hz_|, |samples_per_channel_|, and diff --git a/webrtc/modules/audio_coding/neteq/mock/mock_delay_manager.h b/webrtc/modules/audio_coding/neteq/mock/mock_delay_manager.h index 7a66b68c63..85ed71dc70 100644 --- a/webrtc/modules/audio_coding/neteq/mock/mock_delay_manager.h +++ b/webrtc/modules/audio_coding/neteq/mock/mock_delay_manager.h @@ -45,6 +45,7 @@ class MockDelayManager : public DelayManager { void(int* lower_limit, int* higher_limit)); MOCK_CONST_METHOD0(TargetLevel, int()); + MOCK_METHOD0(RegisterEmptyPacket, void()); MOCK_METHOD1(set_extra_delay_ms, void(int16_t delay)); MOCK_CONST_METHOD0(base_target_level, diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl.cc b/webrtc/modules/audio_coding/neteq/neteq_impl.cc index 2d91b8c1c5..f512d75a56 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_impl.cc @@ -146,6 +146,14 @@ int NetEqImpl::InsertPacket(const RTPHeader& rtp_header, return kOK; } +void NetEqImpl::InsertEmptyPacket(const RTPHeader& /*rtp_header*/) { + // TODO(henrik.lundin) Handle NACK as well. This will make use of the + // rtp_header parameter. + // https://bugs.chromium.org/p/webrtc/issues/detail?id=7611 + rtc::CritScope lock(&crit_sect_); + delay_manager_->RegisterEmptyPacket(); +} + namespace { void SetAudioFrameActivityAndType(bool vad_enabled, NetEqImpl::OutputType type, diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl.h b/webrtc/modules/audio_coding/neteq/neteq_impl.h index 8a789bc037..343676532d 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl.h +++ b/webrtc/modules/audio_coding/neteq/neteq_impl.h @@ -109,6 +109,8 @@ class NetEqImpl : public webrtc::NetEq { rtc::ArrayView payload, uint32_t receive_timestamp) override; + void InsertEmptyPacket(const RTPHeader& rtp_header) override; + int GetAudio(AudioFrame* audio_frame, bool* muted) override; void SetCodecs(const std::map& codecs) override; diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc b/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc index fb87111fd0..d1703e91fb 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc @@ -1284,6 +1284,21 @@ TEST_F(NetEqImplTest, TargetDelayMs) { EXPECT_EQ(17 * 30, neteq_->TargetDelayMs()); } +TEST_F(NetEqImplTest, InsertEmptyPacket) { + UseNoMocks(); + use_mock_delay_manager_ = true; + CreateInstance(); + + RTPHeader rtp_header; + rtp_header.payloadType = 17; + rtp_header.sequenceNumber = 0x1234; + rtp_header.timestamp = 0x12345678; + rtp_header.ssrc = 0x87654321; + + EXPECT_CALL(*mock_delay_manager_, RegisterEmptyPacket()); + neteq_->InsertEmptyPacket(rtp_header); +} + class Decoder120ms : public AudioDecoder { public: Decoder120ms(int sample_rate_hz, SpeechType speech_type) diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc b/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc index acc5926e5b..7b66fd3490 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc @@ -212,9 +212,13 @@ int32_t RTPReceiverAudio::ParseAudioCodecSpecific( size_t payload_length, const AudioPayload& audio_specific, bool is_red) { - - if (payload_length == 0) { - return 0; + RTC_DCHECK_GE(payload_length, rtp_header->header.paddingLength); + const size_t payload_data_length = + payload_length - rtp_header->header.paddingLength; + if (payload_data_length == 0) { + rtp_header->type.Audio.isCNG = false; + rtp_header->frameType = kEmptyFrame; + return data_callback_->OnReceivedPayloadData(nullptr, 0, rtp_header); } bool telephone_event_packet = @@ -229,16 +233,17 @@ int32_t RTPReceiverAudio::ParseAudioCodecSpecific( // | event |E|R| volume | duration | // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ // - if (payload_length % 4 != 0) { + if (payload_data_length % 4 != 0) { return -1; } - size_t number_of_events = payload_length / 4; + size_t number_of_events = payload_data_length / 4; // sanity if (number_of_events >= MAX_NUMBER_OF_PARALLEL_TELEPHONE_EVENTS) { number_of_events = MAX_NUMBER_OF_PARALLEL_TELEPHONE_EVENTS; } for (size_t n = 0; n < number_of_events; ++n) { + RTC_DCHECK_GE(payload_data_length, (4 * n) + 2); bool end = (payload_data[(4 * n) + 1] & 0x80) ? true : false; std::set::iterator event = @@ -291,17 +296,18 @@ int32_t RTPReceiverAudio::ParseAudioCodecSpecific( } } // TODO(holmer): Break this out to have RED parsing handled generically. + RTC_DCHECK_GT(payload_data_length, 0); if (is_red && !(payload_data[0] & 0x80)) { // we recive only one frame packed in a RED packet remove the RED wrapper rtp_header->header.payloadType = payload_data[0]; // only one frame in the RED strip the one byte to help NetEq return data_callback_->OnReceivedPayloadData( - payload_data + 1, payload_length - 1, rtp_header); + payload_data + 1, payload_data_length - 1, rtp_header); } rtp_header->type.Audio.channel = audio_specific.channels; - return data_callback_->OnReceivedPayloadData( - payload_data, payload_length, rtp_header); + return data_callback_->OnReceivedPayloadData(payload_data, + payload_data_length, rtp_header); } } // namespace webrtc