Delete class PlatformFile.
Add seek methods to FileWrapper, and refactor WavReader to use that
class instead.
Bug: webrtc:6463
Change-Id: Ifbb1989a072da6280ea5fc04b4beff991614dd53
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/147265
Commit-Queue: Niels Moller <nisse@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28770}
diff --git a/common_audio/wav_file.cc b/common_audio/wav_file.cc
index 3f2aa7e..30968c2 100644
--- a/common_audio/wav_file.cc
+++ b/common_audio/wav_file.cc
@@ -34,40 +34,38 @@
// 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 @@
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 @@
}
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 @@
#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 @@
}
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 @@
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 e071499..65f2453 100644
--- a/common_audio/wav_file.h
+++ b/common_audio/wav_file.h
@@ -17,7 +17,6 @@
#include <string>
#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 @@
// 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 @@
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 713fb34..fab0872 100644
--- a/rtc_base/BUILD.gn
+++ b/rtc_base/BUILD.gn
@@ -92,8 +92,6 @@
"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 @@
"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 e4f8b5e..0000000
--- 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 <io.h>
-
-#include "rtc_base/string_utils.h" // For ToUtf16
-#else
-#include <fcntl.h>
-#include <sys/stat.h>
-#include <unistd.h>
-#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<intptr_t>(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 303d9f5..0000000
--- 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 <stdio.h>
-
-#include <string>
-
-#if defined(WEBRTC_WIN)
-#include <windows.h>
-#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 26cf6a1..0000000
--- 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 d39f1f2..ddb0b5b 100644
--- a/rtc_base/system/BUILD.gn
+++ b/rtc_base/system/BUILD.gn
@@ -38,6 +38,7 @@
deps = [
"..:checks",
"..:criticalsection",
+ "..:safe_conversions",
]
}
diff --git a/rtc_base/system/file_wrapper.cc b/rtc_base/system/file_wrapper.cc
index 9d99cef..5409d74 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 <cerrno>
@@ -78,9 +79,14 @@
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<long>(offset), SEEK_CUR) == 0;
+}
+
+bool FileWrapper::SeekTo(int64_t position) {
+ RTC_DCHECK(file_);
+ return fseek(file_, rtc::checked_cast<long>(position), SEEK_SET) == 0;
}
bool FileWrapper::Flush() {
@@ -93,6 +99,11 @@
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 9062abd..63d1c17 100644
--- a/rtc_base/system/file_wrapper.h
+++ b/rtc_base/system/file_wrapper.h
@@ -72,11 +72,23 @@
// 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.