diff --git a/common_audio/wav_file.cc b/common_audio/wav_file.cc index 37f249e7fa..1217e40ba7 100644 --- a/common_audio/wav_file.cc +++ b/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) - : 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."; + // 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) { + // 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 --git a/common_audio/wav_file.h b/common_audio/wav_file.h index f7afe9239c..befe73339a 100644 --- a/common_audio/wav_file.h +++ b/common_audio/wav_file.h @@ -18,6 +18,7 @@ #include #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 --git a/common_audio/wav_file_unittest.cc b/common_audio/wav_file_unittest.cc index 7113b47c75..248273ba0b 100644 --- a/common_audio/wav_file_unittest.cc +++ b/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 --git a/modules/audio_processing/aec/echo_cancellation.cc b/modules/audio_processing/aec/echo_cancellation.cc index 864db5308e..163306883b 100644 --- a/modules/audio_processing/aec/echo_cancellation.cc +++ b/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 --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn index fbec98b485..348be5ce8d 100644 --- a/rtc_base/BUILD.gn +++ b/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 --git a/rtc_base/platform_file.cc b/rtc_base/platform_file.cc index 35a2622bfd..a4c906a1ed 100644 --- a/rtc_base/platform_file.cc +++ b/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(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 --git a/rtc_base/platform_file.h b/rtc_base/platform_file.h index 8e911be6d4..52fbaff115 100644 --- a/rtc_base/platform_file.h +++ b/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 --git a/rtc_base/platform_file_unittest.cc b/rtc_base/platform_file_unittest.cc new file mode 100644 index 0000000000..4e72400da2 --- /dev/null +++ b/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 --git a/test/testsupport/fileutils.cc b/test/testsupport/fileutils.cc index 2aff75a4d9..309c885f57 100644 --- a/test/testsupport/fileutils.cc +++ b/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> ReadDirectory(std::string path) { if (path.length() == 0) return rtc::Optional>(); diff --git a/test/testsupport/fileutils.h b/test/testsupport/fileutils.h index f0560cf979..8bf19192e7 100644 --- a/test/testsupport/fileutils.h +++ b/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. diff --git a/test/testsupport/fileutils_unittest.cc b/test/testsupport/fileutils_unittest.cc index 79988c0dd3..4c0be26dbe 100644 --- a/test/testsupport/fileutils_unittest.cc +++ b/test/testsupport/fileutils_unittest.cc @@ -127,6 +127,19 @@ TEST_F(FileUtilsTest, TempFilename) { remove(temp_filename.c_str()); } +TEST_F(FileUtilsTest, GenerateTempFilename) { + std::string temp_filename = webrtc::test::GenerateTempFilename( + webrtc::test::OutputPath(), "TempFilenameTest"); + ASSERT_FALSE(webrtc::test::FileExists(temp_filename)) + << "File exists: " << temp_filename; + FILE* file = fopen(temp_filename.c_str(), "wb"); + ASSERT_TRUE(file != NULL) << "Failed to open file: " << temp_filename; + ASSERT_GT(fprintf(file, "%s", "Dummy data"), 0) + << "Failed to write to file: " << temp_filename; + fclose(file); + remove(temp_filename.c_str()); +} + // Only tests that the code executes #if defined(WEBRTC_IOS) #define MAYBE_CreateDir DISABLED_CreateDir