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}
This commit is contained in:
committed by
WebRTC LUCI CQ
parent
b01e6457fe
commit
1061686107
@ -24,6 +24,8 @@ namespace webrtc {
|
||||
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 @@ 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 || 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 @@ AudioEncoder::EncodedInfo AudioEncoderCopyRed::EncodeImpl(
|
||||
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 @@ AudioEncoder::EncodedInfo AudioEncoderCopyRed::EncodeImpl(
|
||||
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;
|
||||
}
|
||||
|
||||
@ -583,6 +583,29 @@ TEST_F(AudioEncoderCopyRedTest, RespectsPayloadMTU) {
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user