Reland "NetEQ RTP Play: Optionally write output audio file"

This reverts commit c4b391a257.

Reason for revert: issue fixed

Original change's description:
> Revert "NetEQ RTP Play: Optionally write output audio file"
>
> This reverts commit 6330818ec8.
>
> Reason for revert: This breaks api/test/neteq_simulator_factory.cc, which unfortunately was not caught by our bots.
>
> Original change's description:
> > NetEQ RTP Play: Optionally write output audio file
> >
> > This CL makes the output audio file optional to more
> > quickly run neteq_rtpplay when no audio output is needed.
> > The CL also includes necessary adaptations because of pre-existing
> > dependencies (e.g., the output audio file name is used to create
> > the plotting script file names).
> >
> > The command line arguments are retro-compatible - i.e., same behavior
> > when specifying the output audio file and the new flag
> > --output_files_base_name is not used.
> >
> > This CL also includes a test script with which the retro-compatibility
> > has been verified.
> >
> > Bug: webrtc:10337
> > Change-Id: Ie3f301b3b2ed0682fb74426d9cf452396f2b112b
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/126224
> > Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
> > Reviewed-by: Ivo Creusen <ivoc@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#27067}
>
> TBR=henrik.lundin@webrtc.org,alessiob@webrtc.org,ivoc@webrtc.org
>
> Change-Id: I0c63a8ba9566ef567ee398f571f2a511916fa742
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: webrtc:10337
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/127293
> Reviewed-by: Ivo Creusen <ivoc@webrtc.org>
> Commit-Queue: Ivo Creusen <ivoc@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#27078}

TBR=henrik.lundin@webrtc.org,alessiob@webrtc.org,ivoc@webrtc.org

Change-Id: Ia7061f7c2d69db61638ad612e82cd429eb49d539
Bug: webrtc:10337
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/127540
Reviewed-by: Ivo Creusen <ivoc@webrtc.org>
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27106}

api/test/neteq_simulator_factory.cc

@@ -10,6 +10,8 @@
 
 #include "api/test/neteq_simulator_factory.h"
 
+#include <string>
+
 #include "absl/memory/memory.h"
 #include "modules/audio_coding/neteq/tools/neteq_test_factory.h"
 #include "rtc_base/checks.h"
@@ -43,10 +45,12 @@ std::unique_ptr<NetEqSimulator> NetEqSimulatorFactory::CreateSimulator(
   RTC_CHECK_EQ(argc, 3) << "Wrong number of input arguments. Expected 3, got "
                         << argc;
   // TODO(ivoc) Stop (ab)using command-line flags in this function.
+  const std::string output_audio_filename(argv[2]);
   NetEqTestFactory::Config config;
   config.replacement_audio_file = FLAG_replacement_audio_file;
   config.max_nr_packets_in_buffer = FLAG_max_nr_packets_in_buffer;
-  return factory_->InitializeTest(argv[1], argv[2], config);
+  config.output_audio_filename = output_audio_filename;
+  return factory_->InitializeTest(argv[1], config);
 }
 
 }  // namespace test

modules/audio_coding/BUILD.gn

@@ -1462,8 +1462,11 @@ if (rtc_include_tests) {
         ":neteq_test_factory",
         ":neteq_test_tools",
         "../../rtc_base:rtc_base_approved",
+        "../../rtc_base:stringutils",
         "../../system_wrappers:field_trial",
         "../../test:field_trial",
+        "//third_party/abseil-cpp/absl/strings",
+        "//third_party/abseil-cpp/absl/types:optional",
       ]
       sources = [
         "neteq/tools/neteq_rtpplay.cc",

modules/audio_coding/neteq/tools/README.md

@@ -0,0 +1,17 @@
+# NetEQ RTP Play tool
+
+## Testing of the command line arguments
+The command line tool `neteq_rtpplay` can be tested by running `neteq_rtpplay_test.sh`, which is not use on try bots, but it can be used before submitting any CLs that may break the behavior of the command line arguments of `neteq_rtpplay`.
+
+Run `neteq_rtpplay_test.sh` as follows from the `src/` folder:
+```
+src$ ./modules/audio_coding/neteq/tools/neteq_rtpplay_test.sh  \
+  out/Default/neteq_rtpplay  \
+  resources/audio_coding/neteq_opus.rtp  \
+  resources/short_mixed_mono_48.pcm
+```
+
+You can replace the RTP and PCM files with any other compatible files.
+If you get an error using the files indicated above, try running `gclient sync`.
+
+Requirements: `awk` and `md5sum`.

modules/audio_coding/neteq/tools/neteq_rtpplay.cc

@@ -11,9 +11,12 @@
 #include <iostream>
 #include <string>
 
+#include "absl/strings/string_view.h"
+#include "absl/types/optional.h"
 #include "modules/audio_coding/neteq/tools/neteq_test.h"
 #include "modules/audio_coding/neteq/tools/neteq_test_factory.h"
 #include "rtc_base/flags.h"
+#include "rtc_base/strings/string_builder.h"
 #include "system_wrappers/include/field_trial.h"
 #include "test/field_trial.h"
 
@@ -117,6 +120,10 @@ WEBRTC_DEFINE_int(video_content_type,
 WEBRTC_DEFINE_int(video_timing,
                   TestConfig::default_video_timing(),
                   "Extension ID for video timing");
+WEBRTC_DEFINE_string(output_files_base_name,
+                     "",
+                     "Custom path used as prefix for the output files - i.e., "
+                     "matlab plot, python plot, text log.");
 WEBRTC_DEFINE_bool(matlabplot,
                    false,
                    "Generates a matlab script for plotting the delay profile");
@@ -210,6 +217,54 @@ void PrintCodecMapping() {
   PrintCodecMappingEntry("comfort noise (48 kHz)", FLAG_cn_swb48);
 }
 
+bool ValidateOutputFilesOptions(bool textlog,
+                                bool plotting,
+                                absl::string_view output_files_base_name,
+                                absl::string_view output_audio_filename) {
+  bool output_files_base_name_specified = !output_files_base_name.empty();
+  if (!textlog && !plotting && output_files_base_name_specified) {
+    std::cout << "Error: --output_files_base_name cannot be used without at "
+              << "least one of the following flags: --textlog, --matlabplot, "
+              << "--pythonplot." << std::endl;
+    return false;
+  }
+  // Without |output_audio_filename|, |output_files_base_name| is required when
+  // one or more output files must be generated (in order to form a valid output
+  // file name).
+  if (output_audio_filename.empty() && (textlog || plotting) &&
+      !output_files_base_name_specified) {
+    std::cout << "Error: when no output audio file is specified and --textlog, "
+              << "--matlabplot and/or --pythonplot are used, "
+              << "--output_files_base_name must be also used." << std::endl;
+    return false;
+  }
+  return true;
+}
+
+absl::optional<std::string> CreateOptionalOutputFileName(
+    bool output_requested,
+    absl::string_view basename,
+    absl::string_view output_audio_filename,
+    absl::string_view suffix) {
+  if (!output_requested) {
+    return absl::nullopt;
+  }
+  if (!basename.empty()) {
+    // Override the automatic assignment.
+    rtc::StringBuilder sb(basename);
+    sb << suffix;
+    return sb.str();
+  }
+  if (!output_audio_filename.empty()) {
+    // Automatically assign name.
+    rtc::StringBuilder sb(output_audio_filename);
+    sb << suffix;
+    return sb.str();
+  }
+  std::cout << "Error: invalid text log file parameters.";
+  return absl::nullopt;
+}
+
 }  // namespace
 
 int main(int argc, char* argv[]) {
@@ -221,7 +276,7 @@ int main(int argc, char* argv[]) {
       program_name +
       " --help for usage.\n"
       "Example usage:\n" +
-      program_name + " input.rtp output.{pcm, wav}\n";
+      program_name + " input.rtp [output.{pcm, wav}]\n";
   if (rtc::FlagList::SetFlagsFromCommandLine(&argc, argv, true)) {
     exit(1);
   }
@@ -234,11 +289,16 @@ int main(int argc, char* argv[]) {
     PrintCodecMapping();
     exit(0);
   }
-  if (argc != 3) {
+  if (argc < 2 || argc > 3) {  // The output audio file is optional.
     // Print usage information.
     std::cout << usage;
     exit(0);
   }
+  const std::string output_audio_filename((argc == 3) ? argv[2] : "");
+  const std::string output_files_base_name(FLAG_output_files_base_name);
+  RTC_CHECK(ValidateOutputFilesOptions(
+      FLAG_textlog, FLAG_matlabplot || FLAG_pythonplot, output_files_base_name,
+      output_audio_filename));
   RTC_CHECK(ValidatePayloadType(FLAG_pcmu));
   RTC_CHECK(ValidatePayloadType(FLAG_pcma));
   RTC_CHECK(ValidatePayloadType(FLAG_ilbc));
@@ -297,10 +357,19 @@ int main(int argc, char* argv[]) {
   config.video_timing = FLAG_video_timing;
   config.matlabplot = FLAG_matlabplot;
   config.pythonplot = FLAG_pythonplot;
-  config.textlog = FLAG_textlog;
   config.concealment_events = FLAG_concealment_events;
   config.max_nr_packets_in_buffer = FLAG_max_nr_packets_in_buffer;
   config.enable_fast_accelerate = FLAG_enable_fast_accelerate;
+  if (!output_audio_filename.empty()) {
+    config.output_audio_filename = output_audio_filename;
+  }
+  config.textlog_filename =
+      CreateOptionalOutputFileName(FLAG_textlog, output_files_base_name,
+                                   output_audio_filename, ".text_log.txt");
+  config.plot_scripts_basename = CreateOptionalOutputFileName(
+      FLAG_matlabplot || FLAG_pythonplot, output_files_base_name,
+      output_audio_filename, "");
+
   // Check if an SSRC value was provided.
   if (strlen(FLAG_ssrc) > 0) {
     uint32_t ssrc;
@@ -308,8 +377,8 @@ int main(int argc, char* argv[]) {
     config.ssrc_filter = absl::make_optional(ssrc);
   }
 
-  std::unique_ptr<webrtc::test::NetEqTest> test =
-      factory.InitializeTest(argv[1], argv[2], config);
+  std::unique_ptr<webrtc::test::NetEqTest> test = factory.InitializeTest(
+      /*input_filename=*/argv[1], config);
   test->Run();
   return 0;
 }

modules/audio_coding/neteq/tools/neteq_rtpplay_test.sh

@@ -0,0 +1,183 @@
+#!/bin/bash
+#
+#  Copyright (c) 2019 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.
+#
+
+# Aliases.
+BIN=$1
+TEST_RTC_EVENT_LOG=$2
+INPUT_PCM_FILE=$3
+
+# Check setup.
+if [ ! -f $BIN ]; then
+  echo "Cannot find neteq_rtpplay binary."
+  exit 99
+fi
+if [ ! -f $TEST_RTC_EVENT_LOG ]; then
+  echo "Cannot find RTC event log file."
+  exit 99
+fi
+if [ ! -f $INPUT_PCM_FILE ]; then
+  echo "Cannot find PCM file."
+  exit 99
+fi
+
+# Defines.
+
+TMP_DIR=$(mktemp -d /tmp/tmp_XXXXXXXXXX)
+PASS=0
+FAIL=1
+TEST_SUITE_RESULT=$PASS
+
+file_hash () {
+  md5sum $1 | awk '{ print $1 }'
+}
+
+test_passed () {
+  echo PASS
+}
+
+test_failed () {
+  echo "FAIL: $1"
+  TEST_SUITE_RESULT=$FAIL
+}
+
+test_file_checksums_match () {
+  if [ ! -f $1 ] || [ ! -f $2 ]; then
+    test_failed "Cannot compare hash values: file(s) not found."
+    return
+  fi
+  HASH1=$(file_hash $1)
+  HASH2=$(file_hash $2)
+  if [ "$HASH1" = "$HASH2" ]; then
+    test_passed
+  else
+    test_failed "$1 differs from $2"
+  fi
+}
+
+test_file_exists () {
+  if [ -f $1 ]; then
+    test_passed
+  else
+    test_failed "$1 does not exist"
+  fi
+}
+
+test_exit_code_0 () {
+  if [ $1 -eq 0 ]; then
+    test_passed
+  else
+    test_failed "$1 did not return 0"
+  fi
+}
+
+test_exit_code_not_0 () {
+  if [ $1 -eq 0 ]; then
+    test_failed "$1 returned 0"
+  else
+    test_passed
+  fi
+}
+
+# Generate test data.
+
+# Case 1. Pre-existing way.
+CASE1_WAV=$TMP_DIR/case1.wav
+$BIN $TEST_RTC_EVENT_LOG $CASE1_WAV  \
+    --replacement_audio_file $INPUT_PCM_FILE  \
+    --textlog --pythonplot --matlabplot  \
+    > $TMP_DIR/case1.stdout 2> /dev/null
+CASE1_RETURN_CODE=$?
+CASE1_TEXTLOG=$TMP_DIR/case1.wav.text_log.txt
+CASE1_PYPLOT=$TMP_DIR/case1_wav.py
+CASE1_MATPLOT=$TMP_DIR/case1_wav.m
+
+# Case 2. No output files.
+$BIN $TEST_RTC_EVENT_LOG --replacement_audio_file $INPUT_PCM_FILE  \
+    > $TMP_DIR/case2.stdout 2> /dev/null
+CASE2_RETURN_CODE=$?
+
+# Case 3. No output audio file.
+
+# Case 3.1 Without --output_files_base_name (won't run).
+$BIN $TEST_RTC_EVENT_LOG  \
+    --replacement_audio_file $INPUT_PCM_FILE  \
+    --textlog --pythonplot --matlabplot  \
+    &> /dev/null
+CASE3_1_RETURN_CODE=$?
+
+# Case 3.2 With --output_files_base_name (runs).
+$BIN $TEST_RTC_EVENT_LOG  \
+    --replacement_audio_file $INPUT_PCM_FILE  \
+    --output_files_base_name $TMP_DIR/case3_2  \
+    --textlog --pythonplot --matlabplot  \
+    > $TMP_DIR/case3_2.stdout 2> /dev/null
+CASE3_2_RETURN_CODE=$?
+CASE3_2_TEXTLOG=$TMP_DIR/case3_2.text_log.txt
+CASE3_2_PYPLOT=$TMP_DIR/case3_2.py
+CASE3_2_MATPLOT=$TMP_DIR/case3_2.m
+
+# Case 4. With output audio file and --output_files_base_name.
+CASE4_WAV=$TMP_DIR/case4.wav
+$BIN $TEST_RTC_EVENT_LOG $TMP_DIR/case4.wav \
+    --replacement_audio_file $INPUT_PCM_FILE  \
+    --output_files_base_name $TMP_DIR/case4  \
+    --textlog --pythonplot --matlabplot  \
+    > $TMP_DIR/case4.stdout 2> /dev/null
+CASE4_RETURN_CODE=$?
+CASE4_TEXTLOG=$TMP_DIR/case4.text_log.txt
+CASE4_PYPLOT=$TMP_DIR/case4.py
+CASE4_MATPLOT=$TMP_DIR/case4.m
+
+# Tests.
+
+echo Check exit codes
+test_exit_code_0 $CASE1_RETURN_CODE
+test_exit_code_0 $CASE2_RETURN_CODE
+test_exit_code_not_0 $CASE3_1_RETURN_CODE
+test_exit_code_0 $CASE3_2_RETURN_CODE
+test_exit_code_0 $CASE4_RETURN_CODE
+
+echo Check that the expected output files exist
+test_file_exists $CASE1_TEXTLOG
+test_file_exists $CASE3_2_TEXTLOG
+test_file_exists $CASE4_TEXTLOG
+test_file_exists $CASE1_PYPLOT
+test_file_exists $CASE3_2_PYPLOT
+test_file_exists $CASE4_PYPLOT
+test_file_exists $CASE1_MATPLOT
+test_file_exists $CASE3_2_MATPLOT
+test_file_exists $CASE4_MATPLOT
+
+echo Check that the same WAV file is produced
+test_file_checksums_match $CASE1_WAV $CASE4_WAV
+
+echo Check that the same text log is produced
+test_file_checksums_match $CASE1_TEXTLOG $CASE3_2_TEXTLOG
+test_file_checksums_match $CASE1_TEXTLOG $CASE4_TEXTLOG
+
+echo Check that the same python plot scripts is produced
+test_file_checksums_match $CASE1_PYPLOT $CASE3_2_PYPLOT
+test_file_checksums_match $CASE1_PYPLOT $CASE4_PYPLOT
+
+echo Check that the same matlab plot scripts is produced
+test_file_checksums_match $CASE1_MATPLOT $CASE3_2_MATPLOT
+test_file_checksums_match $CASE1_MATPLOT $CASE4_MATPLOT
+
+# Clean up
+rm -fr $TMP_DIR
+
+if [ $TEST_SUITE_RESULT -eq $PASS ]; then
+  echo All tests passed.
+  exit 0
+else
+  echo One or more tests failed.
+  exit 1
+fi

modules/audio_coding/neteq/tools/neteq_test_factory.cc

@@ -24,6 +24,7 @@
 #include "absl/memory/memory.h"
 #include "api/audio_codecs/builtin_audio_decoder_factory.h"
 #include "modules/audio_coding/neteq/include/neteq.h"
+#include "modules/audio_coding/neteq/tools/audio_sink.h"
 #include "modules/audio_coding/neteq/tools/fake_decode_from_file.h"
 #include "modules/audio_coding/neteq/tools/input_audio_file.h"
 #include "modules/audio_coding/neteq/tools/neteq_delay_analyzer.h"
@@ -109,7 +110,6 @@ NetEqTestFactory::Config::~Config() = default;
 
 std::unique_ptr<NetEqTest> NetEqTestFactory::InitializeTest(
     std::string input_file_name,
-    std::string output_file_name,
     const Config& config) {
   // Gather RTP header extensions in a map.
   NetEqPacketSourceInput::RtpHeaderExtensionMap rtp_ext_map = {
@@ -166,20 +166,26 @@ std::unique_ptr<NetEqTest> NetEqTestFactory::InitializeTest(
     RTC_NOTREACHED();
   }
 
-  // Open the output file now that we know the sample rate. (Rate is only needed
-  // for wav files.)
+  // If an output file is requested, open it.
   std::unique_ptr<AudioSink> output;
-  if (output_file_name.size() >= 4 &&
-      output_file_name.substr(output_file_name.size() - 4) == ".wav") {
-    // Open a wav file.
-    output.reset(new OutputWavFile(output_file_name, *sample_rate_hz));
+  if (!config.output_audio_filename.has_value()) {
+    output = absl::make_unique<VoidAudioSink>();
+    std::cout << "No output audio file" << std::endl;
+  } else if (config.output_audio_filename->size() >= 4 &&
+             config.output_audio_filename->substr(
+                 config.output_audio_filename->size() - 4) == ".wav") {
+    // Open a wav file with the known sample rate.
+    output = absl::make_unique<OutputWavFile>(*config.output_audio_filename,
+                                              *sample_rate_hz);
+    std::cout << "Output WAV file: " << *config.output_audio_filename
+              << std::endl;
   } else {
     // Open a pcm file.
-    output.reset(new OutputAudioFile(output_file_name));
+    output = absl::make_unique<OutputAudioFile>(*config.output_audio_filename);
+    std::cout << "Output PCM file: " << *config.output_audio_filename
+              << std::endl;
   }
 
-  std::cout << "Output file: " << output_file_name << std::endl;
-
   NetEqTest::DecoderMap codecs = NetEqTest::StandardDecoderMap();
 
   rtc::scoped_refptr<AudioDecoderFactory> decoder_factory =
@@ -234,16 +240,16 @@ std::unique_ptr<NetEqTest> NetEqTestFactory::InitializeTest(
 
   // Create a text log file if needed.
   std::unique_ptr<std::ofstream> text_log;
-  if (config.textlog) {
-    text_log =
-        absl::make_unique<std::ofstream>(output_file_name + ".text_log.txt");
+  if (config.textlog_filename.has_value()) {
+    text_log = absl::make_unique<std::ofstream>(*config.textlog_filename);
   }
 
   NetEqTest::Callbacks callbacks;
-  stats_plotter_.reset(
-      new NetEqStatsPlotter(config.matlabplot, config.pythonplot,
-                            config.concealment_events, output_file_name));
+  stats_plotter_ = absl::make_unique<NetEqStatsPlotter>(
+      config.matlabplot, config.pythonplot, config.concealment_events,
+      config.plot_scripts_basename.value_or(""));
 
+  RTC_CHECK(stats_plotter_);
   ssrc_switch_detector_.reset(
       new SsrcSwitchDetector(stats_plotter_->stats_getter()->delay_analyzer()));
   callbacks.post_insert_packet = ssrc_switch_detector_.get();

modules/audio_coding/neteq/tools/neteq_test_factory.h

@@ -117,8 +117,6 @@ class NetEqTestFactory {
     bool matlabplot = false;
     // Generates a python script for plotting the delay profile.
     bool pythonplot = false;
-    // Generates a text log describing the simulation on a step-by-step basis.
-    bool textlog = false;
     // Prints concealment events.
     bool concealment_events = false;
     // Maximum allowed number of packets in the buffer.
@@ -126,10 +124,16 @@ class NetEqTestFactory {
     int max_nr_packets_in_buffer = default_max_nr_packets_in_buffer();
     // Enables jitter buffer fast accelerate.
     bool enable_fast_accelerate = false;
+    // Path to the output text log file that describes the simulation on a
+    // step-by-step basis.
+    absl::optional<std::string> textlog_filename;
+    // Base name for the output script files for plotting the delay profile.
+    absl::optional<std::string> plot_scripts_basename;
+    // Path to the output audio file.
+    absl::optional<std::string> output_audio_filename;
   };
 
   std::unique_ptr<NetEqTest> InitializeTest(std::string input_filename,
-                                            std::string output_filename,
                                             const Config& config);
 
  private: