Extend WavReader and WavWriter API.

Add ability to read and write wav files using rtc::PlatformFile instead
of file name.

Bug: webrtc:8946
Change-Id: If18d9465f2155a33547f800edbdac45971a0e878
Reviewed-on: https://webrtc-review.googlesource.com/61424
Commit-Queue: Artem Titov <[email protected]>
Reviewed-by: Karl Wiberg <[email protected]>
Cr-Commit-Position: refs/heads/master@{#22497}

Author: Artem Titov

Diff for: common_audio/wav_file.cc

@@ -18,6 +18,7 @@
 #include "common_audio/include/audio_util.h"
 #include "common_audio/wav_header.h"
 #include "rtc_base/checks.h"
+#include "rtc_base/logging.h"
 #include "rtc_base/numerics/safe_conversions.h"
 
 namespace webrtc {
@@ -47,8 +48,21 @@ std::string WavFile::FormatAsString() const {
 }
 
 WavReader::WavReader(const std::string& filename)
-    : file_handle_(fopen(filename.c_str(), "rb")) {
-  RTC_CHECK(file_handle_) << "Could not open wav file for reading.";
+    : WavReader(rtc::OpenPlatformFileReadOnly(filename)) {}
+
+WavReader::WavReader(rtc::PlatformFile file) {
+  RTC_CHECK_NE(file, rtc::kInvalidPlatformFileValue)
+      << "Invalid file. Could not create file handle for wav file.";
+  file_handle_ = rtc::FdopenPlatformFile(file, "rb");
+  if (!file_handle_) {
+    RTC_LOG(LS_ERROR) << "Could not open wav file for reading: " << errno;
+    // Even though we failed to open a FILE*, the file is still open
+    // and needs to be closed.
+    if (!rtc::ClosePlatformFile(file)) {
+      RTC_LOG(LS_ERROR) << "Can't close file.";
+    }
+    FATAL() << "Could not open wav file for reading.";
+  }
 
   ReadableWavFile readable(file_handle_);
   WavFormat format;
@@ -110,13 +124,31 @@ void WavReader::Close() {
   file_handle_ = nullptr;
 }
 
-WavWriter::WavWriter(const std::string& filename, int sample_rate,
+WavWriter::WavWriter(const std::string& filename,
+                     int sample_rate,
+                     size_t num_channels)
+    // Unlike plain fopen, CreatePlatformFile takes care of filename utf8 ->
+    // wchar conversion on windows.
+    : WavWriter(rtc::CreatePlatformFile(filename), sample_rate, num_channels) {}
+
+WavWriter::WavWriter(rtc::PlatformFile file,
+                     int sample_rate,
                      size_t num_channels)
-    : sample_rate_(sample_rate),
-      num_channels_(num_channels),
-      num_samples_(0),
-      file_handle_(fopen(filename.c_str(), "wb")) {
-  RTC_CHECK(file_handle_) << "Could not open wav file for writing.";
+    : sample_rate_(sample_rate), num_channels_(num_channels), num_samples_(0) {
+  // Handle errors from the CreatePlatformFile call in above constructor.
+  RTC_CHECK_NE(file, rtc::kInvalidPlatformFileValue)
+      << "Invalid file. Could not create wav file.";
+  file_handle_ = rtc::FdopenPlatformFile(file, "wb");
+  if (!file_handle_) {
+    RTC_LOG(LS_ERROR) << "Could not open wav file for writing.";
+    // Even though we failed to open a FILE*, the file is still open
+    // and needs to be closed.
+    if (!rtc::ClosePlatformFile(file)) {
+      RTC_LOG(LS_ERROR) << "Can't close file.";
+    }
+    FATAL() << "Could not open wav file for writing.";
+  }
+
   RTC_CHECK(CheckWavParameters(num_channels_, sample_rate_, kWavFormat,
                                kBytesPerSample, num_samples_));
 

Diff for: common_audio/wav_file.h

@@ -18,6 +18,7 @@
 #include <string>
 
 #include "rtc_base/constructormagic.h"
+#include "rtc_base/platform_file.h"
 
 namespace webrtc {
 
@@ -41,6 +42,9 @@ class WavWriter final : public WavFile {
   // Open a new WAV file for writing.
   WavWriter(const std::string& filename, int sample_rate, size_t num_channels);
 
+  // Open a new WAV file for writing.
+  WavWriter(rtc::PlatformFile file, int sample_rate, size_t num_channels);
+
   // Close the WAV file, after writing its header.
   ~WavWriter() override;
 
@@ -70,6 +74,9 @@ class WavReader final : public WavFile {
   // Opens an existing WAV file for reading.
   explicit WavReader(const std::string& filename);
 
+  // Opens an existing WAV file for reading.
+  explicit WavReader(rtc::PlatformFile file);
+
   // Close the WAV file.
   ~WavReader() override;
 

Diff for: common_audio/wav_file_unittest.cc

@@ -174,4 +174,72 @@ TEST(WavWriterTest, LargeFile) {
   }
 }
 
+// Write a tiny WAV file with the the std::FILE interface and verify the
+// result.
+TEST(WavWriterTest, CPPFileDescriptor) {
+  const std::string outfile = test::OutputPath() + "wavtest1.wav";
+  static constexpr size_t kNumSamples = 3;
+  {
+    WavWriter w(rtc::CreatePlatformFile(outfile), 14099, 1);
+    EXPECT_EQ(14099, w.sample_rate());
+    EXPECT_EQ(1u, w.num_channels());
+    EXPECT_EQ(0u, w.num_samples());
+    w.WriteSamples(kSamples, kNumSamples);
+    EXPECT_EQ(kNumSamples, w.num_samples());
+  }
+  // Write some extra "metadata" to the file that should be silently ignored
+  // by WavReader. We don't use WavWriter directly for this because it doesn't
+  // support metadata.
+  static constexpr uint8_t kMetadata[] = {101, 202};
+  {
+    FILE* f = fopen(outfile.c_str(), "ab");
+    ASSERT_TRUE(f);
+    ASSERT_EQ(1u, fwrite(kMetadata, sizeof(kMetadata), 1, f));
+    fclose(f);
+  }
+  static const uint8_t kExpectedContents[] = {
+      // clang-format off
+    'R', 'I', 'F', 'F',
+    42, 0, 0, 0,       // size of whole file - 8: 6 + 44 - 8
+    'W', 'A', 'V', 'E',
+    'f', 'm', 't', ' ',
+    16, 0, 0, 0,       // size of fmt block - 8: 24 - 8
+    1, 0,              // format: PCM (1)
+    1, 0,              // channels: 1
+    0x13, 0x37, 0, 0,  // sample rate: 14099
+    0x26, 0x6e, 0, 0,  // byte rate: 2 * 14099
+    2, 0,              // block align: NumChannels * BytesPerSample
+    16, 0,             // bits per sample: 2 * 8
+    'd', 'a', 't', 'a',
+    6, 0, 0, 0,        // size of payload: 6
+    0, 0,              // first sample: 0.0
+    10, 0,             // second sample: 10.0
+    0xff, 0x7f,        // third sample: 4e4 (saturated)
+    kMetadata[0], kMetadata[1],
+      // clang-format on
+  };
+  static constexpr size_t kContentSize =
+      kWavHeaderSize + kNumSamples * sizeof(int16_t) + sizeof(kMetadata);
+  static_assert(sizeof(kExpectedContents) == kContentSize, "");
+  EXPECT_EQ(kContentSize, test::GetFileSize(outfile));
+  FILE* f = fopen(outfile.c_str(), "rb");
+  ASSERT_TRUE(f);
+  uint8_t contents[kContentSize];
+  ASSERT_EQ(1u, fread(contents, kContentSize, 1, f));
+  EXPECT_EQ(0, fclose(f));
+  EXPECT_EQ(0, memcmp(kExpectedContents, contents, kContentSize));
+
+  {
+    WavReader r(rtc::OpenPlatformFileReadOnly(outfile));
+    EXPECT_EQ(14099, r.sample_rate());
+    EXPECT_EQ(1u, r.num_channels());
+    EXPECT_EQ(kNumSamples, r.num_samples());
+    static constexpr float kTruncatedSamples[] = {0.0, 10.0, 32767.0};
+    float samples[kNumSamples];
+    EXPECT_EQ(kNumSamples, r.ReadSamples(kNumSamples, samples));
+    EXPECT_EQ(0, memcmp(kTruncatedSamples, samples, sizeof(samples)));
+    EXPECT_EQ(0u, r.ReadSamples(kNumSamples, samples));
+  }
+}
+
 }  // namespace webrtc

Diff for: modules/audio_processing/aec/echo_cancellation.cc

@@ -677,7 +677,7 @@ static int ProcessNormal(Aec* aecInst,
 }
 
 static void ProcessExtended(Aec* self,
-                            const float* const* near,
+                            const float* const* nearend,
                             size_t num_bands,
                             float* const* out,
                             size_t num_samples,
@@ -709,8 +709,8 @@ static void ProcessExtended(Aec* self,
   if (!self->farend_started) {
     for (i = 0; i < num_bands; ++i) {
       // Only needed if they don't already point to the same place.
-      if (near[i] != out[i]) {
-        memcpy(out[i], near[i], sizeof(near[i][0]) * num_samples);
+      if (nearend[i] != out[i]) {
+        memcpy(out[i], nearend[i], sizeof(nearend[i][0]) * num_samples);
       }
     }
     return;
@@ -746,7 +746,7 @@ static void ProcessExtended(Aec* self,
     const int adjusted_known_delay =
         WEBRTC_SPL_MAX(0, self->knownDelay + delay_diff_offset);
 
-    WebRtcAec_ProcessFrames(self->aec, near, num_bands, num_samples,
+    WebRtcAec_ProcessFrames(self->aec, nearend, num_bands, num_samples,
                             adjusted_known_delay, out);
   }
 }

Diff for: rtc_base/BUILD.gn

@@ -1194,6 +1194,7 @@ if (rtc_include_tests) {
       "numerics/safe_minmax_unittest.cc",
       "onetimeevent_unittest.cc",
       "pathutils_unittest.cc",
+      "platform_file_unittest.cc",
       "platform_thread_unittest.cc",
       "random_unittest.cc",
       "rate_limiter_unittest.cc",

Diff for: rtc_base/platform_file.cc

@@ -23,17 +23,21 @@
 
 namespace rtc {
 
+FILE* FdopenPlatformFileForWriting(PlatformFile file) {
+  return FdopenPlatformFile(file, "w");
+}
+
 #if defined(WEBRTC_WIN)
 const PlatformFile kInvalidPlatformFileValue = INVALID_HANDLE_VALUE;
 
-FILE* FdopenPlatformFileForWriting(PlatformFile file) {
+FILE* FdopenPlatformFile(PlatformFile file, const char* modes) {
   if (file == kInvalidPlatformFileValue)
     return nullptr;
   int fd = _open_osfhandle(reinterpret_cast<intptr_t>(file), 0);
   if (fd < 0)
     return nullptr;
 
-  return _fdopen(fd, "w");
+  return _fdopen(fd, modes);
 }
 
 bool ClosePlatformFile(PlatformFile file) {
@@ -49,6 +53,11 @@ PlatformFile OpenPlatformFile(const std::string& path) {
                       nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr);
 }
 
+PlatformFile OpenPlatformFileReadOnly(const std::string& path) {
+  return ::CreateFile(ToUtf16(path).c_str(), GENERIC_READ, FILE_SHARE_READ,
+                      nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr);
+}
+
 PlatformFile CreatePlatformFile(const std::string& path) {
   return ::CreateFile(ToUtf16(path).c_str(), GENERIC_READ | GENERIC_WRITE, 0,
                       nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr);
@@ -58,8 +67,8 @@ PlatformFile CreatePlatformFile(const std::string& path) {
 
 const PlatformFile kInvalidPlatformFileValue = -1;
 
-FILE* FdopenPlatformFileForWriting(PlatformFile file) {
-  return fdopen(file, "w");
+FILE* FdopenPlatformFile(PlatformFile file, const char* modes) {
+  return fdopen(file, modes);
 }
 
 bool ClosePlatformFile(PlatformFile file) {
@@ -74,6 +83,10 @@ PlatformFile OpenPlatformFile(const std::string& path) {
   return ::open(path.c_str(), O_RDWR);
 }
 
+PlatformFile OpenPlatformFileReadOnly(const std::string& path) {
+  return ::open(path.c_str(), O_RDONLY);
+}
+
 PlatformFile CreatePlatformFile(const std::string& path) {
   return ::open(path.c_str(), O_CREAT | O_TRUNC | O_RDWR, S_IRUSR | S_IWUSR);
 }

Diff for: rtc_base/platform_file.h

@@ -35,6 +35,11 @@ extern const PlatformFile kInvalidPlatformFileValue;
 // the PlatformFile should no longer be used.
 FILE* FdopenPlatformFileForWriting(PlatformFile file);
 
+// Associates a standard FILE stream with an existing PlatformFile.
+// Note that after this function has returned a valid FILE stream,
+// the PlatformFile should no longer be used.
+FILE* FdopenPlatformFile(PlatformFile file, const char* modes);
+
 // Closes a PlatformFile. Returns true on success, false on failure.
 // Don't use ClosePlatformFile to close a file opened with FdopenPlatformFile.
 // Use fclose instead.
@@ -47,6 +52,10 @@ bool RemoveFile(const std::string& path);
 // instead.
 PlatformFile OpenPlatformFile(const std::string& path);
 
+// Opens a file for reading only. You might want to use base/file.h
+// instead.
+PlatformFile OpenPlatformFileReadOnly(const std::string& path);
+
 // Creates a new file for reading and writing. If the file already exists it
 // will be overwritten. You might want to use base/file.h instead.
 PlatformFile CreatePlatformFile(const std::string& path);

Diff for: rtc_base/platform_file_unittest.cc

@@ -0,0 +1,77 @@
+/*
+ *  Copyright (c) 2018 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.
+ */
+
+#include "rtc_base/platform_file.h"
+#include "test/gtest.h"
+#include "test/testsupport/fileutils.h"
+
+namespace rtc {
+
+void FillWithDummyDataAndClose(FILE* const file, const std::string& filename) {
+  EXPECT_GT(fprintf(file, "%s", "Dummy data"), 0)
+      << "Failed to write to file: " << filename;
+  fclose(file);
+}
+
+TEST(PlatformFileTest, CreateWriteAndDelete) {
+  const std::string filename = webrtc::test::GenerateTempFilename(
+      webrtc::test::OutputPath(), ".testfile");
+  const PlatformFile fd = rtc::CreatePlatformFile(filename);
+  ASSERT_NE(fd, rtc::kInvalidPlatformFileValue)
+      << "Failed to create file descriptor for file: " << filename;
+  FILE* const file = rtc::FdopenPlatformFile(fd, "w");
+  ASSERT_TRUE(file != nullptr) << "Failed to open file: " << filename;
+  FillWithDummyDataAndClose(file, filename);
+  webrtc::test::RemoveFile(filename);
+}
+
+TEST(PlatformFileTest, OpenExistingWriteAndDelete) {
+  const std::string filename = webrtc::test::GenerateTempFilename(
+      webrtc::test::OutputPath(), ".testfile");
+
+  // Create file with dummy data.
+  FILE* file = fopen(filename.c_str(), "wb");
+  ASSERT_TRUE(file != nullptr) << "Failed to open file: " << filename;
+  FillWithDummyDataAndClose(file, filename);
+
+  // Open it for write, write and delete.
+  const PlatformFile fd = rtc::OpenPlatformFile(filename);
+  ASSERT_NE(fd, rtc::kInvalidPlatformFileValue)
+      << "Failed to open file descriptor for file: " << filename;
+  file = rtc::FdopenPlatformFile(fd, "w");
+  ASSERT_TRUE(file != nullptr) << "Failed to open file: " << filename;
+  FillWithDummyDataAndClose(file, filename);
+  webrtc::test::RemoveFile(filename);
+}
+
+TEST(PlatformFileTest, OpenExistingReadOnlyAndDelete) {
+  const std::string filename = webrtc::test::GenerateTempFilename(
+      webrtc::test::OutputPath(), ".testfile");
+
+  // Create file with dummy data.
+  FILE* file = fopen(filename.c_str(), "wb");
+  ASSERT_TRUE(file != nullptr) << "Failed to open file: " << filename;
+  FillWithDummyDataAndClose(file, filename);
+
+  // Open it for read, read and delete.
+  const PlatformFile fd = rtc::OpenPlatformFileReadOnly(filename);
+  ASSERT_NE(fd, rtc::kInvalidPlatformFileValue)
+      << "Failed to open file descriptor for file: " << filename;
+  file = rtc::FdopenPlatformFile(fd, "r");
+  ASSERT_TRUE(file != nullptr) << "Failed to open file: " << filename;
+
+  int buf[]{0};
+  ASSERT_GT(fread(&buf, 1, 1, file), 0u)
+      << "Failed to read from file: " << filename;
+  fclose(file);
+  webrtc::test::RemoveFile(filename);
+}
+
+}  // namespace rtc

Diff for: test/testsupport/fileutils.cc

@@ -215,6 +215,13 @@ std::string TempFilename(const std::string &dir, const std::string &prefix) {
 #endif
 }
 
+std::string GenerateTempFilename(const std::string& dir,
+                                 const std::string& prefix) {
+  std::string filename = TempFilename(dir, prefix);
+  RemoveFile(filename);
+  return filename;
+}
+
 rtc::Optional<std::vector<std::string>> ReadDirectory(std::string path) {
   if (path.length() == 0)
     return rtc::Optional<std::vector<std::string>>();

Diff for: test/testsupport/fileutils.h

@@ -40,8 +40,14 @@ std::string OutputPath();
 
 // Generates an empty file with a unique name in the specified directory and
 // returns the file name and path.
+// TODO(titovartem) rename to TempFile and next method to TempFilename
 std::string TempFilename(const std::string &dir, const std::string &prefix);
 
+// Generates a unique file name that can be used for file creation. Doesn't
+// create any files.
+std::string GenerateTempFilename(const std::string& dir,
+                                 const std::string& prefix);
+
 // Returns a path to a resource file for the currently executing platform.
 // Adapts to what filenames are currently present in the
 // [project-root]/resources/ dir.