Moved Opus-specific payload splitting into AudioDecoderOpus.

webrtc/modules/audio_coding/neteq/neteq_network_stats_unittest.cc

Index: webrtc/modules/audio_coding/neteq/neteq_network_stats_unittest.cc
diff --git a/webrtc/modules/audio_coding/neteq/neteq_network_stats_unittest.cc b/webrtc/modules/audio_coding/neteq/neteq_network_stats_unittest.cc
index 15c60a83b5dedf5f3a44210a702580479e23a3c7..e8254a1d73663a74d20cb94d8615d2142e491714 100644
--- a/webrtc/modules/audio_coding/neteq/neteq_network_stats_unittest.cc
+++ b/webrtc/modules/audio_coding/neteq/neteq_network_stats_unittest.cc
@@ -39,18 +39,56 @@ class MockAudioDecoder final : public AudioDecoder {
 
   MOCK_METHOD0(Reset, void());
 
-  int PacketDuration(const uint8_t* encoded,
-                     size_t encoded_len) const /* override */ {
-    return kPacketDuration;
+  class MockFrame : public AudioDecoder::EncodedAudioFrame {
+   public:
+    MockFrame(size_t num_channels) : num_channels_(num_channels) {}
+
+    size_t Duration() const override { return kPacketDuration; }
+
+    rtc::Optional<DecodeResult> Decode(
+        rtc::ArrayView<int16_t> decoded) const override {
+      const size_t output_size =
+          sizeof(int16_t) * kPacketDuration * num_channels_;
+      if (decoded.size() >= output_size) {
+        memset(decoded.data(), 0,
+               sizeof(int16_t) * kPacketDuration * num_channels_);
+        return rtc::Optional<DecodeResult>(
+            {kPacketDuration * num_channels_, kSpeech});
+      } else {
+        EXPECT_GE(decoded.size(), output_size);

[kwiberg-webrtc, 2016/09/20 09:14:45]

Isn't it always the case that !(decoded.size() >= output_size) in this else
clause, so that this EXPECT always fails?

[ossu, 2016/09/20 13:51:56]

Yup! Putting an ASSERT in here won't work, since we don't return void.
I'll clarify that I'm using EXPECT_GE just to get the proper printouts (as
opposed to using ADD_FAILURE() like I did below).

[kwiberg-webrtc, 2016/09/20 14:56:22]

Does something like this work?

  ADD_FAILURE() << "Expected " << decoded.size() << " to be >= "
      << output_size << ", but the world is a cruel place.";

Alternatively, something like this?

  EXPECT_GE(a, b);
  if (a < b) {
    // The expectation failed.
    return empty optional;
  }
  do_stuff_that_requires_that(a >= b);

Just trying my best to turn that comment into something the compiler can read.
:-)

[ossu, 2016/09/20 15:45:21]

1) Yes, but it's more to write.
2) Isn't that more-or-less what's already being done, except with the check
being >= and the executing code is in the if-clause. Is there a benefit to
having the "correct" code running outside of the if-statement?

[kwiberg-webrtc, 2016/09/20 16:19:15]

1) It's more code, but you don't need that multiline comment that explains what
the code is doing. You'll be replacing 1 line of code + 3 lines of comment with
2 lines of code.

2) Yes, it's doing the same thing, but again structured in a way that should let
you eliminate the comment that explains what's going on. (I think I like the
first suggestion better, though.)

+        return rtc::Optional<DecodeResult>();
+      }
+    }
+
+   private:
+    const size_t num_channels_;
+  };
+
+  std::vector<ParseResult> ParsePayload(rtc::Buffer&& payload,
+                                        uint32_t timestamp) /* override */ {
+    std::vector<ParseResult> results;
+    if (fec_enabled_) {
+      std::unique_ptr<MockFrame> fec_frame(new MockFrame(num_channels_));
+      results.emplace_back(timestamp - kPacketDuration, 1,
+                           std::move(fec_frame));
+    }
+
+    std::unique_ptr<MockFrame> frame(new MockFrame(num_channels_));
+    results.emplace_back(timestamp, 0, std::move(frame));
+    return results;
   }
 
-  int PacketDurationRedundant(const uint8_t* encoded,
-                              size_t encoded_len) const /* override */ {
+  int PacketDuration(const uint8_t* encoded, size_t encoded_len) const
+  /* override */ {
+    ADD_FAILURE() << "Since going through ParsePayload, PacketDuration should "
+                     "never get called.";
     return kPacketDuration;
   }
 
   bool PacketHasFec(
       const uint8_t* encoded, size_t encoded_len) const /* override */ {
+    ADD_FAILURE() << "Since going through ParsePayload, PacketHasFec should "
+                     "never get called.";
     return fec_enabled_;
   }
 
@@ -63,24 +101,14 @@ class MockAudioDecoder final : public AudioDecoder {
   bool fec_enabled() const { return fec_enabled_; }
 
  protected:
-  // Override the following methods such that no actual payload is needed.
   int DecodeInternal(const uint8_t* encoded,
                      size_t encoded_len,
-                     int /*sample_rate_hz*/,
+                     int sample_rate_hz,
                      int16_t* decoded,
                      SpeechType* speech_type) /* override */ {
-    *speech_type = kSpeech;
-    memset(decoded, 0, sizeof(int16_t) * kPacketDuration * Channels());
-    return kPacketDuration * Channels();
-  }
-
-  int DecodeRedundantInternal(const uint8_t* encoded,
-                              size_t encoded_len,
-                              int sample_rate_hz,
-                              int16_t* decoded,
-                              SpeechType* speech_type) /* override */ {
-    return DecodeInternal(encoded, encoded_len, sample_rate_hz, decoded,
-                          speech_type);
+    ADD_FAILURE() << "Since going through ParsePayload, DecodeInternal should "
+                     "never get called.";
+    return -1;
   }
 
  private: