red: handle opus dtx 400ms timestamp gap
by not encoding redundancy. The timestamp gap of 400ms means a
rtp timestamp difference of 19200 which would overflow the 14 bit
RED timestamp difference field.
To test in Chrome, go to
https://webrtc.github.io/samples/src/content/peerconnection/audio/
set `useDtx = true` in the console and be very quiet.
BUG=webrtc:13182,webrtc:11640
Change-Id: I1cedc7d846ac7ae821bb7cb5c0f37a17511ac727
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/231940
Reviewed-by: Henrik Lundin <henrik.lundin@webrtc.org>
Commit-Queue: Henrik Lundin <henrik.lundin@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35005}
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 0e9e100..9643c7b 100644
--- a/modules/audio_coding/codecs/red/audio_encoder_copy_red.cc
+++ b/modules/audio_coding/codecs/red/audio_encoder_copy_red.cc
@@ -24,6 +24,8 @@
static constexpr const int kRedMaxPacketSize =
1 << 10; // RED packets must be less than 1024 bytes to fit the 10 bit
// block length.
+static constexpr const size_t kRedMaxTimestampDelta =
+ 1 << 14; // RED packets can encode a timestamp delta of 14 bits.
static constexpr const size_t kAudioMaxRtpPacketLen =
1200; // The typical MTU is 1200 bytes.
@@ -100,7 +102,7 @@
RTC_CHECK(info.redundant.empty()) << "Cannot use nested redundant encoders.";
RTC_DCHECK_EQ(primary_encoded_.size(), info.encoded_bytes);
- if (info.encoded_bytes == 0 || info.encoded_bytes > kRedMaxPacketSize) {
+ if (info.encoded_bytes == 0 || info.encoded_bytes >= kRedMaxPacketSize) {
return info;
}
RTC_DCHECK_GT(max_packet_length_, info.encoded_bytes);
@@ -110,7 +112,9 @@
auto it = redundant_encodings_.begin();
// Determine how much redundancy we can fit into our packet by
- // iterating forward.
+ // iterating forward. This is determined both by the length as well
+ // as the timestamp difference. The latter can occur with opus DTX which
+ // has timestamp gaps of 400ms which exceeds REDs timestamp delta field size.
for (; it != redundant_encodings_.end(); it++) {
if (bytes_available < kRedHeaderLength + it->first.encoded_bytes) {
break;
@@ -118,6 +122,9 @@
if (it->first.encoded_bytes == 0) {
break;
}
+ if (rtp_timestamp - it->first.encoded_timestamp >= kRedMaxTimestampDelta) {
+ break;
+ }
bytes_available -= kRedHeaderLength + it->first.encoded_bytes;
header_length_bytes += kRedHeaderLength;
}
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 168ac3a..0eeac01 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
@@ -583,6 +583,29 @@
EXPECT_EQ(encoded_.size(), 5u + 500u + 400u);
}
+TEST_F(AudioEncoderCopyRedTest, LargeTimestampGap) {
+ const int primary_payload_type = red_payload_type_ + 1;
+ AudioEncoder::EncodedInfo info;
+ info.encoded_bytes = 100;
+ info.encoded_timestamp = timestamp_;
+ info.payload_type = primary_payload_type;
+
+ EXPECT_CALL(*mock_encoder_, EncodeImpl(_, _, _))
+ .WillOnce(Invoke(MockAudioEncoder::FakeEncoding(info)));
+ Encode();
+ // Update timestamp to simulate a 400ms gap like the one
+ // opus DTX causes.
+ timestamp_ += 19200;
+ info.encoded_timestamp = timestamp_; // update timestamp.
+ info.encoded_bytes = 200;
+ EXPECT_CALL(*mock_encoder_, EncodeImpl(_, _, _))
+ .WillOnce(Invoke(MockAudioEncoder::FakeEncoding(info)));
+ Encode();
+
+ // The old packet will be dropped.
+ EXPECT_EQ(encoded_.size(), 1u + 200u);
+}
+
#if GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
// This test fixture tests various error conditions that makes the
