NetEqTest: Extend the callback structure

webrtc/modules/audio_coding/neteq/tools/neteq_test.cc

Index: webrtc/modules/audio_coding/neteq/tools/neteq_test.cc
diff --git a/webrtc/modules/audio_coding/neteq/tools/neteq_test.cc b/webrtc/modules/audio_coding/neteq/tools/neteq_test.cc
index 7cca0fed8abfecc5aa1b4a04566cd657461afbfb..6ff46bcd90b537c4674d953c12ab8bfb3fe173e8 100644
--- a/webrtc/modules/audio_coding/neteq/tools/neteq_test.cc
+++ b/webrtc/modules/audio_coding/neteq/tools/neteq_test.cc
@@ -41,11 +41,11 @@ NetEqTest::NetEqTest(const NetEq::Config& config,
                      const ExtDecoderMap& ext_codecs,
                      std::unique_ptr<NetEqInput> input,
                      std::unique_ptr<AudioSink> output,
-                     NetEqTestErrorCallback* error_callback)
+                     Callbacks callbacks)
     : neteq_(NetEq::Create(config, CreateBuiltinAudioDecoderFactory())),
       input_(std::move(input)),
       output_(std::move(output)),
-      error_callback_(error_callback),
+      callbacks_(callbacks),
       sample_rate_hz_(config.sample_rate_hz) {
   RTC_CHECK(!config.enable_muted_state)
       << "The code does not handle enable_muted_state";
@@ -69,25 +69,37 @@ int64_t NetEqTest::Run() {
           packet_data->header,
           rtc::ArrayView<const uint8_t>(packet_data->payload),
           static_cast<uint32_t>(packet_data->time_ms * sample_rate_hz_ / 1000));
-      if (error != NetEq::kOK && error_callback_) {
-        error_callback_->OnInsertPacketError(neteq_->LastError(), *packet_data);
+      if (error != NetEq::kOK && callbacks_.error_callback) {

[AleBzk, 2017/05/02 10:36:55]

For sure, it's fine as it is now. Just wondering if it'd be better (mainly for
readability) to explicitly check if pointers != nullptr. I found that this is
done elsewhere in webrtc (see
https://cs.chromium.org/search/?q=%22!%3D+nullptr%22+package:%5Echromium$+web...).

If you decide to change, also adapt the other if(callbacks_.*) statements below.

[hlundin-webrtc, 2017/05/04 12:50:07]

I think we have a fair amount of the shorter form too. I'd rather stick to that.

+        callbacks_.error_callback->OnInsertPacketError(neteq_->LastError(),
+                                                       *packet_data);
+      }
+      if (callbacks_.post_insert_packet) {
+        callbacks_.post_insert_packet->AfterInsertPacket(*packet_data,
+                                                         neteq_.get());

[AleBzk, 2017/05/02 10:36:55]

I wonder if there was an error for which a packet has not been inserted (not
sure if this can happen). If so, do you want to call the post_insert_packet
callback?

[hlundin-webrtc, 2017/05/04 12:50:07]

There could be. But I think we can leave that up to the callback object to sort
it out.

       }
     }
 
     // Check if it is time to get output audio.
     if (input_->NextOutputEventTime() &&
         time_now_ms >= *input_->NextOutputEventTime()) {
+      if (callbacks_.get_audio_callback) {
+        callbacks_.get_audio_callback->BeforeGetAudio(neteq_.get());
+      }
       AudioFrame out_frame;
       bool muted;
       int error = neteq_->GetAudio(&out_frame, &muted);
       RTC_CHECK(!muted) << "The code does not handle enable_muted_state";
       if (error != NetEq::kOK) {
-        if (error_callback_) {
-          error_callback_->OnGetAudioError(neteq_->LastError());
+        if (callbacks_.error_callback) {
+          callbacks_.error_callback->OnGetAudioError(neteq_->LastError());
         }
       } else {
         sample_rate_hz_ = out_frame.sample_rate_hz_;
       }
+      if (callbacks_.get_audio_callback) {
+        callbacks_.get_audio_callback->AfterGetAudio(time_now_ms, out_frame,
+                                                     muted, neteq_.get());
+      }
 
       if (output_) {
         RTC_CHECK(output_->WriteArray(