diff --git a/common_audio/wav_file.cc b/common_audio/wav_file.cc index 3f2aa7e50f..30968c25c9 100644 --- a/common_audio/wav_file.cc +++ b/common_audio/wav_file.cc @@ -34,40 +34,38 @@ constexpr size_t kBytesPerSample = 2; // Doesn't take ownership of the file handle and won't close it. class ReadableWavFile : public ReadableWav { public: - explicit ReadableWavFile(FILE* file) : file_(file) {} + explicit ReadableWavFile(FileWrapper* file) : file_(file) {} ReadableWavFile(const ReadableWavFile&) = delete; ReadableWavFile& operator=(const ReadableWavFile&) = delete; size_t Read(void* buf, size_t num_bytes) override { - return fread(buf, 1, num_bytes, file_); + size_t count = file_->Read(buf, num_bytes); + pos_ += count; + return count; } bool SeekForward(uint32_t num_bytes) override { - return fseek(file_, num_bytes, SEEK_CUR) == 0; + bool success = file_->SeekRelative(num_bytes); + if (success) { + pos_ += num_bytes; + } + return success; } + int64_t GetPosition() { return pos_; } private: - FILE* file_; + FileWrapper* file_; + int64_t pos_ = 0; }; } // namespace WavReader::WavReader(const std::string& filename) - : WavReader(rtc::OpenPlatformFileReadOnly(filename)) {} + : WavReader(FileWrapper::OpenReadOnly(filename)) {} -WavReader::WavReader(rtc::PlatformFile file) { - RTC_CHECK_NE(file, rtc::kInvalidPlatformFileValue) +WavReader::WavReader(FileWrapper file) : file_(std::move(file)) { + RTC_CHECK(file_.is_open()) << "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_); + ReadableWavFile readable(&file_); WavFormat format; size_t bytes_per_sample; RTC_CHECK(ReadWavHeader(&readable, &num_channels_, &sample_rate_, &format, @@ -75,8 +73,7 @@ WavReader::WavReader(rtc::PlatformFile file) { num_samples_remaining_ = num_samples_; RTC_CHECK_EQ(kWavFormat, format); RTC_CHECK_EQ(kBytesPerSample, bytes_per_sample); - RTC_CHECK_EQ(0, fgetpos(file_handle_, &data_start_pos_)) - << "Failed to get WAV data position from file"; + data_start_pos_ = readable.GetPosition(); } WavReader::~WavReader() { @@ -84,7 +81,7 @@ WavReader::~WavReader() { } void WavReader::Reset() { - RTC_CHECK_EQ(0, fsetpos(file_handle_, &data_start_pos_)) + RTC_CHECK(file_.SeekTo(data_start_pos_)) << "Failed to set position in the file to WAV data start position"; num_samples_remaining_ = num_samples_; } @@ -107,13 +104,16 @@ size_t WavReader::ReadSamples(size_t num_samples, int16_t* samples) { #endif // There could be metadata after the audio; ensure we don't read it. num_samples = std::min(num_samples, num_samples_remaining_); - const size_t read = - fread(samples, sizeof(*samples), num_samples, file_handle_); + const size_t num_bytes = num_samples * sizeof(*samples); + const size_t read_bytes = file_.Read(samples, num_bytes); // If we didn't read what was requested, ensure we've reached the EOF. - RTC_CHECK(read == num_samples || feof(file_handle_)); - RTC_CHECK_LE(read, num_samples_remaining_); - num_samples_remaining_ -= read; - return read; + RTC_CHECK(read_bytes == num_bytes || file_.ReadEof()); + RTC_CHECK_EQ(read_bytes % 2, 0) + << "End of file in the middle of a 16-bit sample"; + const size_t read_samples = read_bytes / 2; + RTC_CHECK_LE(read_samples, num_samples_remaining_); + num_samples_remaining_ -= read_samples; + return read_samples; } size_t WavReader::ReadSamples(size_t num_samples, float* samples) { @@ -131,14 +131,13 @@ size_t WavReader::ReadSamples(size_t num_samples, float* samples) { } void WavReader::Close() { - RTC_CHECK_EQ(0, fclose(file_handle_)); - file_handle_ = nullptr; + file_.Close(); } WavWriter::WavWriter(const std::string& filename, int sample_rate, size_t num_channels) - // Unlike plain fopen, CreatePlatformFile takes care of filename utf8 -> + // Unlike plain fopen, OpenWriteOnly takes care of filename utf8 -> // wchar conversion on windows. : WavWriter(FileWrapper::OpenWriteOnly(filename), sample_rate, @@ -149,7 +148,7 @@ WavWriter::WavWriter(FileWrapper file, int sample_rate, size_t num_channels) num_channels_(num_channels), num_samples_(0), file_(std::move(file)) { - // Handle errors from the CreatePlatformFile call in above constructor. + // Handle errors from the OpenWriteOnly call in above constructor. RTC_CHECK(file_.is_open()) << "Invalid file. Could not create wav file."; RTC_CHECK(CheckWavParameters(num_channels_, sample_rate_, kWavFormat, diff --git a/common_audio/wav_file.h b/common_audio/wav_file.h index e071499579..65f2453736 100644 --- a/common_audio/wav_file.h +++ b/common_audio/wav_file.h @@ -17,7 +17,6 @@ #include #include "rtc_base/constructor_magic.h" -#include "rtc_base/platform_file.h" #include "rtc_base/system/file_wrapper.h" namespace webrtc { @@ -71,6 +70,9 @@ class WavReader final : public WavFile { // Opens an existing WAV file for reading. explicit WavReader(const std::string& filename); + // Use an existing WAV file for reading. + explicit WavReader(FileWrapper file); + // Close the WAV file. ~WavReader() override; @@ -87,16 +89,14 @@ class WavReader final : public WavFile { size_t num_samples() const override; private: - // Opens an existing WAV file for reading. - explicit WavReader(rtc::PlatformFile file); - void Close(); int sample_rate_; size_t num_channels_; size_t num_samples_; // Total number of samples in the file. size_t num_samples_remaining_; - FILE* file_handle_; // Input file, owned by this class. - fpos_t data_start_pos_; // Position in the file immediately after WAV header. + FileWrapper file_; // Input file, owned by this class. + int64_t + data_start_pos_; // Position in the file immediately after WAV header. RTC_DISALLOW_COPY_AND_ASSIGN(WavReader); }; diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn index 713fb34ef9..fab08725cb 100644 --- a/rtc_base/BUILD.gn +++ b/rtc_base/BUILD.gn @@ -92,8 +92,6 @@ rtc_source_set("rtc_base_approved") { "numerics/sample_counter.cc", "numerics/sample_counter.h", "one_time_event.h", - "platform_file.cc", - "platform_file.h", "race_checker.cc", "race_checker.h", "random.cc", @@ -1162,7 +1160,6 @@ if (rtc_include_tests) { "numerics/safe_minmax_unittest.cc", "numerics/sample_counter_unittest.cc", "one_time_event_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 deleted file mode 100644 index e4f8b5e492..0000000000 --- a/rtc_base/platform_file.cc +++ /dev/null @@ -1,95 +0,0 @@ -/* - * Copyright 2014 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" - -#if defined(WEBRTC_WIN) -#include - -#include "rtc_base/string_utils.h" // For ToUtf16 -#else -#include -#include -#include -#endif - -namespace rtc { - -FILE* FdopenPlatformFileForWriting(PlatformFile file) { - return FdopenPlatformFile(file, "w"); -} - -#if defined(WEBRTC_WIN) -const PlatformFile kInvalidPlatformFileValue = INVALID_HANDLE_VALUE; - -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, modes); -} - -bool ClosePlatformFile(PlatformFile file) { - return CloseHandle(file) != 0; -} - -bool RemoveFile(const std::string& path) { - return ::DeleteFileW(ToUtf16(path).c_str()) != 0; -} - -PlatformFile OpenPlatformFile(const std::string& path) { - return ::CreateFileW(ToUtf16(path).c_str(), GENERIC_READ | GENERIC_WRITE, 0, - nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr); -} - -PlatformFile OpenPlatformFileReadOnly(const std::string& path) { - return ::CreateFileW(ToUtf16(path).c_str(), GENERIC_READ, FILE_SHARE_READ, - nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr); -} - -PlatformFile CreatePlatformFile(const std::string& path) { - return ::CreateFileW(ToUtf16(path).c_str(), GENERIC_READ | GENERIC_WRITE, 0, - nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr); -} - -#else // defined(WEBRTC_WIN) - -const PlatformFile kInvalidPlatformFileValue = -1; - -FILE* FdopenPlatformFile(PlatformFile file, const char* modes) { - return fdopen(file, modes); -} - -bool ClosePlatformFile(PlatformFile file) { - return close(file) == 0; -} - -bool RemoveFile(const std::string& path) { - return ::unlink(path.c_str()) == 0; -} - -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); -} - -#endif - -} // namespace rtc diff --git a/rtc_base/platform_file.h b/rtc_base/platform_file.h deleted file mode 100644 index 303d9f5f7b..0000000000 --- a/rtc_base/platform_file.h +++ /dev/null @@ -1,66 +0,0 @@ -/* - * Copyright 2014 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. - */ - -#ifndef RTC_BASE_PLATFORM_FILE_H_ -#define RTC_BASE_PLATFORM_FILE_H_ - -#include - -#include - -#if defined(WEBRTC_WIN) -#include -#endif - -namespace rtc { - -#if defined(WEBRTC_WIN) -typedef HANDLE PlatformFile; -#elif defined(WEBRTC_POSIX) -typedef int PlatformFile; -#else -#error Unsupported platform -#endif - -extern const PlatformFile kInvalidPlatformFileValue; - -// 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* 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. -bool ClosePlatformFile(PlatformFile file); - -// Removes a file in the filesystem. -bool RemoveFile(const std::string& path); - -// Opens a file for reading and writing. You might want to use base/file.h -// 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); - -} // namespace rtc - -#endif // RTC_BASE_PLATFORM_FILE_H_ diff --git a/rtc_base/platform_file_unittest.cc b/rtc_base/platform_file_unittest.cc deleted file mode 100644 index 26cf6a1d78..0000000000 --- a/rtc_base/platform_file_unittest.cc +++ /dev/null @@ -1,78 +0,0 @@ -/* - * 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/file_utils.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/rtc_base/system/BUILD.gn b/rtc_base/system/BUILD.gn index d39f1f20e0..ddb0b5b14a 100644 --- a/rtc_base/system/BUILD.gn +++ b/rtc_base/system/BUILD.gn @@ -38,6 +38,7 @@ rtc_source_set("file_wrapper") { deps = [ "..:checks", "..:criticalsection", + "..:safe_conversions", ] } diff --git a/rtc_base/system/file_wrapper.cc b/rtc_base/system/file_wrapper.cc index 9d99cefaac..5409d74ef6 100644 --- a/rtc_base/system/file_wrapper.cc +++ b/rtc_base/system/file_wrapper.cc @@ -9,6 +9,7 @@ */ #include "rtc_base/system/file_wrapper.h" +#include "rtc_base/numerics/safe_conversions.h" #include @@ -78,9 +79,14 @@ FileWrapper& FileWrapper::operator=(FileWrapper&& other) { return *this; } -bool FileWrapper::Rewind() { +bool FileWrapper::SeekRelative(int64_t offset) { RTC_DCHECK(file_); - return fseek(file_, 0, SEEK_SET) == 0; + return fseek(file_, rtc::checked_cast(offset), SEEK_CUR) == 0; +} + +bool FileWrapper::SeekTo(int64_t position) { + RTC_DCHECK(file_); + return fseek(file_, rtc::checked_cast(position), SEEK_SET) == 0; } bool FileWrapper::Flush() { @@ -93,6 +99,11 @@ size_t FileWrapper::Read(void* buf, size_t length) { return fread(buf, 1, length, file_); } +bool FileWrapper::ReadEof() const { + RTC_DCHECK(file_); + return feof(file_); +} + bool FileWrapper::Write(const void* buf, size_t length) { RTC_DCHECK(file_); return fwrite(buf, 1, length, file_) == length; diff --git a/rtc_base/system/file_wrapper.h b/rtc_base/system/file_wrapper.h index 9062abd6bf..63d1c17c11 100644 --- a/rtc_base/system/file_wrapper.h +++ b/rtc_base/system/file_wrapper.h @@ -72,11 +72,23 @@ class FileWrapper final { // Seeks to the beginning of file. Returns true on success, false on failure, // e.g., if the underlying file isn't seekable. - bool Rewind(); + bool Rewind() { return SeekTo(0); } + // TODO(nisse): The seek functions are used only by the WavReader. If that + // code is demoted to test code, seek functions can be deleted from this + // utility. + // Seek relative to current file position. + bool SeekRelative(int64_t offset); + // Seek to given position. + bool SeekTo(int64_t position); // Returns number of bytes read. Short count indicates EOF or error. size_t Read(void* buf, size_t length); + // If the most recent Read() returned a short count, this methods returns true + // if the short count was due to EOF, and false it it was due to some i/o + // error. + bool ReadEof() const; + // Returns true if all data was successfully written (or buffered), or false // if there was an error. Writing buffered data can fail later, and is // reported with return value from Flush or Close.