From c908c5575f8d3556eecc087b1e52d60562693d9a Mon Sep 17 00:00:00 2001 From: Philipp Hancke Date: Mon, 3 Aug 2020 15:08:42 +0200 Subject: [PATCH] red: do not generate packets which are > 1200 bytes and do not generate redundancy for packets that are larger than 1024 bytes which is the maximum size red can encode. Bug: webrtc:11640 Change-Id: I211cb196eee2a0659f22a601a6dee4b7dd4e5116 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/178781 Commit-Queue: Harald Alvestrand Reviewed-by: Karl Wiberg Reviewed-by: Harald Alvestrand Cr-Commit-Position: refs/heads/master@{#31846} --- .../codecs/red/audio_encoder_copy_red.cc | 34 ++++++++++++++----- .../codecs/red/audio_encoder_copy_red.h | 6 +++- .../red/audio_encoder_copy_red_unittest.cc | 26 ++++++++++++++ 3 files changed, 57 insertions(+), 9 deletions(-) diff --git a/modules/audio_coding/codecs/red/audio_encoder_copy_red.cc b/modules/audio_coding/codecs/red/audio_encoder_copy_red.cc index 2bfd2c44df..1432e3182f 100644 --- a/modules/audio_coding/codecs/red/audio_encoder_copy_red.cc +++ b/modules/audio_coding/codecs/red/audio_encoder_copy_red.cc @@ -19,7 +19,10 @@ #include "rtc_base/checks.h" namespace webrtc { -static const int kRedMaxPacketSize = 1 << 10; +// RED packets must be less than 1024 bytes to fit the 10 bit block length. +static constexpr const int kRedMaxPacketSize = 1 << 10; +// The typical MTU is 1200 bytes. +static constexpr const size_t kAudioMaxRtpPacketLen = 1200; AudioEncoderCopyRed::Config::Config() = default; AudioEncoderCopyRed::Config::Config(Config&&) = default; @@ -27,6 +30,7 @@ AudioEncoderCopyRed::Config::~Config() = default; AudioEncoderCopyRed::AudioEncoderCopyRed(Config&& config) : speech_encoder_(std::move(config.speech_encoder)), + max_packet_length_(kAudioMaxRtpPacketLen), red_payload_type_(config.payload_type) { RTC_CHECK(speech_encoder_) << "Speech encoder not provided."; } @@ -57,12 +61,16 @@ int AudioEncoderCopyRed::GetTargetBitrate() const { return speech_encoder_->GetTargetBitrate(); } -size_t AudioEncoderCopyRed::CalculateHeaderLength() const { +size_t AudioEncoderCopyRed::CalculateHeaderLength(size_t encoded_bytes) const { size_t header_size = 1; - if (secondary_info_.encoded_bytes > 0) { + size_t bytes_available = max_packet_length_ - encoded_bytes; + if (secondary_info_.encoded_bytes > 0 && + secondary_info_.encoded_bytes < bytes_available) { header_size += 4; + bytes_available -= secondary_info_.encoded_bytes; } - if (tertiary_info_.encoded_bytes > 0) { + if (tertiary_info_.encoded_bytes > 0 && + tertiary_info_.encoded_bytes < bytes_available) { header_size += 4; } return header_size > 1 ? header_size : 0; @@ -78,19 +86,22 @@ AudioEncoder::EncodedInfo AudioEncoderCopyRed::EncodeImpl( RTC_CHECK(info.redundant.empty()) << "Cannot use nested redundant encoders."; RTC_DCHECK_EQ(primary_encoded.size(), info.encoded_bytes); - if (info.encoded_bytes == 0) { + if (info.encoded_bytes == 0 || info.encoded_bytes > kRedMaxPacketSize) { return info; } + RTC_DCHECK_GT(max_packet_length_, info.encoded_bytes); // Allocate room for RFC 2198 header if there is redundant data. // Otherwise this will send the primary payload type without // wrapping in RED. - const size_t header_length_bytes = CalculateHeaderLength(); + const size_t header_length_bytes = CalculateHeaderLength(info.encoded_bytes); encoded->SetSize(header_length_bytes); size_t header_offset = 0; + size_t bytes_available = max_packet_length_ - info.encoded_bytes; if (tertiary_info_.encoded_bytes > 0 && - tertiary_info_.encoded_bytes < kRedMaxPacketSize) { + tertiary_info_.encoded_bytes + secondary_info_.encoded_bytes < + bytes_available) { encoded->AppendData(tertiary_encoded_); const uint32_t timestamp_delta = @@ -101,10 +112,11 @@ AudioEncoder::EncodedInfo AudioEncoderCopyRed::EncodeImpl( (timestamp_delta << 2) | (tertiary_info_.encoded_bytes >> 8)); encoded->data()[header_offset + 3] = tertiary_info_.encoded_bytes & 0xff; header_offset += 4; + bytes_available -= tertiary_info_.encoded_bytes; } if (secondary_info_.encoded_bytes > 0 && - secondary_info_.encoded_bytes < kRedMaxPacketSize) { + secondary_info_.encoded_bytes < bytes_available) { encoded->AppendData(secondary_encoded_); const uint32_t timestamp_delta = @@ -115,6 +127,7 @@ AudioEncoder::EncodedInfo AudioEncoderCopyRed::EncodeImpl( (timestamp_delta << 2) | (secondary_info_.encoded_bytes >> 8)); encoded->data()[header_offset + 3] = secondary_info_.encoded_bytes & 0xff; header_offset += 4; + bytes_available -= secondary_info_.encoded_bytes; } encoded->AppendData(primary_encoded); @@ -200,4 +213,9 @@ AudioEncoderCopyRed::GetFrameLengthRange() const { return speech_encoder_->GetFrameLengthRange(); } +void AudioEncoderCopyRed::OnReceivedOverhead(size_t overhead_bytes_per_packet) { + max_packet_length_ = kAudioMaxRtpPacketLen - overhead_bytes_per_packet; + return speech_encoder_->OnReceivedOverhead(overhead_bytes_per_packet); +} + } // namespace webrtc diff --git a/modules/audio_coding/codecs/red/audio_encoder_copy_red.h b/modules/audio_coding/codecs/red/audio_encoder_copy_red.h index 4d7fc404f1..9806772ba4 100644 --- a/modules/audio_coding/codecs/red/audio_encoder_copy_red.h +++ b/modules/audio_coding/codecs/red/audio_encoder_copy_red.h @@ -62,6 +62,7 @@ class AudioEncoderCopyRed final : public AudioEncoder { void OnReceivedUplinkBandwidth( int target_audio_bitrate_bps, absl::optional bwe_period_ms) override; + void OnReceivedOverhead(size_t overhead_bytes_per_packet) override; absl::optional> GetFrameLengthRange() const override; @@ -71,13 +72,16 @@ class AudioEncoderCopyRed final : public AudioEncoder { rtc::Buffer* encoded) override; private: - size_t CalculateHeaderLength() const; + size_t CalculateHeaderLength(size_t encoded_bytes) const; + std::unique_ptr speech_encoder_; + size_t max_packet_length_; int red_payload_type_; rtc::Buffer secondary_encoded_; EncodedInfoLeaf secondary_info_; rtc::Buffer tertiary_encoded_; EncodedInfoLeaf tertiary_info_; + RTC_DISALLOW_COPY_AND_ASSIGN(AudioEncoderCopyRed); }; diff --git a/modules/audio_coding/codecs/red/audio_encoder_copy_red_unittest.cc b/modules/audio_coding/codecs/red/audio_encoder_copy_red_unittest.cc index fbc0b8aa38..33527997b5 100644 --- a/modules/audio_coding/codecs/red/audio_encoder_copy_red_unittest.cc +++ b/modules/audio_coding/codecs/red/audio_encoder_copy_red_unittest.cc @@ -355,6 +355,32 @@ TEST_F(AudioEncoderCopyRedTest, CheckRFC2198Header) { EXPECT_EQ(encoded_[8], primary_payload_type); } +TEST_F(AudioEncoderCopyRedTest, RespectsPayloadMTU) { + const int primary_payload_type = red_payload_type_ + 1; + AudioEncoder::EncodedInfo info; + info.encoded_bytes = 600; + info.encoded_timestamp = timestamp_; + info.payload_type = primary_payload_type; + + EXPECT_CALL(*mock_encoder_, EncodeImpl(_, _, _)) + .WillOnce(Invoke(MockAudioEncoder::FakeEncoding(info))); + Encode(); + info.encoded_timestamp = timestamp_; // update timestamp. + info.encoded_bytes = 500; + EXPECT_CALL(*mock_encoder_, EncodeImpl(_, _, _)) + .WillOnce(Invoke(MockAudioEncoder::FakeEncoding(info))); + Encode(); // Second call will produce a redundant encoding. + + EXPECT_EQ(encoded_.size(), 5u + 600u + 500u); + + info.encoded_timestamp = timestamp_; // update timestamp. + info.encoded_bytes = 400; + EXPECT_CALL(*mock_encoder_, EncodeImpl(_, _, _)) + .WillOnce(Invoke(MockAudioEncoder::FakeEncoding(info))); + Encode(); // Third call will drop the oldest packet. + EXPECT_EQ(encoded_.size(), 5u + 500u + 400u); +} + #if GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID) // This test fixture tests various error conditions that makes the