Skip to content

Commit

Permalink
roc-streaminggh-246 Sndfile backend improvements
Browse files Browse the repository at this point in the history 
- Use SampleSpec::use_defaults() for simpler code.

- Simplify how SampleSpec is populated.

- In SndfileSink, don't call seek, because some formats
  are not seekable and will fail.

- In SndfileSource, check if file is seekable. If yes,
  implement restart() using seek, otherwise implement
  it by reopening file.

- In SndfileSource, use StringBuffer for path, because
  provided const char* may point to a temporary string.

- If close fails, print error instead of panic.
  • Loading branch information
@gavv
gavv committed Apr 9, 2024
1 parent 6db52c7 commit 35c87b9
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 106 deletions.
65 changes: 12 additions & 53 deletions src/internal_modules/roc_sndio/target_sndfile/roc_sndio/sndfile_sink.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,11 +137,6 @@ bool map_to_sndfile(const char** driver, const char* path, SF_INFO& file_info_)
SndfileSink::SndfileSink(core::IArena& arena, const Config& config)
: file_(NULL)
, valid_(false) {
if (config.sample_spec.num_channels() == 0) {
roc_log(LogError, "sndfile sink: # of channels is zero");
return;
}

if (config.latency != 0) {
roc_log(LogError,
"sndfile sink: setting io latency not supported by sndfile backend");
Expand All @@ -150,20 +145,9 @@ SndfileSink::SndfileSink(core::IArena& arena, const Config& config)

sample_spec_ = config.sample_spec;

if (sample_spec_.sample_format() == audio::SampleFormat_Invalid) {
sample_spec_.set_sample_format(audio::SampleFormat_Pcm);
sample_spec_.set_pcm_format(audio::PcmFormat_SInt32);
}

if (sample_spec_.sample_rate() == 0) {
sample_spec_.set_sample_rate(41000);
}

if (!sample_spec_.channel_set().is_valid()) {
sample_spec_.channel_set().set_layout(audio::ChanLayout_Surround);
sample_spec_.channel_set().set_order(audio::ChanOrder_Smpte);
sample_spec_.channel_set().set_channel_range(0, 2, true);
}
sample_spec_.use_defaults(audio::Sample_RawFormat, audio::ChanLayout_Surround,
audio::ChanOrder_Smpte, audio::ChanMask_Surround_Stereo,
44100);

memset(&file_info_, 0, sizeof(file_info_));

Expand Down Expand Up @@ -234,25 +218,11 @@ bool SndfileSink::restart() {
}

audio::SampleSpec SndfileSink::sample_spec() const {
roc_panic_if(!valid_);

if (!file_) {
roc_panic("sndfile sink: sample_rate(): non-open output file");
}

if (file_info_.channels == 1) {
return audio::SampleSpec(size_t(file_info_.samplerate), audio::Sample_RawFormat,
audio::ChanLayout_Surround, audio::ChanOrder_Smpte,
audio::ChanMask_Surround_Mono);
}

if (file_info_.channels == 2) {
return audio::SampleSpec(size_t(file_info_.samplerate), audio::Sample_RawFormat,
audio::ChanLayout_Surround, audio::ChanOrder_Smpte,
audio::ChanMask_Surround_Stereo);
roc_panic("sndfile sink: not opened");
}

roc_panic("sndfile sink: unsupported channel count");
return sample_spec_;
}

core::nanoseconds_t SndfileSink::latency() const {
Expand Down Expand Up @@ -286,14 +256,10 @@ void SndfileSink::write(audio::Frame& frame) {
}

bool SndfileSink::open_(const char* driver, const char* path) {
unsigned long in_rate = (unsigned long)file_info_.samplerate;

unsigned long out_rate = (unsigned long)file_info_.samplerate;

if (!map_to_sndfile(&driver, path, file_info_)) {
roc_log(LogDebug,
"sndfile sink: map_to_sndfile(): Cannot find valid subtype format for "
"major format type");
"sndfile sink: map_to_sndfile():"
" cannot find valid subtype format for major format type");
return false;
}

Expand All @@ -312,16 +278,8 @@ bool SndfileSink::open_(const char* driver, const char* path) {

sample_spec_.set_sample_rate((unsigned long)file_info_.samplerate);

roc_log(LogInfo,
"sndfile sink:"
" opened: out_rate=%lu in_rate=%lu ch=%lu",
out_rate, in_rate, (unsigned long)file_info_.channels);

sf_count_t err = sf_seek(file_, 0, SEEK_SET);
if (err == -1) {
roc_log(LogError, "sndfile sink: sf_seek(): %s", sf_strerror(file_));
return false;
}
roc_log(LogInfo, "sndfile sink: opened: %s",
audio::sample_spec_to_str(sample_spec_).c_str());

return true;
}
Expand All @@ -335,8 +293,9 @@ void SndfileSink::close_() {

int err = sf_close(file_);
if (err != 0) {
roc_panic("sndfile sink: sf_close() failed. Cannot close output: %s",
sf_error_number(err));
roc_log(LogError,
"sndfile sink: sf_close() failed, cannot properly close output: %s",
sf_error_number(err));
}

file_ = NULL;
Expand Down
81 changes: 33 additions & 48 deletions src/internal_modules/roc_sndio/target_sndfile/roc_sndio/sndfile_source.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

#include "roc_sndio/sndfile_source.h"
#include "roc_audio/sample_spec_to_str.h"
#include "roc_core/log.h"
#include "roc_core/panic.h"
#include "roc_sndio/backend_map.h"
Expand All @@ -16,8 +17,7 @@ namespace sndio {

SndfileSource::SndfileSource(core::IArena& arena, const Config& config)
: file_(NULL)
, path_(NULL)
, eof_(false)
, path_(arena)
, valid_(false) {
if (config.latency != 0) {
roc_log(LogError,
Expand All @@ -30,16 +30,7 @@ SndfileSource::SndfileSource(core::IArena& arena, const Config& config)
return;
}

frame_length_ = config.frame_length;
sample_spec_ = config.sample_spec;

if (frame_length_ == 0) {
roc_log(LogError, "sndfile source: frame length is zero");
return;
}

memset(&file_info_, 0, sizeof(file_info_));
sample_rate_ = config.sample_spec.sample_rate();
valid_ = true;
}

Expand All @@ -54,17 +45,22 @@ bool SndfileSource::is_valid() const {
bool SndfileSource::open(const char* driver, const char* path) {
roc_panic_if(!valid_);

if (path) {
path_ = path;
}

roc_log(LogInfo, "sndfile source: opening: driver=%s path=%s", driver, path);
roc_log(LogDebug, "sndfile source: opening: driver=%s path=%s", driver, path);

if (file_) {
roc_panic("sndfile source: can't call open() more than once");
}

if (!open_(path)) {
if (!path) {
roc_panic("sndfile sink: path is null");
}

if (!path_.assign(path)) {
roc_log(LogError, "sndfile source: can't allocate string");
return false;
}

if (!open_()) {
return false;
}

Expand Down Expand Up @@ -101,7 +97,7 @@ bool SndfileSource::restart() {

roc_log(LogDebug, "sndfile source: restarting");

if (!eof_) {
if (file_ && file_info_.seekable) {
if (!seek_(0)) {
roc_log(LogError, "sndfile source: seek failed when restarting");
return false;
Expand All @@ -111,14 +107,12 @@ bool SndfileSource::restart() {
close_();
}

if (!open_(path_)) {
if (!open_()) {
roc_log(LogError, "sndfile source: open failed when restarting");
return false;
}
}

eof_ = false;

return true;
}

Expand All @@ -127,13 +121,7 @@ audio::SampleSpec SndfileSource::sample_spec() const {
roc_panic("sndfile source: not opened");
}

audio::ChannelSet channel_set;
channel_set.set_layout(audio::ChanLayout_Surround);
channel_set.set_order(audio::ChanOrder_Smpte);
channel_set.set_channel_range(0, (size_t)file_info_.channels - 1, true);

return audio::SampleSpec(size_t(file_info_.samplerate), audio::Sample_RawFormat,
channel_set);
return sample_spec_;
}

core::nanoseconds_t SndfileSource::latency() const {
Expand All @@ -156,30 +144,24 @@ bool SndfileSource::read(audio::Frame& frame) {
roc_panic_if(!valid_);

if (!file_) {
roc_panic("sndfile source: read: non-open input file");
roc_panic("sndfile source: not opened");
}

audio::sample_t* frame_data = frame.raw_samples();
// size_t num_channels = (size_t)file_info_.channels;
sf_count_t frame_left = (sf_count_t)frame.num_raw_samples();
// size_t samples_per_ch = frame.num_raw_samples() / num_channels;

sf_count_t n_samples = sf_read_float(file_, frame_data, frame_left);
if (sf_error(file_) != 0) {
// TODO(gh-183): return error instead of panic
roc_panic("sndfile source: sf_read_float() failed: %s", sf_strerror(file_));
}

if (n_samples == 0) {
eof_ = true;
}

if (n_samples < frame_left && n_samples != 0) {
memset(frame.raw_samples() + (size_t)n_samples, 0,
(size_t)(frame_left - n_samples) * sizeof(audio::sample_t));
}

return !eof_;
return n_samples != 0;
}

bool SndfileSource::seek_(size_t offset) {
Expand All @@ -198,28 +180,29 @@ bool SndfileSource::seek_(size_t offset) {
return true;
}

bool SndfileSource::open_(const char* path) {
bool SndfileSource::open_() {
if (file_) {
roc_panic("sndfile source: can't open: already opened");
}

file_info_.format = 0;

file_ = sf_open(path, SFM_READ, &file_info_);
file_ = sf_open(path_.c_str(), SFM_READ, &file_info_);
if (!file_) {
roc_log(LogInfo, "sndfile source: can't open: input=%s, %s", !path ? NULL : path,
roc_log(LogInfo, "sndfile source: can't open: input=%s, %s", path_.c_str(),
sf_strerror(file_));
return false;
}

sample_spec_.set_sample_rate((unsigned long)file_info_.samplerate);
sample_spec_.set_sample_format(audio::SampleFormat_Pcm);
sample_spec_.set_pcm_format(audio::Sample_RawFormat);
sample_spec_.set_sample_rate((size_t)file_info_.samplerate);
sample_spec_.channel_set().set_layout(audio::ChanLayout_Surround);
sample_spec_.channel_set().set_order(audio::ChanOrder_Smpte);
sample_spec_.channel_set().set_range(0, (size_t)file_info_.channels - 1);

roc_log(LogInfo,
"sndfile source:"
" in_rate=%lu out_rate=%lu"
" in_ch=%lu out_ch=%lu",
(unsigned long)file_info_.samplerate, (unsigned long)sample_rate_,
(unsigned long)file_info_.channels, (unsigned long)0);
roc_log(LogInfo, "sndfile source: opened: %s",
audio::sample_spec_to_str(sample_spec_).c_str());

return true;
}
Expand All @@ -230,10 +213,12 @@ void SndfileSource::close_() {
}

roc_log(LogInfo, "sndfile source: closing input");

int err = sf_close(file_);
if (err != 0) {
roc_panic("sndfile source: sf_close() failed. Cannot close input: %s",
sf_error_number(err));
roc_log(LogError,
"sndfile source: sf_close() failed, cannot properly close input: %s",
sf_error_number(err));
}

file_ = NULL;
Expand Down
7 changes: 2 additions & 5 deletions src/internal_modules/roc_sndio/target_sndfile/roc_sndio/sndfile_source.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,19 +91,16 @@ class SndfileSource : public ISource, private core::NonCopyable<> {
virtual bool read(audio::Frame&);

private:
bool open_(const char* path);
bool open_();
void close_();

bool seek_(size_t offset);

core::nanoseconds_t frame_length_;
audio::SampleSpec sample_spec_;

SNDFILE* file_;
SF_INFO file_info_;
const char* path_;
size_t sample_rate_;
bool eof_;
core::StringBuffer path_;
bool valid_;
};

Expand Down

0 comments on commit 35c87b9

Please sign in to comment.