From 863b5361003e36451a946ca6f9886efb3c1bfa53 Mon Sep 17 00:00:00 2001 From: "henrikg@webrtc.org" Date: Fri, 6 Dec 2013 16:05:17 +0000 Subject: [PATCH] Allow opening an AEC dump from an existing file handle. This is necessary for Chromium to be able enable the dump with the sanbox enabled. It will open the file in the browser process and pass the handle to the render process. This changes FileWrapper to deal with the case were the file handle is not managed by the wrapper. BUG=2567 R=andrew@webrtc.org, henrika@webrtc.org, perkj@webrtc.org Review URL: https://webrtc-codereview.appspot.com/4649004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@5239 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../audio_processing/audio_processing_impl.cc | 29 +++++++++++++++ .../audio_processing/audio_processing_impl.h | 1 + .../include/audio_processing.h | 5 +++ .../test/audio_processing_unittest.cc | 36 ++++++++++++++++++- .../system_wrappers/interface/file_wrapper.h | 9 +++++ webrtc/system_wrappers/source/file_impl.cc | 35 +++++++++++++++--- webrtc/system_wrappers/source/file_impl.h | 6 ++++ .../include/voe_audio_processing.h | 6 ++++ .../voice_engine/voe_audio_processing_impl.cc | 11 ++++++ .../voice_engine/voe_audio_processing_impl.h | 1 + 10 files changed, 134 insertions(+), 5 deletions(-) diff --git a/webrtc/modules/audio_processing/audio_processing_impl.cc b/webrtc/modules/audio_processing/audio_processing_impl.cc index 2eb97bfd8..4d36ff7e7 100644 --- a/webrtc/modules/audio_processing/audio_processing_impl.cc +++ b/webrtc/modules/audio_processing/audio_processing_impl.cc @@ -540,6 +540,35 @@ int AudioProcessingImpl::StartDebugRecording( #endif // WEBRTC_AUDIOPROC_DEBUG_DUMP } +int AudioProcessingImpl::StartDebugRecording(FILE* handle) { + CriticalSectionScoped crit_scoped(crit_); + + if (handle == NULL) { + return kNullPointerError; + } + +#ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP + // Stop any ongoing recording. + if (debug_file_->Open()) { + if (debug_file_->CloseFile() == -1) { + return kFileError; + } + } + + if (debug_file_->OpenFromFileHandle(handle, true, false) == -1) { + return kFileError; + } + + int err = WriteInitMessage(); + if (err != kNoError) { + return err; + } + return kNoError; +#else + return kUnsupportedFunctionError; +#endif // WEBRTC_AUDIOPROC_DEBUG_DUMP +} + int AudioProcessingImpl::StopDebugRecording() { CriticalSectionScoped crit_scoped(crit_); diff --git a/webrtc/modules/audio_processing/audio_processing_impl.h b/webrtc/modules/audio_processing/audio_processing_impl.h index 6c228781d..e48a2c18a 100644 --- a/webrtc/modules/audio_processing/audio_processing_impl.h +++ b/webrtc/modules/audio_processing/audio_processing_impl.h @@ -79,6 +79,7 @@ class AudioProcessingImpl : public AudioProcessing { virtual int delay_offset_ms() const OVERRIDE; virtual int StartDebugRecording( const char filename[kMaxFilenameSize]) OVERRIDE; + virtual int StartDebugRecording(FILE* handle) OVERRIDE; virtual int StopDebugRecording() OVERRIDE; virtual EchoCancellation* echo_cancellation() const OVERRIDE; virtual EchoControlMobile* echo_control_mobile() const OVERRIDE; diff --git a/webrtc/modules/audio_processing/include/audio_processing.h b/webrtc/modules/audio_processing/include/audio_processing.h index 82976499d..b5c856de2 100644 --- a/webrtc/modules/audio_processing/include/audio_processing.h +++ b/webrtc/modules/audio_processing/include/audio_processing.h @@ -12,6 +12,7 @@ #define WEBRTC_MODULES_AUDIO_PROCESSING_INCLUDE_AUDIO_PROCESSING_H_ #include // size_t +#include // FILE #include "webrtc/common.h" #include "webrtc/modules/interface/module.h" @@ -234,6 +235,10 @@ class AudioProcessing : public Module { static const size_t kMaxFilenameSize = 1024; virtual int StartDebugRecording(const char filename[kMaxFilenameSize]) = 0; + // Same as above but uses an existing file handle. Takes ownership + // of |handle| and closes it at StopDebugRecording(). + virtual int StartDebugRecording(FILE* handle) = 0; + // Stops recording debugging information, and closes the file. Recording // cannot be resumed in the same file (without overwriting it). virtual int StopDebugRecording() = 0; diff --git a/webrtc/modules/audio_processing/test/audio_processing_unittest.cc b/webrtc/modules/audio_processing/test/audio_processing_unittest.cc index bb6bc2eb8..a69574e0b 100644 --- a/webrtc/modules/audio_processing/test/audio_processing_unittest.cc +++ b/webrtc/modules/audio_processing/test/audio_processing_unittest.cc @@ -1356,7 +1356,8 @@ TEST_F(ApmTest, SplittingFilter) { // TODO(andrew): expand test to verify output. TEST_F(ApmTest, DebugDump) { const std::string filename = webrtc::test::OutputPath() + "debug.aec"; - EXPECT_EQ(apm_->kNullPointerError, apm_->StartDebugRecording(NULL)); + EXPECT_EQ(apm_->kNullPointerError, + apm_->StartDebugRecording(static_cast(NULL))); #ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP // Stopping without having started should be OK. @@ -1384,6 +1385,39 @@ TEST_F(ApmTest, DebugDump) { #endif // WEBRTC_AUDIOPROC_DEBUG_DUMP } +// TODO(andrew): expand test to verify output. +TEST_F(ApmTest, DebugDumpFromFileHandle) { + FILE* fid = NULL; + EXPECT_EQ(apm_->kNullPointerError, apm_->StartDebugRecording(fid)); + const std::string filename = webrtc::test::OutputPath() + "debug.aec"; + fid = fopen(filename.c_str(), "w"); + ASSERT_TRUE(fid); + +#ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP + // Stopping without having started should be OK. + EXPECT_EQ(apm_->kNoError, apm_->StopDebugRecording()); + + EXPECT_EQ(apm_->kNoError, apm_->StartDebugRecording(fid)); + EXPECT_EQ(apm_->kNoError, apm_->AnalyzeReverseStream(revframe_)); + EXPECT_EQ(apm_->kNoError, apm_->ProcessStream(frame_)); + EXPECT_EQ(apm_->kNoError, apm_->StopDebugRecording()); + + // Verify the file has been written. + fid = fopen(filename.c_str(), "r"); + ASSERT_TRUE(fid != NULL); + + // Clean it up. + ASSERT_EQ(0, fclose(fid)); + ASSERT_EQ(0, remove(filename.c_str())); +#else + EXPECT_EQ(apm_->kUnsupportedFunctionError, + apm_->StartDebugRecording(fid)); + EXPECT_EQ(apm_->kUnsupportedFunctionError, apm_->StopDebugRecording()); + + ASSERT_EQ(0, fclose(fid)); +#endif // WEBRTC_AUDIOPROC_DEBUG_DUMP +} + // TODO(andrew): Add a test to process a few frames with different combinations // of enabled components. diff --git a/webrtc/system_wrappers/interface/file_wrapper.h b/webrtc/system_wrappers/interface/file_wrapper.h index 5de6774ad..68dc00501 100644 --- a/webrtc/system_wrappers/interface/file_wrapper.h +++ b/webrtc/system_wrappers/interface/file_wrapper.h @@ -12,6 +12,7 @@ #define WEBRTC_SYSTEM_WRAPPERS_INTERFACE_FILE_WRAPPER_H_ #include +#include #include "webrtc/common_types.h" #include "webrtc/typedefs.h" @@ -37,6 +38,14 @@ class FileWrapper : public InStream, public OutStream { bool loop = false, bool text = false) = 0; + // 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 + // takes ownership of |handle| and closes it in CloseFile(). + virtual int OpenFromFileHandle(FILE* handle, + bool manage_file, + bool read_only, + bool loop = false) = 0; + virtual int CloseFile() = 0; // Limits the file size to |bytes|. Writing will fail after the cap diff --git a/webrtc/system_wrappers/source/file_impl.cc b/webrtc/system_wrappers/source/file_impl.cc index 190f39955..8b21b96eb 100644 --- a/webrtc/system_wrappers/source/file_impl.cc +++ b/webrtc/system_wrappers/source/file_impl.cc @@ -30,6 +30,7 @@ FileWrapper* FileWrapper::Create() { FileWrapperImpl::FileWrapperImpl() : rw_lock_(RWLockWrapper::CreateRWLock()), id_(NULL), + managed_file_handle_(true), open_(false), looping_(false), read_only_(false), @@ -39,7 +40,7 @@ FileWrapperImpl::FileWrapperImpl() } FileWrapperImpl::~FileWrapperImpl() { - if (id_ != NULL) { + if (id_ != NULL && managed_file_handle_) { fclose(id_); } } @@ -71,8 +72,7 @@ int FileWrapperImpl::Flush() { return FlushImpl(); } -int FileWrapperImpl::FileName(char* file_name_utf8, - size_t size) const { +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) { @@ -100,6 +100,8 @@ bool FileWrapperImpl::Open() const { 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; @@ -154,6 +156,7 @@ int FileWrapperImpl::OpenFile(const char* file_name_utf8, bool read_only, fclose(id_); } id_ = tmp_id; + managed_file_handle_ = true; looping_ = loop; open_ = true; return 0; @@ -161,6 +164,29 @@ int FileWrapperImpl::OpenFile(const char* file_name_utf8, bool read_only, return -1; } +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; +} + int FileWrapperImpl::Read(void* buf, int length) { WriteLockScoped write(*rw_lock_); if (length < 0) @@ -233,7 +259,8 @@ bool FileWrapperImpl::Write(const void* buf, int length) { int FileWrapperImpl::CloseFileImpl() { if (id_ != NULL) { - fclose(id_); + if (managed_file_handle_) + fclose(id_); id_ = NULL; } memset(file_name_utf8_, 0, kMaxFileNameSize); diff --git a/webrtc/system_wrappers/source/file_impl.h b/webrtc/system_wrappers/source/file_impl.h index 8cc0979c6..1abf01019 100644 --- a/webrtc/system_wrappers/source/file_impl.h +++ b/webrtc/system_wrappers/source/file_impl.h @@ -35,6 +35,11 @@ class FileWrapperImpl : public FileWrapper { bool loop = false, bool text = false) OVERRIDE; + virtual int OpenFromFileHandle(FILE* handle, + bool manage_file, + bool read_only, + bool loop = false) OVERRIDE; + virtual int CloseFile() OVERRIDE; virtual int SetMaxFileSize(size_t bytes) OVERRIDE; virtual int Flush() OVERRIDE; @@ -51,6 +56,7 @@ class FileWrapperImpl : public FileWrapper { scoped_ptr rw_lock_; FILE* id_; + bool managed_file_handle_; bool open_; bool looping_; bool read_only_; diff --git a/webrtc/voice_engine/include/voe_audio_processing.h b/webrtc/voice_engine/include/voe_audio_processing.h index 3ba7b1a2a..8b40360ae 100644 --- a/webrtc/voice_engine/include/voe_audio_processing.h +++ b/webrtc/voice_engine/include/voe_audio_processing.h @@ -35,6 +35,8 @@ #ifndef WEBRTC_VOICE_ENGINE_VOE_AUDIO_PROCESSING_H #define WEBRTC_VOICE_ENGINE_VOE_AUDIO_PROCESSING_H +#include + #include "webrtc/common_types.h" namespace webrtc { @@ -191,6 +193,10 @@ public: // The file can later be used for off-line analysis of the AP performance. virtual int StartDebugRecording(const char* fileNameUTF8) = 0; + // Same as above but sets and uses an existing file handle. Takes ownership + // of |file_handle| and passes it on to the audio processing module. + virtual int StartDebugRecording(FILE* file_handle) = 0; + // Disables recording of AP debugging information. virtual int StopDebugRecording() = 0; diff --git a/webrtc/voice_engine/voe_audio_processing_impl.cc b/webrtc/voice_engine/voe_audio_processing_impl.cc index a57ede938..63a4ed768 100644 --- a/webrtc/voice_engine/voe_audio_processing_impl.cc +++ b/webrtc/voice_engine/voe_audio_processing_impl.cc @@ -972,6 +972,17 @@ int VoEAudioProcessingImpl::StartDebugRecording(const char* fileNameUTF8) { return _shared->audio_processing()->StartDebugRecording(fileNameUTF8); } +int VoEAudioProcessingImpl::StartDebugRecording(FILE* file_handle) { + WEBRTC_TRACE(kTraceApiCall, kTraceVoice, VoEId(_shared->instance_id(), -1), + "StartDebugRecording()"); + if (!_shared->statistics().Initialized()) { + _shared->SetLastError(VE_NOT_INITED, kTraceError); + return -1; + } + + return _shared->audio_processing()->StartDebugRecording(file_handle); +} + int VoEAudioProcessingImpl::StopDebugRecording() { WEBRTC_TRACE(kTraceApiCall, kTraceVoice, VoEId(_shared->instance_id(), -1), "StopDebugRecording()"); diff --git a/webrtc/voice_engine/voe_audio_processing_impl.h b/webrtc/voice_engine/voe_audio_processing_impl.h index 309d997b5..524439d5c 100644 --- a/webrtc/voice_engine/voe_audio_processing_impl.h +++ b/webrtc/voice_engine/voe_audio_processing_impl.h @@ -79,6 +79,7 @@ class VoEAudioProcessingImpl : public VoEAudioProcessing { virtual int GetEcDelayMetrics(int& delay_median, int& delay_std); virtual int StartDebugRecording(const char* fileNameUTF8); + virtual int StartDebugRecording(FILE* file_handle); virtual int StopDebugRecording();