NetEqTest: Extend the callback structure

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

 /*
  *  Copyright (c) 2016 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 23 matching lines @@
 void DefaultNetEqTestErrorCallback::OnGetAudioError(int error_code) {
   std::cerr << "GetAudio returned error code " << error_code << std::endl;
   FATAL();
 }
 
 NetEqTest::NetEqTest(const NetEq::Config& config,
                      const DecoderMap& codecs,
                      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";
   RegisterDecoders(codecs);
   RegisterExternalDecoders(ext_codecs);
 }
 
 int64_t NetEqTest::Run() {
   const int64_t start_time_ms = *input_->NextEventTime();
   int64_t time_now_ms = start_time_ms;
 
   while (!input_->ended()) {
     // Advance time to next event.
     RTC_DCHECK(input_->NextEventTime());
     time_now_ms = *input_->NextEventTime();
     // Check if it is time to insert packet.
     if (input_->NextPacketTime() && time_now_ms >= *input_->NextPacketTime()) {
       std::unique_ptr<NetEqInput::PacketData> packet_data = input_->PopPacket();
       RTC_CHECK(packet_data);
       int error = neteq_->InsertPacket(
           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(
             out_frame.data_,
             out_frame.samples_per_channel_ * out_frame.num_channels_));
       }
 
       input_->AdvanceOutputEvent();
     }
   }
@@ skipping 22 matching lines @@
         neteq_->RegisterExternalDecoder(c.second.decoder, c.second.codec,
                                         c.second.codec_name, c.first),
         NetEq::kOK)
         << "Cannot register " << c.second.codec_name << " to payload type "
         << c.first;
   }
 }
 
 }  // namespace test
 }  // namespace webrtc