red: fix redundancy shift and add tests
fixes an incorrect redundancy shift and add tests that would have caught this bug.
BUG=webrtc:11640
Change-Id: I6fe2fb21587fffc5fee4d403ac898e12d525a1cd
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/224120
Reviewed-by: Henrik Lundin <henrik.lundin@webrtc.org>
Commit-Queue: Henrik Lundin <henrik.lundin@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34702}
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 1ca7a84..07dd3ca 100644
--- a/modules/audio_coding/codecs/red/audio_encoder_copy_red.cc
+++ b/modules/audio_coding/codecs/red/audio_encoder_copy_red.cc
@@ -161,11 +161,11 @@
}
// Shift the redundant encodings.
- it = redundant_encodings_.begin();
- for (auto next = std::next(it); next != redundant_encodings_.end();
- it++, next = std::next(it)) {
- next->first = it->first;
- next->second.SetData(it->second);
+ auto rit = redundant_encodings_.rbegin();
+ for (auto next = std::next(rit); next != redundant_encodings_.rend();
+ rit++, next = std::next(rit)) {
+ rit->first = next->first;
+ rit->second.SetData(next->second);
}
it = redundant_encodings_.begin();
it->first = info;
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 ddd8244..06404ab 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
@@ -15,6 +15,7 @@
#include "rtc_base/checks.h"
#include "rtc_base/numerics/safe_conversions.h"
+#include "test/field_trial.h"
#include "test/gtest.h"
#include "test/mock_audio_encoder.h"
#include "test/testsupport/rtc_expect_death.h"
@@ -166,6 +167,41 @@
}
// Checks that the correct payload sizes are populated into the redundancy
+// information. Variant with a redundancy level of 1.
+TEST_F(AudioEncoderCopyRedTest, CheckPayloadSizesSingle) {
+ webrtc::test::ScopedFieldTrials field_trials(
+ "WebRTC-Audio-Red-For-Opus/Enabled-1/");
+ // Recreate the RED encoder to take the new field trial setting into account.
+ AudioEncoderCopyRed::Config config;
+ config.payload_type = red_payload_type_;
+ config.speech_encoder = std::move(red_->ReclaimContainedEncoders()[0]);
+ red_.reset(new AudioEncoderCopyRed(std::move(config)));
+
+ // Let the mock encoder return payload sizes 1, 2, 3, ..., 10 for the sequence
+ // of calls.
+ static const int kNumPackets = 10;
+ InSequence s;
+ for (int encode_size = 1; encode_size <= kNumPackets; ++encode_size) {
+ EXPECT_CALL(*mock_encoder_, EncodeImpl(_, _, _))
+ .WillOnce(Invoke(MockAudioEncoder::FakeEncoding(encode_size)));
+ }
+
+ // First call is a special case, since it does not include a secondary
+ // payload.
+ Encode();
+ EXPECT_EQ(0u, encoded_info_.redundant.size());
+ EXPECT_EQ(1u, encoded_info_.encoded_bytes);
+
+ for (size_t i = 2; i <= kNumPackets; ++i) {
+ Encode();
+ ASSERT_EQ(2u, encoded_info_.redundant.size());
+ EXPECT_EQ(i, encoded_info_.redundant[1].encoded_bytes);
+ EXPECT_EQ(i - 1, encoded_info_.redundant[0].encoded_bytes);
+ EXPECT_EQ(5 + i + (i - 1), encoded_info_.encoded_bytes);
+ }
+}
+
+// Checks that the correct payload sizes are populated into the redundancy
// information.
TEST_F(AudioEncoderCopyRedTest, CheckPayloadSizes) {
// Let the mock encoder return payload sizes 1, 2, 3, ..., 10 for the sequence
@@ -183,7 +219,7 @@
EXPECT_EQ(0u, encoded_info_.redundant.size());
EXPECT_EQ(1u, encoded_info_.encoded_bytes);
- // Second call is also special since it does not include a ternary
+ // Second call is also special since it does not include a tertiary
// payload.
Encode();
EXPECT_EQ(2u, encoded_info_.redundant.size());
@@ -199,6 +235,56 @@
}
}
+// Checks that the correct payload sizes are populated into the redundancy
+// information. Variant with a redundancy level of 3.
+TEST_F(AudioEncoderCopyRedTest, CheckPayloadSizesTriple) {
+ webrtc::test::ScopedFieldTrials field_trials(
+ "WebRTC-Audio-Red-For-Opus/Enabled-3/");
+ // Recreate the RED encoder to take the new field trial setting into account.
+ AudioEncoderCopyRed::Config config;
+ config.payload_type = red_payload_type_;
+ config.speech_encoder = std::move(red_->ReclaimContainedEncoders()[0]);
+ red_.reset(new AudioEncoderCopyRed(std::move(config)));
+
+ // Let the mock encoder return payload sizes 1, 2, 3, ..., 10 for the sequence
+ // of calls.
+ static const int kNumPackets = 10;
+ InSequence s;
+ for (int encode_size = 1; encode_size <= kNumPackets; ++encode_size) {
+ EXPECT_CALL(*mock_encoder_, EncodeImpl(_, _, _))
+ .WillOnce(Invoke(MockAudioEncoder::FakeEncoding(encode_size)));
+ }
+
+ // First call is a special case, since it does not include a secondary
+ // payload.
+ Encode();
+ EXPECT_EQ(0u, encoded_info_.redundant.size());
+ EXPECT_EQ(1u, encoded_info_.encoded_bytes);
+
+ // Second call is also special since it does not include a tertiary
+ // payload.
+ Encode();
+ EXPECT_EQ(2u, encoded_info_.redundant.size());
+ EXPECT_EQ(8u, encoded_info_.encoded_bytes);
+
+ // Third call is also special since it does not include a quaternary
+ // payload.
+ Encode();
+ EXPECT_EQ(3u, encoded_info_.redundant.size());
+ EXPECT_EQ(15u, encoded_info_.encoded_bytes);
+
+ for (size_t i = 4; i <= kNumPackets; ++i) {
+ Encode();
+ ASSERT_EQ(4u, encoded_info_.redundant.size());
+ EXPECT_EQ(i, encoded_info_.redundant[3].encoded_bytes);
+ EXPECT_EQ(i - 1, encoded_info_.redundant[2].encoded_bytes);
+ EXPECT_EQ(i - 2, encoded_info_.redundant[1].encoded_bytes);
+ EXPECT_EQ(i - 3, encoded_info_.redundant[0].encoded_bytes);
+ EXPECT_EQ(13 + i + (i - 1) + (i - 2) + (i - 3),
+ encoded_info_.encoded_bytes);
+ }
+}
+
// Checks that the correct timestamps are returned.
TEST_F(AudioEncoderCopyRedTest, CheckTimestamps) {
uint32_t primary_timestamp = timestamp_;
