From 181880617e4c521a4c27348268ec8db897aafdf9 Mon Sep 17 00:00:00 2001 From: Leonardo Invernizzi Date: Sat, 27 Jul 2024 15:24:21 +0000 Subject: [PATCH] gh-748: Add IDevice::close() --- .../roc_pipeline/receiver_loop.cpp | 5 +++ .../roc_pipeline/receiver_loop.h | 1 + .../roc_pipeline/receiver_source.cpp | 4 +++ .../roc_pipeline/receiver_source.h | 3 ++ .../roc_pipeline/sender_loop.cpp | 5 +++ .../roc_pipeline/sender_loop.h | 1 + .../roc_pipeline/sender_sink.cpp | 4 +++ .../roc_pipeline/sender_sink.h | 3 ++ .../roc_pipeline/transcoder_sink.cpp | 4 +++ .../roc_pipeline/transcoder_sink.h | 3 ++ .../roc_pipeline/transcoder_source.cpp | 4 +++ .../roc_pipeline/transcoder_source.h | 3 ++ src/internal_modules/roc_sndio/idevice.h | 7 ++++ src/internal_modules/roc_sndio/pump.cpp | 24 +++++++++++++- src/internal_modules/roc_sndio/pump.h | 1 + .../roc_sndio/pulseaudio_device.cpp | 8 +++++ .../roc_sndio/pulseaudio_device.h | 3 ++ .../target_sndfile/roc_sndio/sndfile_sink.cpp | 15 +++++++-- .../target_sndfile/roc_sndio/sndfile_sink.h | 5 ++- .../roc_sndio/sndfile_source.cpp | 24 +++++++++++--- .../target_sndfile/roc_sndio/sndfile_source.h | 5 ++- .../target_sox/roc_sndio/sox_sink.cpp | 26 ++++++++++++--- .../roc_sndio/target_sox/roc_sndio/sox_sink.h | 5 ++- .../target_sox/roc_sndio/sox_source.cpp | 32 +++++++++++++++---- .../target_sox/roc_sndio/sox_source.h | 5 ++- src/internal_modules/roc_sndio/wav_sink.cpp | 15 +++++++-- src/internal_modules/roc_sndio/wav_sink.h | 5 ++- src/internal_modules/roc_sndio/wav_source.cpp | 18 +++++++---- src/internal_modules/roc_sndio/wav_source.h | 5 ++- .../roc_pipeline/test_helpers/mock_sink.h | 4 +++ .../roc_pipeline/test_helpers/mock_source.h | 4 +++ src/tests/roc_sndio/test_backend_sink.cpp | 2 ++ src/tests/roc_sndio/test_backend_source.cpp | 3 ++ src/tests/roc_sndio/test_helpers/mock_sink.h | 4 +++ .../roc_sndio/test_helpers/mock_source.h | 4 +++ 35 files changed, 230 insertions(+), 34 deletions(-) diff --git a/src/internal_modules/roc_pipeline/receiver_loop.cpp b/src/internal_modules/roc_pipeline/receiver_loop.cpp index c78e0d99c..6c6404f23 100644 --- a/src/internal_modules/roc_pipeline/receiver_loop.cpp +++ b/src/internal_modules/roc_pipeline/receiver_loop.cpp @@ -194,6 +194,11 @@ bool ReceiverLoop::has_clock() const { return source_.has_clock(); } +status::StatusCode ReceiverLoop::close() { + core::Mutex::Lock lock(source_mutex_); + return source_.close(); +} + status::StatusCode ReceiverLoop::rewind() { core::Mutex::Lock lock(source_mutex_); diff --git a/src/internal_modules/roc_pipeline/receiver_loop.h b/src/internal_modules/roc_pipeline/receiver_loop.h index f5a020cfb..ca66fd84b 100644 --- a/src/internal_modules/roc_pipeline/receiver_loop.h +++ b/src/internal_modules/roc_pipeline/receiver_loop.h @@ -153,6 +153,7 @@ class ReceiverLoop : public PipelineLoop, private sndio::ISource { virtual bool has_latency() const; virtual core::nanoseconds_t latency() const; virtual bool has_clock() const; + virtual status::StatusCode close(); virtual status::StatusCode rewind(); virtual void reclock(core::nanoseconds_t timestamp); virtual status::StatusCode read(audio::Frame& frame, diff --git a/src/internal_modules/roc_pipeline/receiver_source.cpp b/src/internal_modules/roc_pipeline/receiver_source.cpp index a0e3fe369..e22b89243 100644 --- a/src/internal_modules/roc_pipeline/receiver_source.cpp +++ b/src/internal_modules/roc_pipeline/receiver_source.cpp @@ -208,6 +208,10 @@ bool ReceiverSource::has_clock() const { return source_config_.common.enable_cpu_clock; } +status::StatusCode ReceiverSource::close() { + return status::StatusOK; +} + status::StatusCode ReceiverSource::rewind() { return status::StatusOK; } diff --git a/src/internal_modules/roc_pipeline/receiver_source.h b/src/internal_modules/roc_pipeline/receiver_source.h index 00fa0a3ae..271c01a2a 100644 --- a/src/internal_modules/roc_pipeline/receiver_source.h +++ b/src/internal_modules/roc_pipeline/receiver_source.h @@ -109,6 +109,9 @@ class ReceiverSource : public sndio::ISource, public core::NonCopyable<> { //! Check if the source has own clock. virtual bool has_clock() const; + //! Explicitly close the source. + virtual ROC_ATTR_NODISCARD status::StatusCode close(); + //! Restart reading from beginning. virtual ROC_ATTR_NODISCARD status::StatusCode rewind(); diff --git a/src/internal_modules/roc_pipeline/sender_loop.cpp b/src/internal_modules/roc_pipeline/sender_loop.cpp index 34bef9128..baee00b6e 100644 --- a/src/internal_modules/roc_pipeline/sender_loop.cpp +++ b/src/internal_modules/roc_pipeline/sender_loop.cpp @@ -158,6 +158,11 @@ bool SenderLoop::has_state() const { return sink_.has_state(); } +status::StatusCode SenderLoop::close() { + core::Mutex::Lock lock(sink_mutex_); + return sink_.close(); +} + sndio::DeviceState SenderLoop::state() const { core::Mutex::Lock lock(sink_mutex_); diff --git a/src/internal_modules/roc_pipeline/sender_loop.h b/src/internal_modules/roc_pipeline/sender_loop.h index 45d1082ba..f76afb038 100644 --- a/src/internal_modules/roc_pipeline/sender_loop.h +++ b/src/internal_modules/roc_pipeline/sender_loop.h @@ -152,6 +152,7 @@ class SenderLoop : public PipelineLoop, private sndio::ISink { virtual core::nanoseconds_t latency() const; virtual bool has_clock() const; virtual status::StatusCode write(audio::Frame& frame); + virtual status::StatusCode close(); // Methods of PipelineLoop virtual core::nanoseconds_t timestamp_imp() const; diff --git a/src/internal_modules/roc_pipeline/sender_sink.cpp b/src/internal_modules/roc_pipeline/sender_sink.cpp index cc4477c90..5507f853a 100644 --- a/src/internal_modules/roc_pipeline/sender_sink.cpp +++ b/src/internal_modules/roc_pipeline/sender_sink.cpp @@ -200,6 +200,10 @@ bool SenderSink::has_clock() const { return sink_config_.enable_cpu_clock; } +status::StatusCode SenderSink::close() { + return status::StatusOK; +} + status::StatusCode SenderSink::write(audio::Frame& frame) { roc_panic_if(init_status_ != status::StatusOK); diff --git a/src/internal_modules/roc_pipeline/sender_sink.h b/src/internal_modules/roc_pipeline/sender_sink.h index 4ef055e37..fa5872e6d 100644 --- a/src/internal_modules/roc_pipeline/sender_sink.h +++ b/src/internal_modules/roc_pipeline/sender_sink.h @@ -106,6 +106,9 @@ class SenderSink : public sndio::ISink, public core::NonCopyable<> { //! Check if the sink has own clock. virtual bool has_clock() const; + //! Explicitly close the sink. + virtual ROC_ATTR_NODISCARD status::StatusCode close(); + //! Write frame. virtual ROC_ATTR_NODISCARD status::StatusCode write(audio::Frame& frame); diff --git a/src/internal_modules/roc_pipeline/transcoder_sink.cpp b/src/internal_modules/roc_pipeline/transcoder_sink.cpp index 22c585a09..134addd04 100644 --- a/src/internal_modules/roc_pipeline/transcoder_sink.cpp +++ b/src/internal_modules/roc_pipeline/transcoder_sink.cpp @@ -123,6 +123,10 @@ bool TranscoderSink::has_clock() const { return false; } +status::StatusCode TranscoderSink::close() { + return status::StatusOK; +} + status::StatusCode TranscoderSink::write(audio::Frame& frame) { roc_panic_if(init_status_ != status::StatusOK); diff --git a/src/internal_modules/roc_pipeline/transcoder_sink.h b/src/internal_modules/roc_pipeline/transcoder_sink.h index 43bd718df..0ddfcedfb 100644 --- a/src/internal_modules/roc_pipeline/transcoder_sink.h +++ b/src/internal_modules/roc_pipeline/transcoder_sink.h @@ -66,6 +66,9 @@ class TranscoderSink : public sndio::ISink, public core::NonCopyable<> { //! Check if the sink has own clock. virtual bool has_clock() const; + //! Explicitly close the sink. + virtual ROC_ATTR_NODISCARD status::StatusCode close(); + //! Write frame. virtual ROC_ATTR_NODISCARD status::StatusCode write(audio::Frame& frame); diff --git a/src/internal_modules/roc_pipeline/transcoder_source.cpp b/src/internal_modules/roc_pipeline/transcoder_source.cpp index e8db73b28..12d29fd90 100644 --- a/src/internal_modules/roc_pipeline/transcoder_source.cpp +++ b/src/internal_modules/roc_pipeline/transcoder_source.cpp @@ -137,6 +137,10 @@ bool TranscoderSource::has_clock() const { return input_source_.has_clock(); } +status::StatusCode TranscoderSource::close() { + return input_source_.close(); +} + status::StatusCode TranscoderSource::rewind() { return input_source_.rewind(); } diff --git a/src/internal_modules/roc_pipeline/transcoder_source.h b/src/internal_modules/roc_pipeline/transcoder_source.h index fd54abab8..829cab600 100644 --- a/src/internal_modules/roc_pipeline/transcoder_source.h +++ b/src/internal_modules/roc_pipeline/transcoder_source.h @@ -77,6 +77,9 @@ class TranscoderSource : public sndio::ISource, public core::NonCopyable<> { //! Check if the source has own clock. virtual bool has_clock() const; + //! Explicitly close the source. + virtual ROC_ATTR_NODISCARD status::StatusCode close(); + //! Restart reading from beginning. virtual ROC_ATTR_NODISCARD status::StatusCode rewind(); diff --git a/src/internal_modules/roc_sndio/idevice.h b/src/internal_modules/roc_sndio/idevice.h index f48c6a385..2c6507a8d 100644 --- a/src/internal_modules/roc_sndio/idevice.h +++ b/src/internal_modules/roc_sndio/idevice.h @@ -117,6 +117,13 @@ class IDevice { //! If false, the user is responsible to maintain the clock and //! perform writes or read in-time. virtual bool has_clock() const = 0; + + //! Explicitly close the device. + //! @remarks + //! This method should be called to release resources held by the device + //! before the object is destructed. If this method is not called before + //! the destructor, the destructor will trigger a panic. + virtual ROC_ATTR_NODISCARD status::StatusCode close() = 0; }; } // namespace sndio diff --git a/src/internal_modules/roc_sndio/pump.cpp b/src/internal_modules/roc_sndio/pump.cpp index ca1b855f6..b613aafae 100644 --- a/src/internal_modules/roc_sndio/pump.cpp +++ b/src/internal_modules/roc_sndio/pump.cpp @@ -72,9 +72,11 @@ status::StatusCode Pump::run() { code = status::StatusOK; // EOF is fine } + status::StatusCode close_code = close_all_devices_(); + roc_log(LogDebug, "pump: exiting main loop"); - return code; + return (code != status::StatusOK) ? code : close_code; } void Pump::stop() { @@ -273,5 +275,25 @@ status::StatusCode Pump::transfer_frame_(ISource& source, ISink& sink) { return status::StatusOK; } +status::StatusCode Pump::close_all_devices_() { + IDevice* devices[] = { &main_source_, &sink_, backup_source_ }; + status::StatusCode first_error = status::StatusOK; + + for (size_t i = 0; i < ROC_ARRAY_SIZE(devices); ++i) { + if (devices[i]) { + status::StatusCode device_code = devices[i]->close(); + if (device_code != status::StatusOK) { + roc_log(LogError, "pump: failed to close device with error %s", + status::code_to_str(device_code)); + if (first_error == status::StatusOK) { + first_error = device_code; + } + } + } + } + + return first_error; +} + } // namespace sndio } // namespace roc diff --git a/src/internal_modules/roc_sndio/pump.h b/src/internal_modules/roc_sndio/pump.h index 24a75bd7b..bfb9593a2 100644 --- a/src/internal_modules/roc_sndio/pump.h +++ b/src/internal_modules/roc_sndio/pump.h @@ -70,6 +70,7 @@ class Pump : public core::NonCopyable<> { status::StatusCode next_(); status::StatusCode switch_source_(ISource* new_source); status::StatusCode transfer_frame_(ISource& source, ISink& sink); + status::StatusCode close_all_devices_(); audio::FrameFactory frame_factory_; diff --git a/src/internal_modules/roc_sndio/target_pulseaudio/roc_sndio/pulseaudio_device.cpp b/src/internal_modules/roc_sndio/target_pulseaudio/roc_sndio/pulseaudio_device.cpp index 1a2fea542..57e4bebf8 100644 --- a/src/internal_modules/roc_sndio/target_pulseaudio/roc_sndio/pulseaudio_device.cpp +++ b/src/internal_modules/roc_sndio/target_pulseaudio/roc_sndio/pulseaudio_device.cpp @@ -177,10 +177,18 @@ PulseaudioDevice::PulseaudioDevice(audio::FrameFactory& frame_factory, } PulseaudioDevice::~PulseaudioDevice() { + if (mainloop_) { + roc_panic("pulseaudio %s: device not closed", device_type_to_str(device_type_)); + } +} + +status::StatusCode PulseaudioDevice::close() { roc_log(LogDebug, "pulseaudio %s: closing device", device_type_to_str(device_type_)); close_(); stop_mainloop_(); + + return status::StatusOK; } status::StatusCode PulseaudioDevice::init_status() const { diff --git a/src/internal_modules/roc_sndio/target_pulseaudio/roc_sndio/pulseaudio_device.h b/src/internal_modules/roc_sndio/target_pulseaudio/roc_sndio/pulseaudio_device.h index dbd513810..46801a7dd 100644 --- a/src/internal_modules/roc_sndio/target_pulseaudio/roc_sndio/pulseaudio_device.h +++ b/src/internal_modules/roc_sndio/target_pulseaudio/roc_sndio/pulseaudio_device.h @@ -79,6 +79,9 @@ class PulseaudioDevice : public ISink, public ISource, public core::NonCopyable< //! Check if the device has own clock. virtual bool has_clock() const; + //! Explicitly close the device. + virtual ROC_ATTR_NODISCARD status::StatusCode close(); + //! Restart reading from beginning. virtual ROC_ATTR_NODISCARD status::StatusCode rewind(); diff --git a/src/internal_modules/roc_sndio/target_sndfile/roc_sndio/sndfile_sink.cpp b/src/internal_modules/roc_sndio/target_sndfile/roc_sndio/sndfile_sink.cpp index 764eaf6d0..8afb109a4 100644 --- a/src/internal_modules/roc_sndio/target_sndfile/roc_sndio/sndfile_sink.cpp +++ b/src/internal_modules/roc_sndio/target_sndfile/roc_sndio/sndfile_sink.cpp @@ -170,7 +170,13 @@ SndfileSink::SndfileSink(audio::FrameFactory& frame_factory, } SndfileSink::~SndfileSink() { - close_(); + if (file_) { + roc_panic("sndfile sink: output file is not closed"); + } +} + +status::StatusCode SndfileSink::close() { + return close_(); } status::StatusCode SndfileSink::init_status() const { @@ -273,9 +279,9 @@ status::StatusCode SndfileSink::open_(const char* driver, const char* path) { return status::StatusOK; } -void SndfileSink::close_() { +status::StatusCode SndfileSink::close_() { if (!file_) { - return; + return status::StatusOK; } roc_log(LogDebug, "sndfile sink: closing output"); @@ -285,9 +291,12 @@ void SndfileSink::close_() { roc_log(LogError, "sndfile sink: sf_close() failed, cannot properly close output: %s", sf_error_number(err)); + return status::StatusErrFile; } file_ = NULL; + + return status::StatusOK; } } // namespace sndio diff --git a/src/internal_modules/roc_sndio/target_sndfile/roc_sndio/sndfile_sink.h b/src/internal_modules/roc_sndio/target_sndfile/roc_sndio/sndfile_sink.h index 7270589d5..1a1b55399 100644 --- a/src/internal_modules/roc_sndio/target_sndfile/roc_sndio/sndfile_sink.h +++ b/src/internal_modules/roc_sndio/target_sndfile/roc_sndio/sndfile_sink.h @@ -63,12 +63,15 @@ class SndfileSink : public ISink, public core::NonCopyable<> { //! Check if the sink has own clock. virtual bool has_clock() const; + //! Explicitly close the sink. + virtual ROC_ATTR_NODISCARD status::StatusCode close(); + //! Write frame. virtual ROC_ATTR_NODISCARD status::StatusCode write(audio::Frame& frame); private: status::StatusCode open_(const char* driver, const char* path); - void close_(); + status::StatusCode close_(); SNDFILE* file_; SF_INFO file_info_; diff --git a/src/internal_modules/roc_sndio/target_sndfile/roc_sndio/sndfile_source.cpp b/src/internal_modules/roc_sndio/target_sndfile/roc_sndio/sndfile_source.cpp index a099af82a..65810d315 100644 --- a/src/internal_modules/roc_sndio/target_sndfile/roc_sndio/sndfile_source.cpp +++ b/src/internal_modules/roc_sndio/target_sndfile/roc_sndio/sndfile_source.cpp @@ -11,6 +11,7 @@ #include "roc_core/log.h" #include "roc_core/panic.h" #include "roc_sndio/backend_map.h" +#include "roc_status/code_to_str.h" namespace roc { namespace sndio { @@ -41,7 +42,13 @@ SndfileSource::SndfileSource(audio::FrameFactory& frame_factory, } SndfileSource::~SndfileSource() { - close_(); + if (file_) { + roc_panic("sndfile source: input file is not closed"); + } +} + +status::StatusCode SndfileSource::close() { + return close_(); } status::StatusCode SndfileSource::init_status() const { @@ -107,8 +114,14 @@ status::StatusCode SndfileSource::rewind() { } if (file_) { - close_(); + const status::StatusCode close_code = close_(); + if (close_code != status::StatusOK) { + roc_log(LogError, "sndfile source: failed to close file during rewind: %s", + status::code_to_str(close_code)); + return close_code; + } } + return open_(); } @@ -197,9 +210,9 @@ status::StatusCode SndfileSource::open_() { return status::StatusOK; } -void SndfileSource::close_() { +status::StatusCode SndfileSource::close_() { if (!file_) { - return; + return status::StatusOK; } roc_log(LogInfo, "sndfile source: closing input"); @@ -209,9 +222,12 @@ void SndfileSource::close_() { roc_log(LogError, "sndfile source: sf_close() failed, cannot properly close input: %s", sf_error_number(err)); + return status::StatusErrFile; } file_ = NULL; + + return status::StatusOK; } } // namespace sndio diff --git a/src/internal_modules/roc_sndio/target_sndfile/roc_sndio/sndfile_source.h b/src/internal_modules/roc_sndio/target_sndfile/roc_sndio/sndfile_source.h index 175756efe..060b1d4e3 100644 --- a/src/internal_modules/roc_sndio/target_sndfile/roc_sndio/sndfile_source.h +++ b/src/internal_modules/roc_sndio/target_sndfile/roc_sndio/sndfile_source.h @@ -64,6 +64,9 @@ class SndfileSource : public ISource, private core::NonCopyable<> { //! Check if the source has own clock. virtual bool has_clock() const; + //! Explicitly close the source. + virtual ROC_ATTR_NODISCARD status::StatusCode close(); + //! Restart reading from beginning. virtual ROC_ATTR_NODISCARD status::StatusCode rewind(); @@ -78,7 +81,7 @@ class SndfileSource : public ISource, private core::NonCopyable<> { private: status::StatusCode open_(); - void close_(); + status::StatusCode close_(); status::StatusCode seek_(size_t offset); diff --git a/src/internal_modules/roc_sndio/target_sox/roc_sndio/sox_sink.cpp b/src/internal_modules/roc_sndio/target_sox/roc_sndio/sox_sink.cpp index 597beb2d0..482931ed5 100644 --- a/src/internal_modules/roc_sndio/target_sox/roc_sndio/sox_sink.cpp +++ b/src/internal_modules/roc_sndio/target_sox/roc_sndio/sox_sink.cpp @@ -10,6 +10,7 @@ #include "roc_core/log.h" #include "roc_core/panic.h" #include "roc_sndio/backend_map.h" +#include "roc_status/code_to_str.h" namespace roc { namespace sndio { @@ -70,7 +71,13 @@ SoxSink::SoxSink(audio::FrameFactory& frame_factory, } SoxSink::~SoxSink() { - close_(); + if (output_) { + roc_panic("sox sink: output file is not closed"); + } +} + +status::StatusCode SoxSink::close() { + return close_(); } status::StatusCode SoxSink::init_status() const { @@ -146,7 +153,12 @@ status::StatusCode SoxSink::pause() { output_name_.c_str()); if (driver_type_ == DriverType_Device) { - close_(); + const status::StatusCode close_code = close_(); + if (close_code != status::StatusOK) { + roc_log(LogError, "sox sink: failed to close output during pause: %s", + status::code_to_str(close_code)); + return close_code; + } } paused_ = true; @@ -312,19 +324,23 @@ status::StatusCode SoxSink::write_(const sox_sample_t* samples, size_t n_samples return status::StatusOK; } -void SoxSink::close_() { +status::StatusCode SoxSink::close_() { if (!output_) { - return; + return status::StatusOK; } roc_log(LogDebug, "sox sink: closing output"); const int err = sox_close(output_); if (err != SOX_SUCCESS) { - roc_panic("sox sink: can't close output: %s", sox_strerror(err)); + roc_log(LogError, "sox sink: can't close output: %s", sox_strerror(err)); + return driver_type_ == DriverType_File ? status::StatusErrFile + : status::StatusErrDevice; } output_ = NULL; + + return status::StatusOK; } } // namespace sndio diff --git a/src/internal_modules/roc_sndio/target_sox/roc_sndio/sox_sink.h b/src/internal_modules/roc_sndio/target_sox/roc_sndio/sox_sink.h index 3500f1e33..fb51119ae 100644 --- a/src/internal_modules/roc_sndio/target_sox/roc_sndio/sox_sink.h +++ b/src/internal_modules/roc_sndio/target_sox/roc_sndio/sox_sink.h @@ -75,6 +75,9 @@ class SoxSink : public ISink, public core::NonCopyable<> { //! Check if the sink has own clock. virtual bool has_clock() const; + //! Explicitly close the sink. + virtual ROC_ATTR_NODISCARD status::StatusCode close(); + //! Write frame. virtual ROC_ATTR_NODISCARD status::StatusCode write(audio::Frame& frame); @@ -84,7 +87,7 @@ class SoxSink : public ISink, public core::NonCopyable<> { status::StatusCode open_(); status::StatusCode write_(const sox_sample_t* samples, size_t n_samples); - void close_(); + status::StatusCode close_(); const DriverType driver_type_; core::StringBuffer driver_name_; diff --git a/src/internal_modules/roc_sndio/target_sox/roc_sndio/sox_source.cpp b/src/internal_modules/roc_sndio/target_sox/roc_sndio/sox_source.cpp index 211464c0e..a51244b74 100644 --- a/src/internal_modules/roc_sndio/target_sox/roc_sndio/sox_source.cpp +++ b/src/internal_modules/roc_sndio/target_sox/roc_sndio/sox_source.cpp @@ -10,6 +10,7 @@ #include "roc_core/log.h" #include "roc_core/panic.h" #include "roc_sndio/backend_map.h" +#include "roc_status/code_to_str.h" namespace roc { namespace sndio { @@ -74,7 +75,13 @@ SoxSource::SoxSource(audio::FrameFactory& frame_factory, } SoxSource::~SoxSource() { - close_(); + if (input_) { + roc_panic("sox source: input file is not closed"); + } +} + +status::StatusCode SoxSource::close() { + return close_(); } status::StatusCode SoxSource::init_status() const { @@ -150,7 +157,12 @@ status::StatusCode SoxSource::pause() { input_name_.c_str()); if (driver_type_ == DriverType_Device) { - close_(); + const status::StatusCode close_code = close_(); + if (close_code != status::StatusOK) { + roc_log(LogError, "sox source: failed to close input during pause: %s", + status::code_to_str(close_code)); + return close_code; + } } paused_ = true; @@ -199,7 +211,12 @@ status::StatusCode SoxSource::rewind() { sample_spec_.clear(); if (input_) { - close_(); + const status::StatusCode close_code = close_(); + if (close_code != status::StatusOK) { + roc_log(LogError, "sox source: failed to close input during rewind: %s", + status::code_to_str(close_code)); + return close_code; + } } const status::StatusCode code = open_(); @@ -386,19 +403,22 @@ status::StatusCode SoxSource::seek_(uint64_t offset) { return status::StatusOK; } -void SoxSource::close_() { +status::StatusCode SoxSource::close_() { if (!input_) { - return; + return status::StatusOK; } roc_log(LogInfo, "sox source: closing input"); const int err = sox_close(input_); if (err != SOX_SUCCESS) { - roc_panic("sox source: can't close input: %s", sox_strerror(err)); + roc_log(LogError, "sox source: can't close input: %s", sox_strerror(err)); + return status::StatusErrFile; } input_ = NULL; + + return status::StatusOK; } } // namespace sndio diff --git a/src/internal_modules/roc_sndio/target_sox/roc_sndio/sox_source.h b/src/internal_modules/roc_sndio/target_sox/roc_sndio/sox_source.h index 517a5eb1f..669b802e0 100644 --- a/src/internal_modules/roc_sndio/target_sox/roc_sndio/sox_source.h +++ b/src/internal_modules/roc_sndio/target_sox/roc_sndio/sox_source.h @@ -76,6 +76,9 @@ class SoxSource : public ISource, private core::NonCopyable<> { //! Check if the source has own clock. virtual bool has_clock() const; + //! Explicitly close the source. + virtual ROC_ATTR_NODISCARD status::StatusCode close(); + //! Restart reading from beginning. virtual ROC_ATTR_NODISCARD status::StatusCode rewind(); @@ -94,7 +97,7 @@ class SoxSource : public ISource, private core::NonCopyable<> { status::StatusCode open_(); status::StatusCode seek_(uint64_t offset); - void close_(); + status::StatusCode close_(); audio::FrameFactory& frame_factory_; diff --git a/src/internal_modules/roc_sndio/wav_sink.cpp b/src/internal_modules/roc_sndio/wav_sink.cpp index 88954b6e9..957bbd878 100644 --- a/src/internal_modules/roc_sndio/wav_sink.cpp +++ b/src/internal_modules/roc_sndio/wav_sink.cpp @@ -48,7 +48,13 @@ WavSink::WavSink(audio::FrameFactory& frame_factory, } WavSink::~WavSink() { - close_(); + if (output_file_) { + roc_panic("wav sink: output file is not closed"); + } +} + +status::StatusCode WavSink::close() { + return close_(); } status::StatusCode WavSink::init_status() const { @@ -161,9 +167,9 @@ status::StatusCode WavSink::open_(const char* path) { return status::StatusOK; } -void WavSink::close_() { +status::StatusCode WavSink::close_() { if (!output_file_) { - return; + return status::StatusOK; } roc_log(LogDebug, "wav sink: closing output file"); @@ -171,9 +177,12 @@ void WavSink::close_() { if (fclose(output_file_)) { roc_log(LogError, "wav sink: can't properly close output file: %s", core::errno_to_str(errno).c_str()); + return status::StatusErrFile; } output_file_ = NULL; + + return status::StatusOK; } } // namespace sndio diff --git a/src/internal_modules/roc_sndio/wav_sink.h b/src/internal_modules/roc_sndio/wav_sink.h index 154023752..507d7c883 100644 --- a/src/internal_modules/roc_sndio/wav_sink.h +++ b/src/internal_modules/roc_sndio/wav_sink.h @@ -60,12 +60,15 @@ class WavSink : public ISink, public core::NonCopyable<> { //! Check if the sink has own clock. virtual bool has_clock() const; + //! Explicitly close the sink. + virtual ROC_ATTR_NODISCARD status::StatusCode close(); + //! Write frame. virtual ROC_ATTR_NODISCARD status::StatusCode write(audio::Frame& frame); private: status::StatusCode open_(const char* path); - void close_(); + status::StatusCode close_(); audio::SampleSpec sample_spec_; diff --git a/src/internal_modules/roc_sndio/wav_source.cpp b/src/internal_modules/roc_sndio/wav_source.cpp index eb390b76c..a331f6bc8 100644 --- a/src/internal_modules/roc_sndio/wav_source.cpp +++ b/src/internal_modules/roc_sndio/wav_source.cpp @@ -36,7 +36,13 @@ WavSource::WavSource(audio::FrameFactory& frame_factory, } WavSource::~WavSource() { - close_(); + if (file_opened_) { + roc_panic("wav source: input file is not closed"); + } +} + +status::StatusCode WavSource::close() { + return close_(); } status::StatusCode WavSource::init_status() const { @@ -183,13 +189,13 @@ status::StatusCode WavSource::open_(const char* path) { return status::StatusOK; } -void WavSource::close_() { - if (!file_opened_) { - return; +status::StatusCode WavSource::close_() { + if (file_opened_) { + file_opened_ = false; + drwav_uninit(&wav_); } - file_opened_ = false; - drwav_uninit(&wav_); + return status::StatusOK; } } // namespace sndio diff --git a/src/internal_modules/roc_sndio/wav_source.h b/src/internal_modules/roc_sndio/wav_source.h index 57b42d811..bc3691c82 100644 --- a/src/internal_modules/roc_sndio/wav_source.h +++ b/src/internal_modules/roc_sndio/wav_source.h @@ -61,6 +61,9 @@ class WavSource : public ISource, private core::NonCopyable<> { //! Check if the source has own clock. virtual bool has_clock() const; + //! Explicitly close the source. + virtual ROC_ATTR_NODISCARD status::StatusCode close(); + //! Restart reading from beginning. virtual ROC_ATTR_NODISCARD status::StatusCode rewind(); @@ -75,7 +78,7 @@ class WavSource : public ISource, private core::NonCopyable<> { private: status::StatusCode open_(const char* path); - void close_(); + status::StatusCode close_(); audio::FrameFactory& frame_factory_; diff --git a/src/tests/roc_pipeline/test_helpers/mock_sink.h b/src/tests/roc_pipeline/test_helpers/mock_sink.h index 1cdc807cb..fb8fb2143 100644 --- a/src/tests/roc_pipeline/test_helpers/mock_sink.h +++ b/src/tests/roc_pipeline/test_helpers/mock_sink.h @@ -57,6 +57,10 @@ class MockSink : public sndio::ISink, public core::NonCopyable<> { return false; } + virtual status::StatusCode close() { + return status::StatusOK; + } + virtual status::StatusCode write(audio::Frame& frame) { CHECK(frame.num_raw_samples() % n_chans_ == 0); diff --git a/src/tests/roc_pipeline/test_helpers/mock_source.h b/src/tests/roc_pipeline/test_helpers/mock_source.h index e0ef443cf..cbffd640a 100644 --- a/src/tests/roc_pipeline/test_helpers/mock_source.h +++ b/src/tests/roc_pipeline/test_helpers/mock_source.h @@ -79,6 +79,10 @@ class MockSource : public sndio::ISource { return false; } + virtual status::StatusCode close() { + return status::StatusOK; + } + virtual status::StatusCode rewind() { state_ = sndio::DeviceState_Active; return status::StatusOK; diff --git a/src/tests/roc_sndio/test_backend_sink.cpp b/src/tests/roc_sndio/test_backend_sink.cpp index fea4baa9d..0df9a0d87 100644 --- a/src/tests/roc_sndio/test_backend_sink.cpp +++ b/src/tests/roc_sndio/test_backend_sink.cpp @@ -67,6 +67,7 @@ TEST(backend_sink, open) { CHECK(!backend_sink->has_state()); CHECK(!backend_sink->has_latency()); CHECK(!backend_sink->has_clock()); + LONGS_EQUAL(status::StatusOK, backend_sink->close()); } } @@ -127,6 +128,7 @@ TEST(backend_sink, open_default_config) { backend_sink); CHECK(backend_sink->sample_spec().is_valid()); + LONGS_EQUAL(status::StatusOK, backend_sink->close()); } } diff --git a/src/tests/roc_sndio/test_backend_source.cpp b/src/tests/roc_sndio/test_backend_source.cpp index 197c373ed..bef99dc4c 100644 --- a/src/tests/roc_sndio/test_backend_source.cpp +++ b/src/tests/roc_sndio/test_backend_source.cpp @@ -112,6 +112,7 @@ TEST(backend_source, open) { CHECK(!backend_source->has_state()); CHECK(!backend_source->has_latency()); CHECK(!backend_source->has_clock()); + LONGS_EQUAL(status::StatusOK, backend_source->close()); } } @@ -189,6 +190,7 @@ TEST(backend_source, rewind) { != 0) { FAIL("frames should be equal"); } + LONGS_EQUAL(status::StatusOK, backend_source->close()); } } @@ -220,6 +222,7 @@ TEST(backend_source, rewind_after_eof) { // rewind LONGS_EQUAL(status::StatusOK, backend_source->rewind()); } + LONGS_EQUAL(status::StatusOK, backend_source->close()); } } diff --git a/src/tests/roc_sndio/test_helpers/mock_sink.h b/src/tests/roc_sndio/test_helpers/mock_sink.h index 302b2046c..5049345ac 100644 --- a/src/tests/roc_sndio/test_helpers/mock_sink.h +++ b/src/tests/roc_sndio/test_helpers/mock_sink.h @@ -65,6 +65,10 @@ class MockSink : public ISink { return false; } + virtual status::StatusCode close() { + return status::StatusOK; + } + virtual status::StatusCode write(audio::Frame& frame) { CHECK(pos_ + frame.num_raw_samples() <= MaxSz); diff --git a/src/tests/roc_sndio/test_helpers/mock_source.h b/src/tests/roc_sndio/test_helpers/mock_source.h index fe1e39f75..68c1ecdf4 100644 --- a/src/tests/roc_sndio/test_helpers/mock_source.h +++ b/src/tests/roc_sndio/test_helpers/mock_source.h @@ -75,6 +75,10 @@ class MockSource : public ISource { return false; } + virtual status::StatusCode close() { + return status::StatusOK; + } + virtual status::StatusCode rewind() { FAIL("not implemented"); return status::StatusAbort;