From a6219cc3ef08dd9b2981b065b6f051d7de0866f8 Mon Sep 17 00:00:00 2001 From: tommi Date: Wed, 15 Jun 2016 10:30:14 -0700 Subject: [PATCH] FileWrapper[Impl] modifications and actually remove the "Impl" class. This is a somewhat involved refactoring of this class. Here's an overview of the changes: * FileWrapper can now be used as a regular class and instances allocated on the stack. * The type now has support for move semantics and copy isn't allowed. * New public ctor with FILE* that can be used instead of OpenFromFileHandle. * New static Open() method. The intent of this is to allow opening a file and getting back a FileWrapper instance. Using this method instead of Create(), will allow us in the future to make the FILE* member pointer, to be const and simplify threading (get rid of the lock). * Rename the Open() method to is_open() and make it inline. * The FileWrapper interface is no longer a pure virtual interface. There's only one implementation so there's no need to go through a vtable for everything. * Functionality offered by the class, is now reduced. No support for looping (not clear if that was actually useful to users of that flag), no need to implement the 'read_only_' functionality in the class, since file APIs implement that already, no support for *not* managing the file handle (this wasn't used). OpenFromFileHandle always "manages" the file. * Delete the unused WriteText() method and don't support opening files in text mode. Text mode is only different on Windows and on Windows it translates \n to \r\n, which means that files such as log files, could have a slightly different format on Windows than other platforms. Besides, tools on Windows can handle UNIX line endings. * Remove FileName(), change Trace code to manage its own path. * Rename id_ member variable to file_. * Removed the open_ member variable since the same functionality can be gotten from just checking the file pointer. * Don't call CloseFile inside of Write. Write shouldn't be changing the state of the class beyond just attempting to write. * Remove concept of looping from FileWrapper and never close inside of Read() * Changed stream base classes to inherit from a common base class instead of both defining the Rewind method. Ultimately, Id' like to remove these interfaces and just have FileWrapper. * Remove read_only param from OpenFromFileHandle * Renamed size_in_bytes_ to position_, since it gets set to 0 when Rewind() is called (and the size actually does not change). * Switch out rw lock for CriticalSection. The r/w lock was only used for reading when checking the open_ flag. BUG= Review-Url: https://codereview.webrtc.org/2054373002 Cr-Commit-Position: refs/heads/master@{#13155} --- webrtc/call/rtc_event_log.cc | 6 +- webrtc/call/rtc_event_log_helper_thread.cc | 30 +- webrtc/common_types.cc | 9 +- webrtc/common_types.h | 36 +- .../audio_device/audio_device_buffer.cc | 18 +- .../audio_device/dummy/file_audio_device.cc | 28 +- .../audio_device/test/func_test_manager.cc | 60 ++- .../audio_processing/audio_processing_impl.cc | 45 +-- .../transient/click_annotate.cc | 8 +- .../audio_processing/transient/file_utils.cc | 16 +- .../transient/file_utils_unittest.cc | 108 ++---- .../transient/transient_detector_unittest.cc | 10 +- .../transient/wpd_tree_unittest.cc | 21 +- webrtc/modules/media_file/media_file_impl.cc | 23 +- .../modules/media_file/media_file_utility.cc | 11 +- .../video_coding/utility/ivf_file_writer.cc | 10 +- .../utility/ivf_file_writer_unittest.cc | 8 +- webrtc/system_wrappers/BUILD.gn | 1 - webrtc/system_wrappers/include/file_wrapper.h | 72 ++-- webrtc/system_wrappers/include/trace.h | 3 - webrtc/system_wrappers/source/data_log.cc | 12 +- webrtc/system_wrappers/source/file_impl.cc | 345 ++++++------------ webrtc/system_wrappers/source/file_impl.h | 70 ---- webrtc/system_wrappers/source/trace_impl.cc | 54 +-- webrtc/system_wrappers/source/trace_impl.h | 8 +- webrtc/system_wrappers/system_wrappers.gyp | 1 - webrtc/test/fake_audio_device.cc | 3 +- 27 files changed, 348 insertions(+), 668 deletions(-) delete mode 100644 webrtc/system_wrappers/source/file_impl.h diff --git a/webrtc/call/rtc_event_log.cc b/webrtc/call/rtc_event_log.cc index 90145735ed..7ea1e167a1 100644 --- a/webrtc/call/rtc_event_log.cc +++ b/webrtc/call/rtc_event_log.cc @@ -188,7 +188,7 @@ bool RtcEventLogImpl::StartLogging(const std::string& file_name, message.start_time = clock_->TimeInMicroseconds(); message.stop_time = std::numeric_limits::max(); message.file.reset(FileWrapper::Create()); - if (message.file->OpenFile(file_name.c_str(), false) != 0) { + if (!message.file->OpenFile(file_name.c_str(), false)) { LOG(LS_ERROR) << "Can't open file. WebRTC event log not started."; return false; } @@ -222,7 +222,7 @@ bool RtcEventLogImpl::StartLogging(rtc::PlatformFile platform_file, } return false; } - if (message.file->OpenFromFileHandle(file_handle, true, false) != 0) { + if (!message.file->OpenFromFileHandle(file_handle)) { LOG(LS_ERROR) << "Can't open file. WebRTC event log not started."; return false; } @@ -443,7 +443,7 @@ bool RtcEventLog::ParseRtcEventLog(const std::string& file_name, char tmp_buffer[1024]; int bytes_read = 0; std::unique_ptr dump_file(FileWrapper::Create()); - if (dump_file->OpenFile(file_name.c_str(), true) != 0) { + if (!dump_file->OpenFile(file_name.c_str(), true)) { return false; } std::string dump_buffer; diff --git a/webrtc/call/rtc_event_log_helper_thread.cc b/webrtc/call/rtc_event_log_helper_thread.cc index 6479908e62..b1b6990a24 100644 --- a/webrtc/call/rtc_event_log_helper_thread.cc +++ b/webrtc/call/rtc_event_log_helper_thread.cc @@ -107,7 +107,7 @@ bool RtcEventLogHelperThread::AppendEventToString(rtclog::Event* event) { } bool RtcEventLogHelperThread::LogToMemory() { - RTC_DCHECK(!file_->Open()); + RTC_DCHECK(!file_->is_open()); bool message_received = false; // Process each event earlier than the current time and append it to the @@ -130,7 +130,7 @@ bool RtcEventLogHelperThread::LogToMemory() { } void RtcEventLogHelperThread::StartLogFile() { - RTC_DCHECK(file_->Open()); + RTC_DCHECK(file_->is_open()); bool stop = false; output_string_.clear(); @@ -157,7 +157,7 @@ void RtcEventLogHelperThread::StartLogFile() { if (!file_->Write(output_string_.data(), output_string_.size())) { LOG(LS_ERROR) << "FileWrapper failed to write WebRtcEventLog file."; // The current FileWrapper implementation closes the file on error. - RTC_DCHECK(!file_->Open()); + RTC_DCHECK(!file_->is_open()); return; } written_bytes_ += output_string_.size(); @@ -168,13 +168,13 @@ void RtcEventLogHelperThread::StartLogFile() { output_string_.shrink_to_fit(); if (stop) { - RTC_DCHECK(file_->Open()); + RTC_DCHECK(file_->is_open()); StopLogFile(); } } bool RtcEventLogHelperThread::LogToFile() { - RTC_DCHECK(file_->Open()); + RTC_DCHECK(file_->is_open()); output_string_.clear(); bool message_received = false; @@ -202,7 +202,7 @@ bool RtcEventLogHelperThread::LogToFile() { if (!file_->Write(output_string_.data(), output_string_.size())) { LOG(LS_ERROR) << "FileWrapper failed to write WebRtcEventLog file."; // The current FileWrapper implementation closes the file on error. - RTC_DCHECK(!file_->Open()); + RTC_DCHECK(!file_->is_open()); return message_received; } written_bytes_ += output_string_.size(); @@ -213,14 +213,14 @@ bool RtcEventLogHelperThread::LogToFile() { // having more events in the queue. if ((has_recent_event_ && most_recent_event_->timestamp_us() > stop_time_) || stop) { - RTC_DCHECK(file_->Open()); + RTC_DCHECK(file_->is_open()); StopLogFile(); } return message_received; } void RtcEventLogHelperThread::StopLogFile() { - RTC_DCHECK(file_->Open()); + RTC_DCHECK(file_->is_open()); output_string_.clear(); rtclog::Event end_event; @@ -233,7 +233,7 @@ void RtcEventLogHelperThread::StopLogFile() { if (!file_->Write(output_string_.data(), output_string_.size())) { LOG(LS_ERROR) << "FileWrapper failed to write WebRtcEventLog file."; // The current FileWrapper implementation closes the file on error. - RTC_DCHECK(!file_->Open()); + RTC_DCHECK(!file_->is_open()); } written_bytes_ += output_string_.size(); } @@ -244,7 +244,7 @@ void RtcEventLogHelperThread::StopLogFile() { stop_time_ = std::numeric_limits::max(); output_string_.clear(); file_->CloseFile(); - RTC_DCHECK(!file_->Open()); + RTC_DCHECK(!file_->is_open()); } void RtcEventLogHelperThread::ProcessEvents() { @@ -256,7 +256,7 @@ void RtcEventLogHelperThread::ProcessEvents() { while (message_queue_->Remove(&message)) { switch (message.message_type) { case ControlMessage::START_FILE: - if (!file_->Open()) { + if (!file_->is_open()) { max_size_bytes_ = message.max_size_bytes; start_time_ = message.start_time; stop_time_ = message.stop_time; @@ -269,19 +269,19 @@ void RtcEventLogHelperThread::ProcessEvents() { message_received = true; break; case ControlMessage::STOP_FILE: - if (file_->Open()) { + if (file_->is_open()) { stop_time_ = message.stop_time; LogToFile(); // Log remaining events from message queues. } // LogToFile might stop on it's own so we need to recheck the state. - if (file_->Open()) { + if (file_->is_open()) { StopLogFile(); } file_finished_.Set(); message_received = true; break; case ControlMessage::TERMINATE_THREAD: - if (file_->Open()) { + if (file_->is_open()) { StopLogFile(); } return; @@ -289,7 +289,7 @@ void RtcEventLogHelperThread::ProcessEvents() { } // Write events to file or memory. - if (file_->Open()) { + if (file_->is_open()) { message_received |= LogToFile(); } else { message_received |= LogToMemory(); diff --git a/webrtc/common_types.cc b/webrtc/common_types.cc index d08630b46d..86e7813e58 100644 --- a/webrtc/common_types.cc +++ b/webrtc/common_types.cc @@ -14,10 +14,6 @@ namespace webrtc { -int InStream::Rewind() { return -1; } - -int OutStream::Rewind() { return -1; } - StreamDataCounters::StreamDataCounters() : first_packet_time_ms(-1) {} RTPHeaderExtension::RTPHeaderExtension() @@ -40,11 +36,10 @@ RTPHeader::RTPHeader() timestamp(0), ssrc(0), numCSRCs(0), + arrOfCSRCs(), paddingLength(0), headerLength(0), payload_type_frequency(0), - extension() { - memset(&arrOfCSRCs, 0, sizeof(arrOfCSRCs)); -} + extension() {} } // namespace webrtc diff --git a/webrtc/common_types.h b/webrtc/common_types.h index 02633ba87a..4c77c77944 100644 --- a/webrtc/common_types.h +++ b/webrtc/common_types.h @@ -54,28 +54,24 @@ namespace webrtc { class Config; -class InStream -{ -public: - // Reads |length| bytes from file to |buf|. Returns the number of bytes read - // or -1 on error. - virtual int Read(void *buf, size_t len) = 0; - virtual int Rewind(); - virtual ~InStream() {} -protected: - InStream() {} +class RewindableStream { + public: + virtual ~RewindableStream() {} + virtual int Rewind() = 0; }; -class OutStream -{ -public: - // Writes |length| bytes from |buf| to file. The actual writing may happen - // some time later. Call Flush() to force a write. - virtual bool Write(const void *buf, size_t len) = 0; - virtual int Rewind(); - virtual ~OutStream() {} -protected: - OutStream() {} +class InStream : public RewindableStream { + public: + // Reads |len| bytes from file to |buf|. Returns the number of bytes read + // or -1 on error. + virtual int Read(void* buf, size_t len) = 0; +}; + +class OutStream : public RewindableStream { + public: + // Writes |len| bytes from |buf| to file. The actual writing may happen + // some time later. Call Flush() to force a write. + virtual bool Write(const void* buf, size_t len) = 0; }; enum TraceModule diff --git a/webrtc/modules/audio_device/audio_device_buffer.cc b/webrtc/modules/audio_device/audio_device_buffer.cc index 48ae88ee90..0bc2f5c225 100644 --- a/webrtc/modules/audio_device/audio_device_buffer.cc +++ b/webrtc/modules/audio_device/audio_device_buffer.cc @@ -313,7 +313,7 @@ int32_t AudioDeviceBuffer::StartInputFileRecording( _recFile.Flush(); _recFile.CloseFile(); - return (_recFile.OpenFile(fileName, false, false, false)); + return _recFile.OpenFile(fileName, false) ? 0 : -1; } // ---------------------------------------------------------------------------- @@ -346,7 +346,7 @@ int32_t AudioDeviceBuffer::StartOutputFileRecording( _playFile.Flush(); _playFile.CloseFile(); - return (_playFile.OpenFile(fileName, false, false, false)); + return _playFile.OpenFile(fileName, false) ? 0 : -1; } // ---------------------------------------------------------------------------- @@ -424,10 +424,9 @@ int32_t AudioDeviceBuffer::SetRecordedBuffer(const void* audioBuffer, } } - if (_recFile.Open()) - { - // write to binary file in mono or stereo (interleaved) - _recFile.Write(&_recBuffer[0], _recSize); + if (_recFile.is_open()) { + // write to binary file in mono or stereo (interleaved) + _recFile.Write(&_recBuffer[0], _recSize); } return 0; @@ -572,10 +571,9 @@ int32_t AudioDeviceBuffer::GetPlayoutData(void* audioBuffer) memcpy(audioBuffer, &_playBuffer[0], _playSize); - if (_playFile.Open()) - { - // write to binary file in mono or stereo (interleaved) - _playFile.Write(&_playBuffer[0], _playSize); + if (_playFile.is_open()) { + // write to binary file in mono or stereo (interleaved) + _playFile.Write(&_playBuffer[0], _playSize); } return static_cast(_playSamples); diff --git a/webrtc/modules/audio_device/dummy/file_audio_device.cc b/webrtc/modules/audio_device/dummy/file_audio_device.cc index aac0962a50..35e77a9d6a 100644 --- a/webrtc/modules/audio_device/dummy/file_audio_device.cc +++ b/webrtc/modules/audio_device/dummy/file_audio_device.cc @@ -46,15 +46,7 @@ FileAudioDevice::FileAudioDevice(const int32_t id, } FileAudioDevice::~FileAudioDevice() { - if (_outputFile.Open()) { - _outputFile.Flush(); - _outputFile.CloseFile(); - } delete &_outputFile; - if (_inputFile.Open()) { - _inputFile.Flush(); - _inputFile.CloseFile(); - } delete &_inputFile; } @@ -202,8 +194,8 @@ int32_t FileAudioDevice::StartPlayout() { } // PLAYOUT - if (!_outputFilename.empty() && _outputFile.OpenFile( - _outputFilename.c_str(), false, false, false) == -1) { + if (!_outputFilename.empty() && + !_outputFile.OpenFile(_outputFilename.c_str(), false)) { printf("Failed to open playout file %s!\n", _outputFilename.c_str()); _playing = false; delete [] _playoutBuffer; @@ -235,11 +227,8 @@ int32_t FileAudioDevice::StopPlayout() { _playoutFramesLeft = 0; delete [] _playoutBuffer; _playoutBuffer = NULL; - if (_outputFile.Open()) { - _outputFile.Flush(); - _outputFile.CloseFile(); - } - return 0; + _outputFile.CloseFile(); + return 0; } bool FileAudioDevice::Playing() const { @@ -257,8 +246,8 @@ int32_t FileAudioDevice::StartRecording() { _recordingBuffer = new int8_t[_recordingBufferSizeIn10MS]; } - if (!_inputFilename.empty() && _inputFile.OpenFile( - _inputFilename.c_str(), true, true, false) == -1) { + if (!_inputFilename.empty() && + !_inputFile.OpenFile(_inputFilename.c_str(), true)) { printf("Failed to open audio input file %s!\n", _inputFilename.c_str()); _recording = false; @@ -490,9 +479,8 @@ bool FileAudioDevice::PlayThreadProcess() _playoutFramesLeft = _ptrAudioBuffer->GetPlayoutData(_playoutBuffer); assert(_playoutFramesLeft == _playoutFramesIn10MS); - if (_outputFile.Open()) { + if (_outputFile.is_open()) { _outputFile.Write(_playoutBuffer, kPlayoutBufferSize); - _outputFile.Flush(); } _lastCallPlayoutMillis = currentTime; } @@ -518,7 +506,7 @@ bool FileAudioDevice::RecThreadProcess() if (_lastCallRecordMillis == 0 || currentTime - _lastCallRecordMillis >= 10) { - if (_inputFile.Open()) { + if (_inputFile.is_open()) { if (_inputFile.Read(_recordingBuffer, kRecordingBufferSize) > 0) { _ptrAudioBuffer->SetRecordedBuffer(_recordingBuffer, _recordingFramesIn10MS); diff --git a/webrtc/modules/audio_device/test/func_test_manager.cc b/webrtc/modules/audio_device/test/func_test_manager.cc index f16f296011..6e58841b22 100644 --- a/webrtc/modules/audio_device/test/func_test_manager.cc +++ b/webrtc/modules/audio_device/test/func_test_manager.cc @@ -169,15 +169,10 @@ int32_t AudioTransportImpl::SetFilePlayout(bool enable, const char* fileName) { _playFromFile = enable; if (enable) - { - return (_playFile.OpenFile(fileName, true, true, false)); - } else - { - _playFile.Flush(); - return (_playFile.CloseFile()); - } + return _playFile.OpenFile(fileName, true) ? 0 : -1; + _playFile.CloseFile(); + return 0; } -; void AudioTransportImpl::SetFullDuplex(bool enable) { @@ -462,36 +457,31 @@ int32_t AudioTransportImpl::NeedMorePlayData( } } // if (_fullDuplex) - if (_playFromFile && _playFile.Open()) - { - int16_t fileBuf[480]; + if (_playFromFile && _playFile.is_open()) { + int16_t fileBuf[480]; - // read mono-file - int32_t len = _playFile.Read((int8_t*) fileBuf, 2 * nSamples); - if (len != 2 * (int32_t) nSamples) - { - _playFile.Rewind(); - _playFile.Read((int8_t*) fileBuf, 2 * nSamples); - } + // read mono-file + int32_t len = _playFile.Read((int8_t*)fileBuf, 2 * nSamples); + if (len != 2 * (int32_t)nSamples) { + _playFile.Rewind(); + _playFile.Read((int8_t*)fileBuf, 2 * nSamples); + } - // convert to stero if required - if (nChannels == 1) - { - memcpy(audioSamples, fileBuf, 2 * nSamples); - } else - { - // mono sample from file is duplicated and sent to left and right - // channels - int16_t* audio16 = (int16_t*) audioSamples; - for (size_t i = 0; i < nSamples; i++) - { - (*audio16) = fileBuf[i]; // left - audio16++; - (*audio16) = fileBuf[i]; // right - audio16++; - } + // convert to stero if required + if (nChannels == 1) { + memcpy(audioSamples, fileBuf, 2 * nSamples); + } else { + // mono sample from file is duplicated and sent to left and right + // channels + int16_t* audio16 = (int16_t*)audioSamples; + for (size_t i = 0; i < nSamples; i++) { + (*audio16) = fileBuf[i]; // left + audio16++; + (*audio16) = fileBuf[i]; // right + audio16++; } - } // if (_playFromFile && _playFile.Open()) + } + } // if (_playFromFile && _playFile.is_open()) _playCount++; diff --git a/webrtc/modules/audio_processing/audio_processing_impl.cc b/webrtc/modules/audio_processing/audio_processing_impl.cc index e75b328034..819a18b62d 100644 --- a/webrtc/modules/audio_processing/audio_processing_impl.cc +++ b/webrtc/modules/audio_processing/audio_processing_impl.cc @@ -211,9 +211,7 @@ AudioProcessingImpl::~AudioProcessingImpl() { public_submodules_->gain_control_for_experimental_agc.reset(); #ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP - if (debug_dump_.debug_file->Open()) { - debug_dump_.debug_file->CloseFile(); - } + debug_dump_.debug_file->CloseFile(); #endif } @@ -326,7 +324,7 @@ int AudioProcessingImpl::InitializeLocked() { InitializeVoiceDetection(); #ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP - if (debug_dump_.debug_file->Open()) { + if (debug_dump_.debug_file->is_open()) { int err = WriteInitMessage(); if (err != kNoError) { return err; @@ -537,7 +535,7 @@ int AudioProcessingImpl::ProcessStream(const float* const* src, formats_.api_format.input_stream().num_frames()); #ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP - if (debug_dump_.debug_file->Open()) { + if (debug_dump_.debug_file->is_open()) { RETURN_ON_ERR(WriteConfigMessage(false)); debug_dump_.capture.event_msg->set_type(audioproc::Event::STREAM); @@ -555,7 +553,7 @@ int AudioProcessingImpl::ProcessStream(const float* const* src, capture_.capture_audio->CopyTo(formats_.api_format.output_stream(), dest); #ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP - if (debug_dump_.debug_file->Open()) { + if (debug_dump_.debug_file->is_open()) { audioproc::Stream* msg = debug_dump_.capture.event_msg->mutable_stream(); const size_t channel_size = sizeof(float) * formats_.api_format.output_stream().num_frames(); @@ -624,7 +622,7 @@ int AudioProcessingImpl::ProcessStream(AudioFrame* frame) { } #ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP - if (debug_dump_.debug_file->Open()) { + if (debug_dump_.debug_file->is_open()) { debug_dump_.capture.event_msg->set_type(audioproc::Event::STREAM); audioproc::Stream* msg = debug_dump_.capture.event_msg->mutable_stream(); const size_t data_size = @@ -638,7 +636,7 @@ int AudioProcessingImpl::ProcessStream(AudioFrame* frame) { capture_.capture_audio->InterleaveTo(frame, output_copy_needed()); #ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP - if (debug_dump_.debug_file->Open()) { + if (debug_dump_.debug_file->is_open()) { audioproc::Stream* msg = debug_dump_.capture.event_msg->mutable_stream(); const size_t data_size = sizeof(int16_t) * frame->samples_per_channel_ * frame->num_channels_; @@ -660,7 +658,7 @@ int AudioProcessingImpl::ProcessStreamLocked() { public_submodules_->echo_control_mobile->is_enabled())); #ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP - if (debug_dump_.debug_file->Open()) { + if (debug_dump_.debug_file->is_open()) { audioproc::Stream* msg = debug_dump_.capture.event_msg->mutable_stream(); msg->set_delay(capture_nonlocked_.stream_delay_ms); msg->set_drift( @@ -828,7 +826,7 @@ int AudioProcessingImpl::AnalyzeReverseStreamLocked( formats_.api_format.reverse_input_stream().num_frames()); #ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP - if (debug_dump_.debug_file->Open()) { + if (debug_dump_.debug_file->is_open()) { debug_dump_.render.event_msg->set_type(audioproc::Event::REVERSE_STREAM); audioproc::ReverseStream* msg = debug_dump_.render.event_msg->mutable_reverse_stream(); @@ -883,7 +881,7 @@ int AudioProcessingImpl::ProcessReverseStream(AudioFrame* frame) { } #ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP - if (debug_dump_.debug_file->Open()) { + if (debug_dump_.debug_file->is_open()) { debug_dump_.render.event_msg->set_type(audioproc::Event::REVERSE_STREAM); audioproc::ReverseStream* msg = debug_dump_.render.event_msg->mutable_reverse_stream(); @@ -990,14 +988,9 @@ int AudioProcessingImpl::StartDebugRecording( #ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP debug_dump_.num_bytes_left_for_log_ = max_log_size_bytes; // Stop any ongoing recording. - if (debug_dump_.debug_file->Open()) { - if (debug_dump_.debug_file->CloseFile() == -1) { - return kFileError; - } - } + debug_dump_.debug_file->CloseFile(); - if (debug_dump_.debug_file->OpenFile(filename, false) == -1) { - debug_dump_.debug_file->CloseFile(); + if (!debug_dump_.debug_file->OpenFile(filename, false)) { return kFileError; } @@ -1023,13 +1016,9 @@ int AudioProcessingImpl::StartDebugRecording(FILE* handle, debug_dump_.num_bytes_left_for_log_ = max_log_size_bytes; // Stop any ongoing recording. - if (debug_dump_.debug_file->Open()) { - if (debug_dump_.debug_file->CloseFile() == -1) { - return kFileError; - } - } + debug_dump_.debug_file->CloseFile(); - if (debug_dump_.debug_file->OpenFromFileHandle(handle, true, false) == -1) { + if (!debug_dump_.debug_file->OpenFromFileHandle(handle)) { return kFileError; } @@ -1057,11 +1046,7 @@ int AudioProcessingImpl::StopDebugRecording() { #ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP // We just return if recording hasn't started. - if (debug_dump_.debug_file->Open()) { - if (debug_dump_.debug_file->CloseFile() == -1) { - return kFileError; - } - } + debug_dump_.debug_file->CloseFile(); return kNoError; #else return kUnsupportedFunctionError; @@ -1362,7 +1347,7 @@ int AudioProcessingImpl::WriteMessageToDebugFile( // Ensure atomic writes of the message. rtc::CritScope cs_debug(crit_debug); - RTC_DCHECK(debug_file->Open()); + RTC_DCHECK(debug_file->is_open()); // Update the byte counter. if (*filesize_limit_bytes >= 0) { *filesize_limit_bytes -= diff --git a/webrtc/modules/audio_processing/transient/click_annotate.cc b/webrtc/modules/audio_processing/transient/click_annotate.cc index dd1c346ebd..b03a714c28 100644 --- a/webrtc/modules/audio_processing/transient/click_annotate.cc +++ b/webrtc/modules/audio_processing/transient/click_annotate.cc @@ -40,15 +40,15 @@ int main(int argc, char* argv[]) { } std::unique_ptr pcm_file(FileWrapper::Create()); - pcm_file->OpenFile(argv[1], true, false, false); - if (!pcm_file->Open()) { + pcm_file->OpenFile(argv[1], true); + if (!pcm_file->is_open()) { printf("\nThe %s could not be opened.\n\n", argv[1]); return -1; } std::unique_ptr dat_file(FileWrapper::Create()); - dat_file->OpenFile(argv[2], false, false, false); - if (!dat_file->Open()) { + dat_file->OpenFile(argv[2], false); + if (!dat_file->is_open()) { printf("\nThe %s could not be opened.\n\n", argv[2]); return -1; } diff --git a/webrtc/modules/audio_processing/transient/file_utils.cc b/webrtc/modules/audio_processing/transient/file_utils.cc index ebe530663c..e69bf79353 100644 --- a/webrtc/modules/audio_processing/transient/file_utils.cc +++ b/webrtc/modules/audio_processing/transient/file_utils.cc @@ -80,7 +80,7 @@ int ConvertDoubleToByteArray(double value, uint8_t out_bytes[8]) { size_t ReadInt16BufferFromFile(FileWrapper* file, size_t length, int16_t* buffer) { - if (!file || !file->Open() || !buffer || length <= 0) { + if (!file || !file->is_open() || !buffer || length <= 0) { return 0; } @@ -106,7 +106,7 @@ size_t ReadInt16BufferFromFile(FileWrapper* file, size_t ReadInt16FromFileToFloatBuffer(FileWrapper* file, size_t length, float* buffer) { - if (!file || !file->Open() || !buffer || length <= 0) { + if (!file || !file->is_open() || !buffer || length <= 0) { return 0; } @@ -124,7 +124,7 @@ size_t ReadInt16FromFileToFloatBuffer(FileWrapper* file, size_t ReadInt16FromFileToDoubleBuffer(FileWrapper* file, size_t length, double* buffer) { - if (!file || !file->Open() || !buffer || length <= 0) { + if (!file || !file->is_open() || !buffer || length <= 0) { return 0; } @@ -142,7 +142,7 @@ size_t ReadInt16FromFileToDoubleBuffer(FileWrapper* file, size_t ReadFloatBufferFromFile(FileWrapper* file, size_t length, float* buffer) { - if (!file || !file->Open() || !buffer || length <= 0) { + if (!file || !file->is_open() || !buffer || length <= 0) { return 0; } @@ -165,7 +165,7 @@ size_t ReadFloatBufferFromFile(FileWrapper* file, size_t ReadDoubleBufferFromFile(FileWrapper* file, size_t length, double* buffer) { - if (!file || !file->Open() || !buffer || length <= 0) { + if (!file || !file->is_open() || !buffer || length <= 0) { return 0; } @@ -188,7 +188,7 @@ size_t ReadDoubleBufferFromFile(FileWrapper* file, size_t WriteInt16BufferToFile(FileWrapper* file, size_t length, const int16_t* buffer) { - if (!file || !file->Open() || !buffer || length <= 0) { + if (!file || !file->is_open() || !buffer || length <= 0) { return 0; } @@ -212,7 +212,7 @@ size_t WriteInt16BufferToFile(FileWrapper* file, size_t WriteFloatBufferToFile(FileWrapper* file, size_t length, const float* buffer) { - if (!file || !file->Open() || !buffer || length <= 0) { + if (!file || !file->is_open() || !buffer || length <= 0) { return 0; } @@ -235,7 +235,7 @@ size_t WriteFloatBufferToFile(FileWrapper* file, size_t WriteDoubleBufferToFile(FileWrapper* file, size_t length, const double* buffer) { - if (!file || !file->Open() || !buffer || length <= 0) { + if (!file || !file->is_open() || !buffer || length <= 0) { return 0; } diff --git a/webrtc/modules/audio_processing/transient/file_utils_unittest.cc b/webrtc/modules/audio_processing/transient/file_utils_unittest.cc index 8520413154..6a848c6539 100644 --- a/webrtc/modules/audio_processing/transient/file_utils_unittest.cc +++ b/webrtc/modules/audio_processing/transient/file_utils_unittest.cc @@ -163,12 +163,9 @@ TEST_F(TransientFileUtilsTest, MAYBE_ReadInt16BufferFromFile) { std::unique_ptr file(FileWrapper::Create()); - file->OpenFile(test_filename.c_str(), - true, // Read only. - true, // Loop. - false); // No text. - ASSERT_TRUE(file->Open()) << "File could not be opened:\n" - << kTestFileName.c_str(); + file->OpenFile(test_filename.c_str(), true); // Read only. + ASSERT_TRUE(file->is_open()) << "File could not be opened:\n" + << kTestFileName.c_str(); const size_t kBufferLength = 12; std::unique_ptr buffer(new int16_t[kBufferLength]); @@ -207,12 +204,9 @@ TEST_F(TransientFileUtilsTest, MAYBE_ReadInt16FromFileToFloatBuffer) { std::unique_ptr file(FileWrapper::Create()); - file->OpenFile(test_filename.c_str(), - true, // Read only. - true, // Loop. - false); // No text. - ASSERT_TRUE(file->Open()) << "File could not be opened:\n" - << kTestFileName.c_str(); + file->OpenFile(test_filename.c_str(), true); // Read only. + ASSERT_TRUE(file->is_open()) << "File could not be opened:\n" + << kTestFileName.c_str(); const size_t kBufferLength = 12; std::unique_ptr buffer(new float[kBufferLength]); @@ -254,12 +248,9 @@ TEST_F(TransientFileUtilsTest, MAYBE_ReadInt16FromFileToDoubleBuffer) { std::unique_ptr file(FileWrapper::Create()); - file->OpenFile(test_filename.c_str(), - true, // Read only. - true, // Loop. - false); // No text. - ASSERT_TRUE(file->Open()) << "File could not be opened:\n" - << kTestFileName.c_str(); + file->OpenFile(test_filename.c_str(), true); // Read only. + ASSERT_TRUE(file->is_open()) << "File could not be opened:\n" + << kTestFileName.c_str(); const size_t kBufferLength = 12; std::unique_ptr buffer(new double[kBufferLength]); @@ -299,12 +290,9 @@ TEST_F(TransientFileUtilsTest, MAYBE_ReadFloatBufferFromFile) { std::unique_ptr file(FileWrapper::Create()); - file->OpenFile(test_filename.c_str(), - true, // Read only. - true, // Loop. - false); // No text. - ASSERT_TRUE(file->Open()) << "File could not be opened:\n" - << kTestFileNamef.c_str(); + file->OpenFile(test_filename.c_str(), true); // Read only. + ASSERT_TRUE(file->is_open()) << "File could not be opened:\n" + << kTestFileNamef.c_str(); const size_t kBufferLength = 3; std::unique_ptr buffer(new float[kBufferLength]); @@ -341,12 +329,9 @@ TEST_F(TransientFileUtilsTest, MAYBE_ReadDoubleBufferFromFile) { std::unique_ptr file(FileWrapper::Create()); - file->OpenFile(test_filename.c_str(), - true, // Read only. - true, // Loop. - false); // No text. - ASSERT_TRUE(file->Open()) << "File could not be opened:\n" - << kTestFileName.c_str(); + file->OpenFile(test_filename.c_str(), true); // Read only. + ASSERT_TRUE(file->is_open()) << "File could not be opened:\n" + << kTestFileName.c_str(); const size_t kBufferLength = 3; std::unique_ptr buffer(new double[kBufferLength]); @@ -384,12 +369,9 @@ TEST_F(TransientFileUtilsTest, MAYBE_WriteInt16BufferToFile) { std::string kOutFileName = CreateTempFilename(test::OutputPath(), "utils_test"); - file->OpenFile(kOutFileName.c_str(), - false, // Write mode. - false, // No loop. - false); // No text. - ASSERT_TRUE(file->Open()) << "File could not be opened:\n" - << kOutFileName.c_str(); + file->OpenFile(kOutFileName.c_str(), false); // Write mode. + ASSERT_TRUE(file->is_open()) << "File could not be opened:\n" + << kOutFileName.c_str(); const size_t kBufferLength = 3; std::unique_ptr written_buffer(new int16_t[kBufferLength]); @@ -405,12 +387,9 @@ TEST_F(TransientFileUtilsTest, MAYBE_WriteInt16BufferToFile) { file->CloseFile(); - file->OpenFile(kOutFileName.c_str(), - true, // Read only. - false, // No loop. - false); // No text. - ASSERT_TRUE(file->Open()) << "File could not be opened:\n" - << kOutFileName.c_str(); + file->OpenFile(kOutFileName.c_str(), true); // Read only. + ASSERT_TRUE(file->is_open()) << "File could not be opened:\n" + << kOutFileName.c_str(); EXPECT_EQ(kBufferLength, ReadInt16BufferFromFile(file.get(), kBufferLength, @@ -431,12 +410,9 @@ TEST_F(TransientFileUtilsTest, MAYBE_WriteFloatBufferToFile) { std::string kOutFileName = CreateTempFilename(test::OutputPath(), "utils_test"); - file->OpenFile(kOutFileName.c_str(), - false, // Write mode. - false, // No loop. - false); // No text. - ASSERT_TRUE(file->Open()) << "File could not be opened:\n" - << kOutFileName.c_str(); + file->OpenFile(kOutFileName.c_str(), false); // Write mode. + ASSERT_TRUE(file->is_open()) << "File could not be opened:\n" + << kOutFileName.c_str(); const size_t kBufferLength = 3; std::unique_ptr written_buffer(new float[kBufferLength]); @@ -452,12 +428,9 @@ TEST_F(TransientFileUtilsTest, MAYBE_WriteFloatBufferToFile) { file->CloseFile(); - file->OpenFile(kOutFileName.c_str(), - true, // Read only. - false, // No loop. - false); // No text. - ASSERT_TRUE(file->Open()) << "File could not be opened:\n" - << kOutFileName.c_str(); + file->OpenFile(kOutFileName.c_str(), true); // Read only. + ASSERT_TRUE(file->is_open()) << "File could not be opened:\n" + << kOutFileName.c_str(); EXPECT_EQ(kBufferLength, ReadFloatBufferFromFile(file.get(), kBufferLength, @@ -478,12 +451,9 @@ TEST_F(TransientFileUtilsTest, MAYBE_WriteDoubleBufferToFile) { std::string kOutFileName = CreateTempFilename(test::OutputPath(), "utils_test"); - file->OpenFile(kOutFileName.c_str(), - false, // Write mode. - false, // No loop. - false); // No text. - ASSERT_TRUE(file->Open()) << "File could not be opened:\n" - << kOutFileName.c_str(); + file->OpenFile(kOutFileName.c_str(), false); // Write mode. + ASSERT_TRUE(file->is_open()) << "File could not be opened:\n" + << kOutFileName.c_str(); const size_t kBufferLength = 3; std::unique_ptr written_buffer(new double[kBufferLength]); @@ -499,12 +469,9 @@ TEST_F(TransientFileUtilsTest, MAYBE_WriteDoubleBufferToFile) { file->CloseFile(); - file->OpenFile(kOutFileName.c_str(), - true, // Read only. - false, // No loop. - false); // No text. - ASSERT_TRUE(file->Open()) << "File could not be opened:\n" - << kOutFileName.c_str(); + file->OpenFile(kOutFileName.c_str(), true); // Read only. + ASSERT_TRUE(file->is_open()) << "File could not be opened:\n" + << kOutFileName.c_str(); EXPECT_EQ(kBufferLength, ReadDoubleBufferFromFile(file.get(), kBufferLength, @@ -541,12 +508,9 @@ TEST_F(TransientFileUtilsTest, MAYBE_ExpectedErrorReturnValues) { EXPECT_EQ(0u, WriteInt16BufferToFile(file.get(), 1, int16_buffer.get())); EXPECT_EQ(0u, WriteDoubleBufferToFile(file.get(), 1, double_buffer.get())); - file->OpenFile(test_filename.c_str(), - true, // Read only. - true, // Loop. - false); // No text. - ASSERT_TRUE(file->Open()) << "File could not be opened:\n" - << kTestFileName.c_str(); + file->OpenFile(test_filename.c_str(), true); // Read only. + ASSERT_TRUE(file->is_open()) << "File could not be opened:\n" + << kTestFileName.c_str(); EXPECT_EQ(0u, ReadInt16BufferFromFile(NULL, 1, int16_buffer.get())); EXPECT_EQ(0u, ReadInt16BufferFromFile(file.get(), 1, NULL)); diff --git a/webrtc/modules/audio_processing/transient/transient_detector_unittest.cc b/webrtc/modules/audio_processing/transient/transient_detector_unittest.cc index 18bb42e0eb..fedddb7d8f 100644 --- a/webrtc/modules/audio_processing/transient/transient_detector_unittest.cc +++ b/webrtc/modules/audio_processing/transient/transient_detector_unittest.cc @@ -53,11 +53,9 @@ TEST(TransientDetectorTest, CorrectnessBasedOnFiles) { detect_file->OpenFile( test::ResourcePath(detect_file_name.str(), "dat").c_str(), - true, // Read only. - false, // No loop. - false); // No text. + true); // Read only. - bool file_opened = detect_file->Open(); + bool file_opened = detect_file->is_open(); ASSERT_TRUE(file_opened) << "File could not be opened.\n" << detect_file_name.str().c_str(); @@ -70,9 +68,7 @@ TEST(TransientDetectorTest, CorrectnessBasedOnFiles) { audio_file->OpenFile( test::ResourcePath(audio_file_name.str(), "pcm").c_str(), - true, // Read only. - false, // No loop. - false); // No text. + true); // Read only. // Create detector. TransientDetector detector(sample_rate_hz); diff --git a/webrtc/modules/audio_processing/transient/wpd_tree_unittest.cc b/webrtc/modules/audio_processing/transient/wpd_tree_unittest.cc index 3e202bf088..3b08314d3b 100644 --- a/webrtc/modules/audio_processing/transient/wpd_tree_unittest.cc +++ b/webrtc/modules/audio_processing/transient/wpd_tree_unittest.cc @@ -95,12 +95,9 @@ TEST(WPDTreeTest, CorrectnessBasedOnMatlabFiles) { std::ostringstream matlab_stream; matlab_stream << "audio_processing/transient/wpd" << i; std::string matlab_string = test::ResourcePath(matlab_stream.str(), "dat"); - matlab_files_data[i]->OpenFile(matlab_string.c_str(), - true, // Read only. - false, // No loop. - false); // No text. + matlab_files_data[i]->OpenFile(matlab_string.c_str(), true); // Read only. - bool file_opened = matlab_files_data[i]->Open(); + bool file_opened = matlab_files_data[i]->is_open(); ASSERT_TRUE(file_opened) << "File could not be opened.\n" << matlab_string; // Out files. @@ -110,12 +107,9 @@ TEST(WPDTreeTest, CorrectnessBasedOnMatlabFiles) { out_stream << test::OutputPath() << "wpd_" << i << ".out"; std::string out_string = out_stream.str(); - out_files_data[i]->OpenFile(out_string.c_str(), - false, // Write mode. - false, // No loop. - false); // No text. + out_files_data[i]->OpenFile(out_string.c_str(), false); // Write mode. - file_opened = out_files_data[i]->Open(); + file_opened = out_files_data[i]->is_open(); ASSERT_TRUE(file_opened) << "File could not be opened.\n" << out_string; } @@ -125,12 +119,9 @@ TEST(WPDTreeTest, CorrectnessBasedOnMatlabFiles) { std::unique_ptr test_file(FileWrapper::Create()); - test_file->OpenFile(test_file_name.c_str(), - true, // Read only. - false, // No loop. - false); // No text. + test_file->OpenFile(test_file_name.c_str(), true); // Read only. - bool file_opened = test_file->Open(); + bool file_opened = test_file->is_open(); ASSERT_TRUE(file_opened) << "File could not be opened.\n" << test_file_name; float test_buffer[kTestBufferSize]; diff --git a/webrtc/modules/media_file/media_file_impl.cc b/webrtc/modules/media_file/media_file_impl.cc index 27fe9613a3..9f9511d837 100644 --- a/webrtc/modules/media_file/media_file_impl.cc +++ b/webrtc/modules/media_file/media_file_impl.cc @@ -371,12 +371,11 @@ int32_t MediaFileImpl::StartPlayingAudioFile( return -1; } - if(inputStream->OpenFile(fileName, true, loop) != 0) - { - delete inputStream; - WEBRTC_TRACE(kTraceError, kTraceFile, _id, - "Could not open input file %s", fileName); - return -1; + if (!inputStream->OpenFile(fileName, true)) { + delete inputStream; + WEBRTC_TRACE(kTraceError, kTraceFile, _id, "Could not open input file %s", + fileName); + return -1; } if(StartPlayingStream(*inputStream, loop, notificationTimeMs, @@ -748,13 +747,11 @@ int32_t MediaFileImpl::StartRecordingAudioFile( return -1; } - if(outputStream->OpenFile(fileName, false) != 0) - { - delete outputStream; - WEBRTC_TRACE(kTraceError, kTraceFile, _id, - "Could not open output file '%s' for writing!", - fileName); - return -1; + if (!outputStream->OpenFile(fileName, false)) { + delete outputStream; + WEBRTC_TRACE(kTraceError, kTraceFile, _id, + "Could not open output file '%s' for writing!", fileName); + return -1; } if(maxSizeBytes) diff --git a/webrtc/modules/media_file/media_file_utility.cc b/webrtc/modules/media_file/media_file_utility.cc index 8a815cc25d..8e7c8f6a02 100644 --- a/webrtc/modules/media_file/media_file_utility.cc +++ b/webrtc/modules/media_file/media_file_utility.cc @@ -1456,12 +1456,11 @@ int32_t ModuleFileUtility::FileDurationMs(const char* fileName, "failed to create InStream object!"); return -1; } - if(inStreamObj->OpenFile(fileName, true) == -1) - { - delete inStreamObj; - WEBRTC_TRACE(kTraceError, kTraceFile, _id, - "failed to open file %s!", fileName); - return -1; + if (!inStreamObj->OpenFile(fileName, true)) { + delete inStreamObj; + WEBRTC_TRACE(kTraceError, kTraceFile, _id, "failed to open file %s!", + fileName); + return -1; } switch (fileFormat) diff --git a/webrtc/modules/video_coding/utility/ivf_file_writer.cc b/webrtc/modules/video_coding/utility/ivf_file_writer.cc index 97f1da30e4..a80cf9bf6b 100644 --- a/webrtc/modules/video_coding/utility/ivf_file_writer.cc +++ b/webrtc/modules/video_coding/utility/ivf_file_writer.cc @@ -38,7 +38,7 @@ std::unique_ptr IvfFileWriter::Open(const std::string& file_name, VideoCodecType codec_type) { std::unique_ptr file_writer; std::unique_ptr file(FileWrapper::Create()); - if (file->OpenFile(file_name.c_str(), false) != 0) + if (!file->OpenFile(file_name.c_str(), false)) return file_writer; file_writer.reset(new IvfFileWriter( @@ -139,7 +139,7 @@ bool IvfFileWriter::InitFromFirstFrame(const EncodedImage& encoded_image) { } bool IvfFileWriter::WriteFrame(const EncodedImage& encoded_image) { - RTC_DCHECK(file_->Open()); + RTC_DCHECK(file_->is_open()); if (num_frames_ == 0 && !InitFromFirstFrame(encoded_image)) return false; @@ -178,7 +178,7 @@ bool IvfFileWriter::WriteFrame(const EncodedImage& encoded_image) { } bool IvfFileWriter::Close() { - if (!file_->Open()) + if (!file_->is_open()) return false; if (num_frames_ == 0) { @@ -190,7 +190,9 @@ bool IvfFileWriter::Close() { return true; } - return WriteHeader() && (file_->CloseFile() == 0); + bool ret = WriteHeader(); + file_->CloseFile(); + return ret; } } // namespace webrtc diff --git a/webrtc/modules/video_coding/utility/ivf_file_writer_unittest.cc b/webrtc/modules/video_coding/utility/ivf_file_writer_unittest.cc index 303502487d..ae1283b78d 100644 --- a/webrtc/modules/video_coding/utility/ivf_file_writer_unittest.cc +++ b/webrtc/modules/video_coding/utility/ivf_file_writer_unittest.cc @@ -69,6 +69,7 @@ class IvfFileWriterTest : public ::testing::Test { int height, uint32_t num_frames, bool use_capture_tims_ms) { + ASSERT_TRUE(file->is_open()); uint8_t data[kHeaderSize]; ASSERT_EQ(kHeaderSize, file->Read(data, kHeaderSize)); @@ -118,13 +119,12 @@ class IvfFileWriterTest : public ::testing::Test { EXPECT_TRUE(file_writer_->Close()); std::unique_ptr out_file(FileWrapper::Create()); - ASSERT_EQ(0, out_file->OpenFile(file_name_.c_str(), true)); + ASSERT_TRUE(out_file->OpenFile(file_name_.c_str(), true)); VerifyIvfHeader(out_file.get(), fourcc, kWidth, kHeight, kNumFrames, use_capture_tims_ms); VerifyDummyTestFrames(out_file.get(), kNumFrames); - out_file->Flush(); - EXPECT_EQ(0, out_file->CloseFile()); + out_file->CloseFile(); bool file_removed = false; for (int i = 0; i < kMaxFileRetries; ++i) { @@ -151,7 +151,7 @@ class IvfFileWriterTest : public ::testing::Test { if (file_wrapper.get() != nullptr) rtc::Thread::SleepMs(1000); file_wrapper.reset(FileWrapper::Create()); - file_exists = file_wrapper->OpenFile(file_name_.c_str(), true) == 0; + file_exists = file_wrapper->OpenFile(file_name_.c_str(), true); file_wrapper->CloseFile(); } while (file_exists != expected && ++iterations < kMaxFileRetries); return file_exists; diff --git a/webrtc/system_wrappers/BUILD.gn b/webrtc/system_wrappers/BUILD.gn index e89f733709..9464c6b6e1 100644 --- a/webrtc/system_wrappers/BUILD.gn +++ b/webrtc/system_wrappers/BUILD.gn @@ -56,7 +56,6 @@ source_set("system_wrappers") { "source/event_timer_win.cc", "source/event_timer_win.h", "source/file_impl.cc", - "source/file_impl.h", "source/logging.cc", "source/rtp_to_ntp.cc", "source/rw_lock.cc", diff --git a/webrtc/system_wrappers/include/file_wrapper.h b/webrtc/system_wrappers/include/file_wrapper.h index b32a62f2f9..a1d899c783 100644 --- a/webrtc/system_wrappers/include/file_wrapper.h +++ b/webrtc/system_wrappers/include/file_wrapper.h @@ -14,6 +14,7 @@ #include #include +#include "webrtc/base/criticalsection.h" #include "webrtc/common_types.h" #include "webrtc/typedefs.h" @@ -22,55 +23,64 @@ namespace webrtc { +// TODO(tommi): Remove the base classes, rename to rtc::File and move to base. class FileWrapper : public InStream, public OutStream { public: static const size_t kMaxFileNameSize = 1024; - // Factory method. Constructor disabled. + // Factory methods. + // TODO(tommi): Remove Create(). static FileWrapper* Create(); + static FileWrapper Open(const char* file_name_utf8, bool read_only); + + FileWrapper(FILE* file, size_t max_size); + ~FileWrapper() override; + + // Support for move semantics. + FileWrapper(FileWrapper&& other); + FileWrapper& operator=(FileWrapper&& other); // Returns true if a file has been opened. - virtual bool Open() const = 0; + bool is_open() const { return file_ != nullptr; } // Opens a file in read or write mode, decided by the read_only parameter. - virtual int OpenFile(const char* file_name_utf8, - bool read_only, - bool loop = false, - bool text = false) = 0; + bool OpenFile(const char* file_name_utf8, bool read_only); - // Initializes the wrapper from an existing handle. |read_only| must match in - // the mode the file was opened in. If |manage_file| is true, the wrapper + // Initializes the wrapper from an existing handle. The wrapper // takes ownership of |handle| and closes it in CloseFile(). - virtual int OpenFromFileHandle(FILE* handle, - bool manage_file, - bool read_only, - bool loop = false) = 0; + bool OpenFromFileHandle(FILE* handle); - virtual int CloseFile() = 0; + void CloseFile(); // Limits the file size to |bytes|. Writing will fail after the cap // is hit. Pass zero to use an unlimited size. - virtual int SetMaxFileSize(size_t bytes) = 0; + // TODO(tommi): Could we move this out into a separate class? + void SetMaxFileSize(size_t bytes); - // Flush any pending writes. - virtual int Flush() = 0; + // Flush any pending writes. Note: Flushing when closing, is not required. + int Flush(); - // Returns the opened file's name in |file_name_utf8|. Provide the size of - // the buffer in bytes in |size|. The name will be truncated if |size| is - // too small. - virtual int FileName(char* file_name_utf8, - size_t size) const = 0; - - // Write |format| to the opened file. Arguments are taken in the same manner - // as printf. That is, supply a format string containing text and - // specifiers. Returns the number of characters written or -1 on error. - virtual int WriteText(const char* format, ...) = 0; - - // Inherited from both Instream and OutStream. - // Rewinds the file to the start. Only available when OpenFile() has been - // called with |loop| == true or |readOnly| == true. - // virtual int Rewind() = 0; + // Rewinds the file to the start. int Rewind() override; + int Read(void* buf, size_t length) override; + bool Write(const void* buf, size_t length) override; + + private: + FileWrapper(); + + void CloseFileImpl(); + int FlushImpl(); + + // TODO(tommi): Remove the lock. + rtc::CriticalSection lock_; + + FILE* file_ = nullptr; + size_t position_ = 0; + size_t max_size_in_bytes_ = 0; + + // Copying is not supported. + FileWrapper(const FileWrapper&) = delete; + FileWrapper& operator=(const FileWrapper&) = delete; }; } // namespace webrtc diff --git a/webrtc/system_wrappers/include/trace.h b/webrtc/system_wrappers/include/trace.h index 25a3d746c4..e3ca3c6840 100644 --- a/webrtc/system_wrappers/include/trace.h +++ b/webrtc/system_wrappers/include/trace.h @@ -60,9 +60,6 @@ class Trace { static int32_t SetTraceFile(const char* file_name, const bool add_file_counter = false); - // Returns the name of the file that the trace is currently writing to. - static int32_t TraceFile(char file_name[1024]); - // Registers callback to receive trace messages. // TODO(hellner): Why not use OutStream instead? Why is TraceCallback not // defined in this file? diff --git a/webrtc/system_wrappers/source/data_log.cc b/webrtc/system_wrappers/source/data_log.cc index 778769603b..6d140a7d1c 100644 --- a/webrtc/system_wrappers/source/data_log.cc +++ b/webrtc/system_wrappers/source/data_log.cc @@ -205,16 +205,10 @@ int LogTable::InsertCell(const std::string& column_name, int LogTable::CreateLogFile(const std::string& file_name) { if (file_name.length() == 0) return -1; - if (file_->Open()) + if (file_->is_open()) return -1; - file_->OpenFile(file_name.c_str(), - false, // Open with read/write permissions - false, // Don't wraparound and write at the beginning when - // the file is full - true); // Open as a text file - if (file_ == NULL) - return -1; - return 0; + // Open with read/write permissions + return file_->OpenFile(file_name.c_str(), false) ? 0 : -1; } void LogTable::Flush() { diff --git a/webrtc/system_wrappers/source/file_impl.cc b/webrtc/system_wrappers/source/file_impl.cc index e033c0eb73..8e1ba24f8d 100644 --- a/webrtc/system_wrappers/source/file_impl.cc +++ b/webrtc/system_wrappers/source/file_impl.cc @@ -8,9 +8,7 @@ * be found in the AUTHORS file in the root of the source tree. */ -#include "webrtc/system_wrappers/source/file_impl.h" - -#include +#include "webrtc/system_wrappers/include/file_wrapper.h" #ifdef _WIN32 #include @@ -20,258 +18,135 @@ #endif #include "webrtc/base/checks.h" -#include "webrtc/system_wrappers/include/rw_lock_wrapper.h" namespace webrtc { - -FileWrapper* FileWrapper::Create() { - return new FileWrapperImpl(); -} - -FileWrapperImpl::FileWrapperImpl() - : rw_lock_(RWLockWrapper::CreateRWLock()), - id_(NULL), - managed_file_handle_(true), - open_(false), - looping_(false), - read_only_(false), - max_size_in_bytes_(0), - size_in_bytes_(0) { - memset(file_name_utf8_, 0, kMaxFileNameSize); -} - -FileWrapperImpl::~FileWrapperImpl() { - if (id_ != NULL && managed_file_handle_) { - fclose(id_); - } -} - -int FileWrapperImpl::CloseFile() { - WriteLockScoped write(*rw_lock_); - return CloseFileImpl(); -} - -int FileWrapperImpl::Rewind() { - WriteLockScoped write(*rw_lock_); - if (looping_ || !read_only_) { - if (id_ != NULL) { - size_in_bytes_ = 0; - return fseek(id_, 0, SEEK_SET); - } - } - return -1; -} - -int FileWrapperImpl::SetMaxFileSize(size_t bytes) { - WriteLockScoped write(*rw_lock_); - max_size_in_bytes_ = bytes; - return 0; -} - -int FileWrapperImpl::Flush() { - WriteLockScoped write(*rw_lock_); - return FlushImpl(); -} - -int FileWrapperImpl::FileName(char* file_name_utf8, size_t size) const { - ReadLockScoped read(*rw_lock_); - size_t length = strlen(file_name_utf8_); - if (length > kMaxFileNameSize) { - assert(false); - return -1; - } - if (length < 1) { - return -1; - } - - // Make sure to NULL terminate - if (size < length) { - length = size - 1; - } - memcpy(file_name_utf8, file_name_utf8_, length); - file_name_utf8[length] = 0; - return 0; -} - -bool FileWrapperImpl::Open() const { - ReadLockScoped read(*rw_lock_); - return open_; -} - -int FileWrapperImpl::OpenFile(const char* file_name_utf8, bool read_only, - bool loop, bool text) { - WriteLockScoped write(*rw_lock_); - if (id_ != NULL && !managed_file_handle_) - return -1; - size_t length = strlen(file_name_utf8); - if (length > kMaxFileNameSize - 1) { - return -1; - } - - read_only_ = read_only; - - FILE* tmp_id = NULL; -#if defined _WIN32 - wchar_t wide_file_name[kMaxFileNameSize]; - wide_file_name[0] = 0; - - MultiByteToWideChar(CP_UTF8, - 0, // UTF8 flag - file_name_utf8, - -1, // Null terminated string - wide_file_name, - kMaxFileNameSize); - if (text) { - if (read_only) { - tmp_id = _wfopen(wide_file_name, L"rt"); - } else { - tmp_id = _wfopen(wide_file_name, L"wt"); - } - } else { - if (read_only) { - tmp_id = _wfopen(wide_file_name, L"rb"); - } else { - tmp_id = _wfopen(wide_file_name, L"wb"); - } - } +namespace { +FILE* FileOpen(const char* file_name_utf8, bool read_only) { +#if defined(_WIN32) + int len = MultiByteToWideChar(CP_UTF8, 0, file_name_utf8, -1, nullptr, 0); + std::wstring wstr(len, 0); + MultiByteToWideChar(CP_UTF8, 0, file_name_utf8, -1, &wstr[0], len); + FILE* file = _wfopen(wstr.c_str(), read_only ? L"rb" : L"wb"); #else - if (text) { - if (read_only) { - tmp_id = fopen(file_name_utf8, "rt"); - } else { - tmp_id = fopen(file_name_utf8, "wt"); - } - } else { - if (read_only) { - tmp_id = fopen(file_name_utf8, "rb"); - } else { - tmp_id = fopen(file_name_utf8, "wb"); - } - } + FILE* file = fopen(file_name_utf8, read_only ? "rb" : "wb"); #endif + return file; +} +} // namespace - if (tmp_id != NULL) { - // +1 comes from copying the NULL termination character. - memcpy(file_name_utf8_, file_name_utf8, length + 1); - if (id_ != NULL) { - fclose(id_); - } - id_ = tmp_id; - managed_file_handle_ = true; - looping_ = loop; - open_ = true; - return 0; - } - return -1; +// static +FileWrapper* FileWrapper::Create() { + return new FileWrapper(); } -int FileWrapperImpl::OpenFromFileHandle(FILE* handle, - bool manage_file, - bool read_only, - bool loop) { - WriteLockScoped write(*rw_lock_); - if (!handle) - return -1; - - if (id_ != NULL) { - if (managed_file_handle_) - fclose(id_); - else - return -1; - } - - id_ = handle; - managed_file_handle_ = manage_file; - read_only_ = read_only; - looping_ = loop; - open_ = true; - return 0; +// static +FileWrapper FileWrapper::Open(const char* file_name_utf8, bool read_only) { + return FileWrapper(FileOpen(file_name_utf8, read_only), 0); } -int FileWrapperImpl::Read(void* buf, size_t length) { - WriteLockScoped write(*rw_lock_); - if (id_ == NULL) - return -1; +FileWrapper::FileWrapper() {} - size_t bytes_read = fread(buf, 1, length, id_); - if (bytes_read != length && !looping_) { - CloseFileImpl(); - } - return static_cast(bytes_read); +FileWrapper::FileWrapper(FILE* file, size_t max_size) + : file_(file), max_size_in_bytes_(max_size) {} + +FileWrapper::~FileWrapper() { + CloseFileImpl(); } -int FileWrapperImpl::WriteText(const char* format, ...) { - WriteLockScoped write(*rw_lock_); - if (format == NULL) - return -1; - - if (read_only_) - return -1; - - if (id_ == NULL) - return -1; - - va_list args; - va_start(args, format); - int num_chars = vfprintf(id_, format, args); - va_end(args); - - if (num_chars >= 0) { - return num_chars; - } else { - CloseFileImpl(); - return -1; - } +FileWrapper::FileWrapper(FileWrapper&& other) { + operator=(std::move(other)); } -bool FileWrapperImpl::Write(const void* buf, size_t length) { - WriteLockScoped write(*rw_lock_); - if (buf == NULL) - return false; - - if (read_only_) - return false; - - if (id_ == NULL) - return false; - - // Check if it's time to stop writing. - if (max_size_in_bytes_ > 0 && - (size_in_bytes_ + length) > max_size_in_bytes_) { - FlushImpl(); - return false; - } - - size_t num_bytes = fwrite(buf, 1, length, id_); - size_in_bytes_ += num_bytes; - if (num_bytes != length) { - CloseFileImpl(); - return false; - } - return true; +FileWrapper& FileWrapper::operator=(FileWrapper&& other) { + file_ = other.file_; + max_size_in_bytes_ = other.max_size_in_bytes_; + position_ = other.position_; + other.file_ = nullptr; + return *this; } -int FileWrapperImpl::CloseFileImpl() { - if (id_ != NULL) { - if (managed_file_handle_) - fclose(id_); - id_ = NULL; - } - memset(file_name_utf8_, 0, kMaxFileNameSize); - open_ = false; - return 0; -} - -int FileWrapperImpl::FlushImpl() { - if (id_ != NULL) { - return fflush(id_); - } - return -1; +void FileWrapper::CloseFile() { + rtc::CritScope lock(&lock_); + CloseFileImpl(); } int FileWrapper::Rewind() { - RTC_DCHECK(false); + rtc::CritScope lock(&lock_); + if (file_ != nullptr) { + position_ = 0; + return fseek(file_, 0, SEEK_SET); + } return -1; } +void FileWrapper::SetMaxFileSize(size_t bytes) { + rtc::CritScope lock(&lock_); + max_size_in_bytes_ = bytes; +} + +int FileWrapper::Flush() { + rtc::CritScope lock(&lock_); + return FlushImpl(); +} + +bool FileWrapper::OpenFile(const char* file_name_utf8, bool read_only) { + size_t length = strlen(file_name_utf8); + if (length > kMaxFileNameSize - 1) + return false; + + rtc::CritScope lock(&lock_); + if (file_ != nullptr) + return false; + + file_ = FileOpen(file_name_utf8, read_only); + return file_ != nullptr; +} + +bool FileWrapper::OpenFromFileHandle(FILE* handle) { + if (!handle) + return false; + rtc::CritScope lock(&lock_); + CloseFileImpl(); + file_ = handle; + return true; +} + +int FileWrapper::Read(void* buf, size_t length) { + rtc::CritScope lock(&lock_); + if (file_ == nullptr) + return -1; + + size_t bytes_read = fread(buf, 1, length, file_); + return static_cast(bytes_read); +} + +bool FileWrapper::Write(const void* buf, size_t length) { + if (buf == nullptr) + return false; + + rtc::CritScope lock(&lock_); + + if (file_ == nullptr) + return false; + + // Check if it's time to stop writing. + if (max_size_in_bytes_ > 0 && (position_ + length) > max_size_in_bytes_) + return false; + + size_t num_bytes = fwrite(buf, 1, length, file_); + position_ += num_bytes; + + return num_bytes == length; +} + +void FileWrapper::CloseFileImpl() { + if (file_ != nullptr) + fclose(file_); + file_ = nullptr; +} + +int FileWrapper::FlushImpl() { + return (file_ != nullptr) ? fflush(file_) : -1; +} + } // namespace webrtc diff --git a/webrtc/system_wrappers/source/file_impl.h b/webrtc/system_wrappers/source/file_impl.h deleted file mode 100644 index 51103d648b..0000000000 --- a/webrtc/system_wrappers/source/file_impl.h +++ /dev/null @@ -1,70 +0,0 @@ -/* - * Copyright (c) 2011 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 WEBRTC_SYSTEM_WRAPPERS_SOURCE_FILE_IMPL_H_ -#define WEBRTC_SYSTEM_WRAPPERS_SOURCE_FILE_IMPL_H_ - -#include - -#include - -#include "webrtc/system_wrappers/include/file_wrapper.h" - -namespace webrtc { - -class RWLockWrapper; - -class FileWrapperImpl : public FileWrapper { - public: - FileWrapperImpl(); - ~FileWrapperImpl() override; - - int FileName(char* file_name_utf8, size_t size) const override; - - bool Open() const override; - - int OpenFile(const char* file_name_utf8, - bool read_only, - bool loop = false, - bool text = false) override; - - int OpenFromFileHandle(FILE* handle, - bool manage_file, - bool read_only, - bool loop = false) override; - - int CloseFile() override; - int SetMaxFileSize(size_t bytes) override; - int Flush() override; - - int Read(void* buf, size_t length) override; - bool Write(const void* buf, size_t length) override; - int WriteText(const char* format, ...) override; - int Rewind() override; - - private: - int CloseFileImpl(); - int FlushImpl(); - - std::unique_ptr rw_lock_; - - FILE* id_; - bool managed_file_handle_; - bool open_; - bool looping_; - bool read_only_; - size_t max_size_in_bytes_; // -1 indicates file size limitation is off - size_t size_in_bytes_; - char file_name_utf8_[kMaxFileNameSize]; -}; - -} // namespace webrtc - -#endif // WEBRTC_SYSTEM_WRAPPERS_SOURCE_FILE_IMPL_H_ diff --git a/webrtc/system_wrappers/source/trace_impl.cc b/webrtc/system_wrappers/source/trace_impl.cc index 5029f5ab6e..30a8816b75 100644 --- a/webrtc/system_wrappers/source/trace_impl.cc +++ b/webrtc/system_wrappers/source/trace_impl.cc @@ -72,7 +72,6 @@ TraceImpl::TraceImpl() } TraceImpl::~TraceImpl() { - trace_file_->Flush(); trace_file_->CloseFile(); } @@ -287,8 +286,8 @@ int32_t TraceImpl::SetTraceFileImpl(const char* file_name_utf8, const bool add_file_counter) { rtc::CritScope lock(&crit_); - trace_file_->Flush(); trace_file_->CloseFile(); + trace_file_path_.clear(); if (file_name_utf8) { if (add_file_counter) { @@ -297,27 +296,22 @@ int32_t TraceImpl::SetTraceFileImpl(const char* file_name_utf8, char file_name_with_counter_utf8[FileWrapper::kMaxFileNameSize]; CreateFileName(file_name_utf8, file_name_with_counter_utf8, file_count_text_); - if (trace_file_->OpenFile(file_name_with_counter_utf8, false, false, - true) == -1) { + if (!trace_file_->OpenFile(file_name_with_counter_utf8, false)) { return -1; } + trace_file_path_ = file_name_with_counter_utf8; } else { file_count_text_ = 0; - if (trace_file_->OpenFile(file_name_utf8, false, false, true) == -1) { + if (!trace_file_->OpenFile(file_name_utf8, false)) { return -1; } + trace_file_path_ = file_name_utf8; } } row_count_text_ = 0; return 0; } -int32_t TraceImpl::TraceFileImpl( - char file_name_utf8[FileWrapper::kMaxFileNameSize]) { - rtc::CritScope lock(&crit_); - return trace_file_->FileName(file_name_utf8, FileWrapper::kMaxFileNameSize); -} - int32_t TraceImpl::SetTraceCallbackImpl(TraceCallback* callback) { rtc::CritScope lock(&crit_); callback_ = callback; @@ -366,7 +360,7 @@ void TraceImpl::AddMessageToList( } void TraceImpl::WriteToFile(const char* msg, uint16_t length) { - if (!trace_file_->Open()) + if (!trace_file_->is_open()) return; if (row_count_text_ > WEBRTC_TRACE_MAX_FILE_SIZE) { @@ -377,20 +371,19 @@ void TraceImpl::WriteToFile(const char* msg, uint16_t length) { if (file_count_text_ == 0) { trace_file_->Rewind(); } else { - char old_file_name[FileWrapper::kMaxFileNameSize]; char new_file_name[FileWrapper::kMaxFileNameSize]; // get current name - trace_file_->FileName(old_file_name, FileWrapper::kMaxFileNameSize); - trace_file_->CloseFile(); - file_count_text_++; + UpdateFileName(new_file_name, file_count_text_); - UpdateFileName(old_file_name, new_file_name, file_count_text_); + trace_file_->CloseFile(); + trace_file_path_.clear(); - if (trace_file_->OpenFile(new_file_name, false, false, true) == -1) { + if (!trace_file_->OpenFile(new_file_name, false)) { return; } + trace_file_path_ = new_file_name; } } if (row_count_text_ == 0) { @@ -462,17 +455,13 @@ bool TraceImpl::TraceCheck(const TraceLevel level) const { } bool TraceImpl::UpdateFileName( - const char file_name_utf8[FileWrapper::kMaxFileNameSize], char file_name_with_counter_utf8[FileWrapper::kMaxFileNameSize], const uint32_t new_count) const { - int32_t length = (int32_t)strlen(file_name_utf8); - if (length < 0) { - return false; - } + int32_t length = static_cast(trace_file_path_.length()); int32_t length_without_file_ending = length - 1; while (length_without_file_ending > 0) { - if (file_name_utf8[length_without_file_ending] == '.') { + if (trace_file_path_[length_without_file_ending] == '.') { break; } else { length_without_file_ending--; @@ -483,17 +472,17 @@ bool TraceImpl::UpdateFileName( } int32_t length_to_ = length_without_file_ending - 1; while (length_to_ > 0) { - if (file_name_utf8[length_to_] == '_') { + if (trace_file_path_[length_to_] == '_') { break; } else { length_to_--; } } - memcpy(file_name_with_counter_utf8, file_name_utf8, length_to_); + memcpy(file_name_with_counter_utf8, &trace_file_path_[0], length_to_); sprintf(file_name_with_counter_utf8 + length_to_, "_%lu%s", static_cast(new_count), - file_name_utf8 + length_without_file_ending); + &trace_file_path_[length_without_file_ending]); return true; } @@ -535,17 +524,6 @@ void Trace::ReturnTrace() { TraceImpl::StaticInstance(kRelease); } -// static -int32_t Trace::TraceFile(char file_name[FileWrapper::kMaxFileNameSize]) { - TraceImpl* trace = TraceImpl::GetTrace(); - if (trace) { - int ret_val = trace->TraceFileImpl(file_name); - ReturnTrace(); - return ret_val; - } - return -1; -} - // static void Trace::set_level_filter(int filter) { rtc::AtomicOps::ReleaseStore(&level_filter_, filter); diff --git a/webrtc/system_wrappers/source/trace_impl.h b/webrtc/system_wrappers/source/trace_impl.h index 182f5809a5..045b3cd5f5 100644 --- a/webrtc/system_wrappers/source/trace_impl.h +++ b/webrtc/system_wrappers/source/trace_impl.h @@ -41,8 +41,6 @@ class TraceImpl : public Trace { static TraceImpl* GetTrace(const TraceLevel level = kTraceAll); int32_t SetTraceFileImpl(const char* file_name, const bool add_file_counter); - int32_t TraceFileImpl(char file_name[FileWrapper::kMaxFileNameSize]); - int32_t SetTraceCallbackImpl(TraceCallback* callback); void AddImpl(const TraceLevel level, const TraceModule module, @@ -82,9 +80,8 @@ class TraceImpl : public Trace { const TraceLevel level); bool UpdateFileName( - const char file_name_utf8[FileWrapper::kMaxFileNameSize], - char file_name_with_counter_utf8[FileWrapper::kMaxFileNameSize], - const uint32_t new_count) const; + char file_name_with_counter_utf8[FileWrapper::kMaxFileNameSize], + const uint32_t new_count) const EXCLUSIVE_LOCKS_REQUIRED(crit_); bool CreateFileName( const char file_name_utf8[FileWrapper::kMaxFileNameSize], @@ -99,6 +96,7 @@ class TraceImpl : public Trace { uint32_t file_count_text_ GUARDED_BY(crit_); const std::unique_ptr trace_file_ GUARDED_BY(crit_); + std::string trace_file_path_ GUARDED_BY(crit_); rtc::CriticalSection crit_; }; diff --git a/webrtc/system_wrappers/system_wrappers.gyp b/webrtc/system_wrappers/system_wrappers.gyp index 91ebdd357d..ea8fdb6cff 100644 --- a/webrtc/system_wrappers/system_wrappers.gyp +++ b/webrtc/system_wrappers/system_wrappers.gyp @@ -58,7 +58,6 @@ 'source/event_timer_win.cc', 'source/event_timer_win.h', 'source/file_impl.cc', - 'source/file_impl.h', 'source/logging.cc', 'source/rtp_to_ntp.cc', 'source/rw_lock.cc', diff --git a/webrtc/test/fake_audio_device.cc b/webrtc/test/fake_audio_device.cc index 435c53f878..43c2933ba7 100644 --- a/webrtc/test/fake_audio_device.cc +++ b/webrtc/test/fake_audio_device.cc @@ -39,8 +39,7 @@ FakeAudioDevice::FakeAudioDevice(Clock* clock, memset(captured_audio_, 0, sizeof(captured_audio_)); memset(playout_buffer_, 0, sizeof(playout_buffer_)); // Open audio input file as read-only and looping. - EXPECT_EQ(0, input_stream_->OpenFile(filename.c_str(), true, true)) - << filename; + EXPECT_TRUE(input_stream_->OpenFile(filename.c_str(), true)) << filename; } FakeAudioDevice::~FakeAudioDevice() {