roc-streaminggh-246 Cosmetic fixes in roc_sndio

- unify formatting
- a few renames
- add a few comments
- initialize all vars
- remove some overly verbose checks inherited from sox source/sink
- remove unused members
- use non-default rate (48000) in tests
  (just to be sure the rate passed from test is really applied)

src/internal_modules/roc_sndio/target_pulseaudio/roc_sndio/pulseaudio_backend.cpp

@@ -60,7 +60,7 @@ IDevice* PulseaudioBackend::open_device(DeviceType device_type,
 }
 
 const char* PulseaudioBackend::name() const {
- return "PulseAudio";
+ return "pulseaudio";
 }
 
 } // namespace sndio

src/internal_modules/roc_sndio/target_pulseaudio/roc_sndio/pulseaudio_device.cpp

@@ -492,7 +492,7 @@ bool PulseaudioDevice::load_device_params_(const pa_sample_spec& device_spec) {
  }
 
  if (sample_spec_.sample_rate() == 0) {
- sample_spec_.set_sample_rate((size_t)device_spec.rate);
+ sample_spec_.set_sample_rate(device_spec.rate);
  }
 
  if (!sample_spec_.channel_set().is_valid()) {
@@ -527,7 +527,7 @@ bool PulseaudioDevice::load_device_params_(const pa_sample_spec& device_spec) {
 
  if (target_latency_ns_ <= 0 || target_latency_samples_ <= 0) {
  roc_log(LogError,
- "pulseaudio %s: traget latency must be > 0:"
+ "pulseaudio %s: target latency must be > 0:"
  " target_latency=%.3fms target_latency_samples=%ld",
  device_type_to_str(device_type_),
  (double)target_latency_ns_ / core::Millisecond,

src/internal_modules/roc_sndio/target_sndfile/roc_sndio/sndfile_backend.cpp

@@ -21,20 +21,20 @@ SndfileBackend::SndfileBackend() {
 }
 
 void SndfileBackend::discover_drivers(core::Array<DriverInfo, MaxDrivers>& driver_list) {
- SF_FORMAT_INFO format_info;
- int total_number_of_drivers;
+ SF_FORMAT_INFO format_info = {};
+ int total_number_of_drivers = 0;
 
  if (int errnum = sf_command(NULL, SFC_GET_FORMAT_MAJOR_COUNT,
  &total_number_of_drivers, sizeof(int))) {
- roc_panic("sndfile backend: sf_command(SFC_GET_FORMAT_MAJOR_COUNT) failed %s",
+ roc_panic("sndfile backend: sf_command(SFC_GET_FORMAT_MAJOR_COUNT) failed: %s",
  sf_error_number(errnum));
  }
 
  for (int format_index = 0; format_index < total_number_of_drivers; format_index++) {
  format_info.format = format_index;
  if (int errnum = sf_command(NULL, SFC_GET_FORMAT_MAJOR, &format_info,
  sizeof(format_info))) {
- roc_panic("sndfile backend: sf_command(SFC_GET_FORMAT_MAJOR) failed %s",
+ roc_panic("sndfile backend: sf_command(SFC_GET_FORMAT_MAJOR) failed: %s",
  sf_error_number(errnum));
  }
 
@@ -50,8 +50,7 @@ void SndfileBackend::discover_drivers(core::Array<DriverInfo, MaxDrivers>& drive
  if (!driver_list.push_back(
  DriverInfo(driver, DriverType_File,
  DriverFlag_SupportsSource | DriverFlag_SupportsSink, this))) {
- roc_panic("sndfile backend: driver_list.push_back(DriverInfo) failed to add "
- "driver");
+ roc_panic("sndfile backend: allocation failed");
  }
  }
 }
@@ -114,5 +113,6 @@ IDevice* SndfileBackend::open_device(DeviceType device_type,
 const char* SndfileBackend::name() const {
  return "sndfile";
 }
+
 } // namespace sndio
 } // namespace roc

src/internal_modules/roc_sndio/target_sndfile/roc_sndio/sndfile_extension_table.cpp

@@ -6,11 +6,15 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
-#include "sndfile_extension_table.h"
-#include "sndfile.h"
+#include "roc_sndio/sndfile_extension_table.h"
+#include "roc_core/macro_helpers.h"
 
-FileMap file_type_map[5] = { { SF_FORMAT_MAT4, "mat4", NULL },
- { SF_FORMAT_MAT5, "mat5", NULL },
- { SF_FORMAT_WAV, "wav", "wav" },
- { SF_FORMAT_NIST, "nist", NULL },
- { SF_FORMAT_WAVEX, "wavex", NULL } };
+#include <sndfile.h>
+
+FileMap file_type_map[ROC_ARRAY_SIZE(file_type_map)] = {
+ { SF_FORMAT_MAT4, "mat4", NULL }, //
+ { SF_FORMAT_MAT5, "mat5", NULL }, //
+ { SF_FORMAT_WAV, "wav", "wav" }, //
+ { SF_FORMAT_NIST, "nist", NULL }, //
+ { SF_FORMAT_WAVEX, "wavex", NULL }, //
+};

src/internal_modules/roc_sndio/target_sndfile/roc_sndio/sndfile_sink.cpp

@@ -6,23 +6,26 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
-#define BUFFER_SIZE 512
-
 #include "roc_sndio/sndfile_sink.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"
 #include "roc_sndio/sndfile_extension_table.h"
 
 namespace roc {
 namespace sndio {
+
 namespace {
 
 bool map_to_sub_format(SF_INFO& file_info_, int format_enum) {
- // Provides the minimum number of sub formats needed to support all possible major
- // formats
- int high_to_low_sub_formats[4] = { SF_FORMAT_PCM_24, SF_FORMAT_PCM_16,
- SF_FORMAT_DPCM_16 };
+ // Provides the minimum number of sub formats needed to support all
+ // possible major formats
+ int high_to_low_sub_formats[] = {
+ SF_FORMAT_PCM_24,
+ SF_FORMAT_PCM_16,
+ SF_FORMAT_DPCM_16,
+ };
 
  for (size_t format_attempt = 0;
  format_attempt < ROC_ARRAY_SIZE(high_to_low_sub_formats); format_attempt++) {
@@ -37,7 +40,10 @@ bool map_to_sub_format(SF_INFO& file_info_, int format_enum) {
 }
 
 bool map_to_sndfile(const char** driver, const char* path, SF_INFO& file_info_) {
- const char* file_extension;
+ roc_panic_if(!driver);
+ roc_panic_if(!path);
+
+ const char* file_extension = NULL;
  const char* dot = strrchr(path, '.');
 
  if (!dot || dot == path) {
@@ -48,7 +54,9 @@ bool map_to_sndfile(const char** driver, const char* path, SF_INFO& file_info_)
 
  int format_enum = 0;
 
+ // First try to select format by iterating through file_map_index.
  if (*driver == NULL) {
+ // If driver is NULL, match by file extension.
  for (size_t file_map_index = 0; file_map_index < ROC_ARRAY_SIZE(file_type_map);
  file_map_index++) {
  if (file_type_map[file_map_index].file_extension != NULL) {
@@ -61,6 +69,7 @@ bool map_to_sndfile(const char** driver, const char* path, SF_INFO& file_info_)
  }
  }
  } else {
+ // If driver is non-NULL, match by driver name.
  for (size_t file_map_index = 0; file_map_index < ROC_ARRAY_SIZE(file_type_map);
  file_map_index++) {
  if (strcmp(*driver, file_type_map[file_map_index].driver_name) == 0) {
@@ -70,30 +79,34 @@ bool map_to_sndfile(const char** driver, const char* path, SF_INFO& file_info_)
  }
  }
 
+ // Then try to select format by iterating through all sndfile formats.
  if (format_enum == 0) {
- SF_FORMAT_INFO info;
- int major_count, format_index;
+ SF_FORMAT_INFO info = {};
+ int major_count = 0, format_index = 0;
  if (int errnum =
  sf_command(NULL, SFC_GET_FORMAT_MAJOR_COUNT, &major_count, sizeof(int))) {
- roc_panic("sndfile backend: sf_command(SFC_GET_FORMAT_MAJOR_COUNT) failed %s",
- sf_error_number(errnum));
+ roc_panic(
+ "sndfile backend: sf_command(SFC_GET_FORMAT_MAJOR_COUNT) failed: %s",
+ sf_error_number(errnum));
  }
 
  for (format_index = 0; format_index < major_count; format_index++) {
  info.format = format_index;
  if (int errnum =
  sf_command(NULL, SFC_GET_FORMAT_MAJOR, &info, sizeof(info))) {
- roc_panic("sndfile backend: sf_command(SFC_GET_FORMAT_MAJOR) failed %s",
+ roc_panic("sndfile backend: sf_command(SFC_GET_FORMAT_MAJOR) failed: %s",
  sf_error_number(errnum));
  }
 
  if (*driver == NULL) {
+ // If driver is NULL, match by file extension.
  if (strcmp(info.extension, file_extension) == 0) {
  format_enum = info.format;
  *driver = file_extension;
  break;
  }
  } else {
+ // If driver is non-NULL, match by driver name.
  if (strcmp(info.extension, *driver) == 0) {
  format_enum = info.format;
  break;
@@ -106,13 +119,15 @@ bool map_to_sndfile(const char** driver, const char* path, SF_INFO& file_info_)
  return false;
  }
 
- roc_log(LogDebug, "detected file format type `%s'", *driver);
+ roc_log(LogDebug, "detected file format type '%s'", *driver);
 
  file_info_.format = format_enum | file_info_.format;
 
  if (sf_format_check(&file_info_)) {
+ // Format is supported as is.
  return true;
  } else {
+ // Format may be supported if combined with a subformat.
  return map_to_sub_format(file_info_, format_enum);
  }
 }
@@ -133,13 +148,6 @@ SndfileSink::SndfileSink(core::IArena& arena, const Config& config)
  return;
  }
 
- frame_length_ = config.frame_length;
-
- if (frame_length_ == 0) {
- roc_log(LogError, "sndfile sink: frame length is zero");
- return;
- }
-
  sample_spec_ = config.sample_spec;
 
  if (sample_spec_.sample_format() == audio::SampleFormat_Invalid) {
@@ -159,7 +167,7 @@ SndfileSink::SndfileSink(core::IArena& arena, const Config& config)
 
  memset(&file_info_, 0, sizeof(file_info_));
 
- // TODO(gh-696): map format from sample_space
+ // TODO(gh-696): map format from sample_spec
  file_info_.format = SF_FORMAT_PCM_32;
  file_info_.channels = (int)sample_spec_.num_channels();
  file_info_.samplerate = (int)sample_spec_.sample_rate();
@@ -184,6 +192,10 @@ bool SndfileSink::open(const char* driver, const char* path) {
  roc_panic("sndfile sink: can't call open() more than once");
  }
 
+ if (!path) {
+ roc_panic("sndfile sink: path is null");
+ }
+
  if (!open_(driver, path)) {
  return false;
  }
@@ -244,37 +256,21 @@ audio::SampleSpec SndfileSink::sample_spec() const {
 }
 
 core::nanoseconds_t SndfileSink::latency() const {
- roc_panic_if(!valid_);
-
- if (!file_) {
- roc_panic("sndfile sink: latency(): non-open output file");
- }
-
  return 0;
 }
 
 bool SndfileSink::has_latency() const {
- roc_panic_if(!valid_);
-
- if (!file_) {
- roc_panic("sndfile sink: has_latency(): non-open output file");
- }
-
  return false;
 }
 
 bool SndfileSink::has_clock() const {
- roc_panic_if(!valid_);
-
- if (!file_) {
- roc_panic("sndfile sink: has_clock(): non-open output file");
- }
-
  return false;
 }
 
 void SndfileSink::write(audio::Frame& frame) {
- roc_panic_if(!valid_);
+ if (!file_) {
+ roc_panic("sndfile sink: not opened");
+ }
 
  audio::sample_t* frame_data = frame.raw_samples();
  sf_count_t frame_left = (sf_count_t)frame.num_raw_samples();

src/internal_modules/roc_sndio/target_sndfile/roc_sndio/sndfile_sink.h

@@ -28,8 +28,8 @@ namespace sndio {
 
 //! Sndfile sink.
 //! @remarks
-//! Writes samples to output file or device.
-//! Supports multiple drivers for different file types and audio systems.
+//! Writes samples to output file.
+//! Supports multiple drivers for different file types.
 class SndfileSink : public ISink, public core::NonCopyable<> {
 public:
  //! Initialize.
@@ -43,11 +43,11 @@ class SndfileSink : public ISink, public core::NonCopyable<> {
  //! Open output file or device.
  //!
  //! @b Parameters
- //! - @p driver is output driver name;
- //! - @p path is output file or device name, "-" for stdout.
+ //! - @p driver is output format name;
+ //! - @p path is output file name, "-" for stdout.
  //!
  //! @remarks
- //! If @p driver or @p path are NULL, defaults are used.
+ //! If @p driver is NULL, default is used.
  bool open(const char* driver, const char* path);
 
  //! Cast IDevice to ISink.
@@ -93,7 +93,6 @@ class SndfileSink : public ISink, public core::NonCopyable<> {
  SNDFILE* file_;
  SF_INFO file_info_;
 
- core::nanoseconds_t frame_length_;
  audio::SampleSpec sample_spec_;
  bool valid_;
 };

src/internal_modules/roc_sndio/target_sndfile/roc_sndio/sndfile_source.cpp

@@ -84,7 +84,6 @@ DeviceType SndfileSource::type() const {
 }
 
 DeviceState SndfileSource::state() const {
- roc_panic_if(!valid_);
  return DeviceState_Active;
 }
 
@@ -124,10 +123,8 @@ bool SndfileSource::restart() {
 }
 
 audio::SampleSpec SndfileSource::sample_spec() const {
- roc_panic_if(!valid_);
-
  if (!file_) {
- roc_panic("sndfile source: sample_rate(): non-open output file");
+ roc_panic("sndfile source: not opened");
  }
 
  audio::ChannelSet channel_set;
@@ -140,32 +137,14 @@ audio::SampleSpec SndfileSource::sample_spec() const {
 }
 
 core::nanoseconds_t SndfileSource::latency() const {
- roc_panic_if(!valid_);
-
- if (!file_) {
- roc_panic("sndfile source: latency(): non-open output file");
- }
-
  return 0;
 }
 
 bool SndfileSource::has_latency() const {
- roc_panic_if(!valid_);
-
- if (!file_) {
- roc_panic("sndfile source: has_latency(): non-open input file");
- }
-
  return false;
 }
 
 bool SndfileSource::has_clock() const {
- roc_panic_if(!valid_);
-
- if (!file_) {
- roc_panic("sndfile source: has_clock(): non-open input file");
- }
-
  return false;
 }
 
@@ -204,10 +183,8 @@ bool SndfileSource::read(audio::Frame& frame) {
 }
 
 bool SndfileSource::seek_(size_t offset) {
- roc_panic_if(!valid_);
-
  if (!file_) {
- roc_panic("sndfile source: seek: non-open input file");
+ roc_panic("sndfile source: can't seek: not opened");
  }
 
  roc_log(LogDebug, "sndfile source: resetting position to %lu", (unsigned long)offset);