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}

modules/audio_coding/codecs/red/audio_encoder_copy_red.cc

@@ -161,11 +161,11 @@ AudioEncoder::EncodedInfo AudioEncoderCopyRed::EncodeImpl(
   }
 
   // Shift the redundant encodings.
-  it = redundant_encodings_.begin();
+  auto rit = redundant_encodings_.rbegin();
-  for (auto next = std::next(it); next != redundant_encodings_.end();
+  for (auto next = std::next(rit); next != redundant_encodings_.rend();
-       it++, next = std::next(it)) {
+       rit++, next = std::next(rit)) {
-    next->first = it->first;
+    rit->first = next->first;
-    next->second.SetData(it->second);
+    rit->second.SetData(next->second);
   }
   it = redundant_encodings_.begin();
   it->first = info;

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"
@@ -165,6 +166,41 @@ TEST_F(AudioEncoderCopyRedTest, CheckNoOutput) {
   ASSERT_EQ(2u, encoded_info_.redundant.size());
 }
 
+// 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) {
@@ -183,7 +219,7 @@ TEST_F(AudioEncoderCopyRedTest, CheckPayloadSizes) {
   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 @@ TEST_F(AudioEncoderCopyRedTest, CheckPayloadSizes) {
   }
 }
 
+// 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_;