[PATCH RESEND 0/9] some old accumulated patches

[PATCH RESEND 0/9] some old accumulated patches

[Eric Wong]

Hi Måns, I guess you're merging stuff these days?

I still use sox every day, but I've mostly stopped hacking C
since gcc and clang takes too long to compile.  Here's a bunch
of non-critical patches I had queued up from years ago.  (I
mailed separately about the wavpack issue).

In particular, I'm really happy with the "|program" speedup
on Linux with F_SETPIPE_SZ and use it all the time.

Anyways, thanks for keeping this project going!

The following changes since commit 9f2d60f9cd52a1cc5ae92e02bd862c8cb4b85f13:

  sox: silence gcc warning in headroom() function (2020-07-17 18:52:20 +0100)

are available in the Git repository at:

  https://80x24.org/sox.git for-mans_20200731

for you to fetch changes up to f5f7a3f32ac2d03fde04ac7433514666dd4e0700:

  Added average power spectrum for stat -freq -a (2020-07-31 09:14:53 +0000)

----------------------------------------------------------------
Alexandre Ratchov (1):
      sndio: handle 24-bit samples properly on OpenBSD

Eric Wong (2):
      use non-blocking stdin for interactive mode
      speed up "|program" inputs on Linux 2.6.35+

Guido Günther (1):
      Handle vorbis_analysis_headerout errors

Jan Stary (1):
      sox.1: fix section name

Martin Guy (2):
      spectrogram: remove arbitrary limit on height of spectrogram
      Add spectrogram -n flag to normalise the output to maximum brightness

Pander (1):
      Added average power spectrum for stat -freq -a

gabor.karsay@gmx.at (1):
      fix manpage warning: "table wider than line width"

 sox.1             | 14 +++++++++++--
 src/formats.c     | 51 ++++++++++++++++++++++++++++++++++++++++++++++
 src/sndio.c       |  8 ++++++--
 src/sox.c         | 12 +++++++++++
 src/spectrogram.c | 60 +++++++++++++++++++++++++++++++++++++++++++------------
 src/stat.c        | 27 ++++++++++++++++++++++++-
 src/vorbis.c      |  8 ++++++--
 7 files changed, 160 insertions(+), 20 deletions(-)


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel

[PATCH RESEND 1/9] use non-blocking stdin for interactive mode

[Eric Wong]

From: Eric Wong <e@80x24.org>

When accepting keyboard input, it is possible for select() to
return a false-positive with spurious wakeups from inside the
update_status callback.

Using the FIONREAD ioctl in place of select is also a possibility,
but may be less portable.
---
 src/sox.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/src/sox.c b/src/sox.c
index 9412614e..6b35041b 100644
--- a/src/sox.c
+++ b/src/sox.c
@@ -1804,6 +1804,18 @@ static int process(void)
     tcsetattr(fileno(stdin), TCSANOW, &modified_termios);
   }
 #endif
+#if defined(F_GETFL) && defined(F_SETFL) && defined(O_NONBLOCK)
+  if (interactive) {
+    int fd = fileno(stdin);
+    int flags = fcntl(fd, F_GETFL);
+    if (flags == -1) {
+      lsx_warn("error getting flags on stdin descriptor: %s", strerror(errno));
+    } else if (!(flags & O_NONBLOCK)) {
+      if (fcntl(fd, F_SETFL, flags | O_NONBLOCK) == -1)
+        lsx_warn("error setting non-blocking on stdin: %s", strerror(errno));
+    }
+  }
+#endif
 
   setsig(SIGTERM, sigint); /* Stop gracefully, as soon as we possibly can. */
   setsig(SIGINT , sigint); /* Either skip current input or behave as SIGTERM. */


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel

[PATCH RESEND 2/9] speed up "|program" inputs on Linux 2.6.35+

[Eric Wong]

Linux 2.6.35+ allows pipe buffer resizing via fcntl(2).   When
running multi-threaded SoX invocations with large buffers, the
default pipe size (64K) can be too small and become a bottleneck
for IPC.  Increasing the pipe to the maximum allowed size
reduces the amount of stalls in data flow between processes (SoX
or otherwise).

When using SOX_OPTS="--multi-thread --buffer 131072" on a 4-core
system, a command like:

	sox -M "|sox $< -p $(lft_fx)" "|sox $< -p $(rgt_fx)" $@

..can run significantly faster (10-80%) depending on the
processing chain, file sizes/formats and effects in use.

Before this patch, using "--buffer 131072" could be hugely
detrimental to performance due to the pipe being only half
the size (64K) of the SoX buffer.
---
 src/formats.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/src/formats.c b/src/formats.c
index f3efe764..1da8489c 100644
--- a/src/formats.c
+++ b/src/formats.c
@@ -18,6 +18,7 @@
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
  */
 
+#define _GNU_SOURCE
 #include "sox_i.h"
 
 #include <assert.h>
@@ -37,6 +38,10 @@
   #include <magic.h>
 #endif
 
+#ifdef HAVE_UNISTD_H
+#  include <unistd.h>
+#endif
+
 #define PIPE_AUTO_DETECT_SIZE 256 /* Only as much as we can rewind a pipe */
 #define AUTO_DETECT_SIZE 4096     /* For seekable file, so no restriction */
 
@@ -370,6 +375,50 @@ static int xfclose(FILE * file, lsx_io_type io_type)
     fclose(file);
 }
 
+static void incr_pipe_size(FILE *f)
+{
+/*
+ * Linux 2.6.35 and later has the ability to expand the pipe buffer
+ * Try to get it as big as possible to avoid stalls when SoX itself
+ * is using big buffers
+ */
+#if defined(F_GETPIPE_SZ) && defined(F_SETPIPE_SZ)
+  static long max_pipe_size;
+
+  /* read the maximum size of the pipe the first time this is called */
+  if (max_pipe_size == 0) {
+    const char path[] = "/proc/sys/fs/pipe-max-size";
+    int fd = open(path, O_RDONLY);
+
+    max_pipe_size = -1;
+    if (fd >= 0) {
+      char buf[80];
+      ssize_t r = read(fd, buf, sizeof(buf));
+
+      if (r > 0) {
+        buf[r] = 0;
+        max_pipe_size = strtol(buf, NULL, 10);
+
+        /* guard against obviously wrong values on messed up systems */
+        if (max_pipe_size <= PIPE_BUF || max_pipe_size > INT_MAX)
+          max_pipe_size = -1;
+      }
+      close(fd);
+    }
+  }
+
+  if (max_pipe_size > PIPE_BUF) {
+    int fd = fileno(f);
+
+    if (fcntl(fd, F_SETPIPE_SZ, max_pipe_size) >= 0)
+      lsx_debug("got pipe %ld bytes\n", max_pipe_size);
+    else
+      lsx_warn("couldn't set pipe size to %ld bytes: %s\n",
+               max_pipe_size, strerror(errno));
+  }
+#endif /* do nothing for platforms without F_{GET,SET}PIPE_SZ */
+}
+
 static FILE * xfopen(char const * identifier, char const * mode, lsx_io_type * io_type)
 {
   *io_type = lsx_io_file;
@@ -382,6 +431,7 @@ static FILE * xfopen(char const * identifier, char const * mode, lsx_io_type * i
 #endif
     f = popen(identifier + 1, POPEN_MODE);
     *io_type = lsx_io_pipe;
+    incr_pipe_size(f);
 #else
     lsx_fail("this build of SoX cannot open pipes");
 #endif
@@ -394,6 +444,7 @@ static FILE * xfopen(char const * identifier, char const * mode, lsx_io_type * i
     char * command = lsx_malloc(strlen(command_format) + strlen(identifier));
     sprintf(command, command_format, identifier);
     f = popen(command, POPEN_MODE);
+    incr_pipe_size(f);
     free(command);
     *io_type = lsx_io_url;
 #else


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel

[PATCH RESEND 3/9] sox.1: fix section name

[Eric Wong]

From: Jan Stary <hans@stare.cz>

The SoX manpage does not have an 'Input File Balancing" section,
but it has an "Input File Combining" section.

ref: <20150715131359.GA12685@www.stare.cz>
---
 sox.1 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sox.1 b/sox.1
index 2c4ca470..35a22d98 100644
--- a/sox.1
+++ b/sox.1
@@ -1085,7 +1085,7 @@ See also the
 .BR vol ,
 and
 .B gain
-effects, and see \fBInput File Balancing\fR above.
+effects, and see \fBInput File Combining\fR above.
 .SS Input & Output File Format Options
 These options apply to the input or output file whose name they
 immediately precede on the command line and are used mainly when


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel

[PATCH RESEND 4/9] sndio: handle 24-bit samples properly on OpenBSD

[Eric Wong]

From: Alexandre Ratchov <alex@caoua.org>

Reported-by: Jan Stary <hans@stare.cz>
cf. http://marc.info/?l=openbsd-ports&m=147395657332262&w=2
---
 src/sndio.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/sndio.c b/src/sndio.c
index e031b5bb..77785ce4 100644
--- a/src/sndio.c
+++ b/src/sndio.c
@@ -113,8 +113,6 @@ static int startany(sox_format_t *ft, unsigned mode)
     else
       reqpar.rchan = ft->signal.channels;
   }
-  if (ft->signal.precision > 0)
-    reqpar.bits = ft->signal.precision;
   switch (ft->encoding.encoding) {
   case SOX_ENCODING_SIGN2:
     reqpar.sig = 1;
@@ -127,6 +125,12 @@ static int startany(sox_format_t *ft, unsigned mode)
   }
   if (ft->encoding.bits_per_sample > 0)
     reqpar.bits = ft->encoding.bits_per_sample;
+  else if (ft->signal.precision > 0)
+    reqpar.bits = ft->signal.precision;
+  else
+    reqpar.bits = SOX_DEFAULT_PRECISION;
+  reqpar.bps = (reqpar.bits + 7) / 8;
+  reqpar.msb = 1;
   if (ft->encoding.reverse_bytes != sox_option_default) {
     reqpar.le = SIO_LE_NATIVE;
     if (ft->encoding.reverse_bytes)


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel

[PATCH RESEND 5/9] Handle vorbis_analysis_headerout errors

[Eric Wong]

From: Guido Günther <agx@sigxcpu.org>

This is related to

    https://github.com/xiph/vorbis/pull/34

but could also happen today with on other errors in the called function.
---
 src/vorbis.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/vorbis.c b/src/vorbis.c
index aafce7e2..9fa234ce 100644
--- a/src/vorbis.c
+++ b/src/vorbis.c
@@ -270,8 +270,11 @@ static int write_vorbis_header(sox_format_t * ft, vorbis_enc_t * ve)
       vc.comment_lengths[i] = strlen(text);
     }
   }
-  vorbis_analysis_headerout(    /* Build the packets */
-      &ve->vd, &vc, &header_main, &header_comments, &header_codebooks);
+  if (vorbis_analysis_headerout(    /* Build the packets */
+      &ve->vd, &vc, &header_main, &header_comments, &header_codebooks) < 0) {
+      ret = HEADER_ERROR;
+      goto cleanup;
+  }
 
   ogg_stream_packetin(&ve->os, &header_main);   /* And stream them out */
   ogg_stream_packetin(&ve->os, &header_comments);
@@ -280,6 +283,7 @@ static int write_vorbis_header(sox_format_t * ft, vorbis_enc_t * ve)
   while (ogg_stream_flush(&ve->os, &ve->og) && ret == HEADER_OK)
     if (!oe_write_page(&ve->og, ft))
       ret = HEADER_ERROR;
+cleanup:
   for (i = 0; i < vc.comments; ++i)
     free(vc.user_comments[i]);
   free(vc.user_comments);


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel

[PATCH RESEND 6/9] fix manpage warning: "table wider than line width"

[Eric Wong]

From: "gabor.karsay@gmx.at" <gabor.karsay@gmx.at>

Reported-by: Jaromír Mikeš <mira.mikes@seznam.cz>
---
 sox.1 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sox.1 b/sox.1
index 35a22d98..5a4766dd 100644
--- a/sox.1
+++ b/sox.1
@@ -2699,7 +2699,7 @@ as detailed in the following table:
 .ne 6
 .TS
 center;
-lB lw52.
+lB lw51.
 \-M/\-I/\-L	Phase response = minimum/intermediate/linear
 \-s	Steep filter (band-width = 99%)
 \-a	Allow aliasing/imaging above the pass-band


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel

[PATCH RESEND 7/9] spectrogram: remove arbitrary limit on height of spectrogram

[Eric Wong]

From: Martin Guy <martinwguy@gmail.com>

Sox had an arbitrary limit of 8193 on the vertical axis size
based on MAX_FFT_SIZE=4096 and had fixed-size arrays for its data.
This is both wasteful of memory for smaller FFTs and stops us producing
more detailed output for no obvious reason.

This patch removes the size limit on Y-axis-height by making array
allocation dynamic. In practice, you can't remove the limit as getopt
insists on minimum and maximum values for numeric arguments, so we
copy the similarly arbitrary limit of 200000 from MAX_X_SIZE.

Tested-by: Eric Wong <normalperson@yhbt.net>
---
 src/spectrogram.c | 37 +++++++++++++++++++++++++++----------
 1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/src/spectrogram.c b/src/spectrogram.c
index afb0b0e8..b38d2124 100644
--- a/src/spectrogram.c
+++ b/src/spectrogram.c
@@ -36,10 +36,13 @@
   #include <io.h>
 #endif
 
-#define MAX_FFT_SIZE 4096
 #define is_p2(x) !(x & (x - 1))
 
+/* These are arbitrary as there is no upper limit, but
+ * sox's getopt() needs an upper limit for each option, so...
+ */
 #define MAX_X_SIZE 200000
+#define MAX_Y_SIZE 200000
 
 typedef enum {Window_Hann, Window_Hamming, Window_Bartlett, Window_Rectangular, Window_Kaiser, Window_Dolph} win_type_t;
 static lsx_enum_item const window_options[] = {
@@ -71,8 +74,11 @@ typedef struct {
   int        dft_size, step_size, block_steps, block_num, rows, cols, read;
   int        x_size, end, end_min, last_end;
   sox_bool   truncated;
-  double     buf[MAX_FFT_SIZE], dft_buf[MAX_FFT_SIZE], window[MAX_FFT_SIZE+1];
-  double     block_norm, max, magnitudes[(MAX_FFT_SIZE>>1) + 1];
+  double     * buf;		/* [dft_size] */
+  double     * dft_buf;		/* [dft_size] */
+  double     * window;		/* [dft_size + 1] */
+  double     block_norm, max;
+  double     * magnitudes;	/* [(dft_size / 2) + 1] */
   float      * dBfs;
 } priv_t;
 
@@ -114,9 +120,9 @@ static int getopts(sox_effect_t * effp, int argc, char **argv)
 
   while ((c = lsx_getopt(&optstate)) != -1) switch (c) {
     GETOPT_NUMERIC(optstate, 'x', x_size0       , 100, MAX_X_SIZE)
-    GETOPT_NUMERIC(optstate, 'X', pixels_per_sec,  1 , 5000)
-    GETOPT_NUMERIC(optstate, 'y', y_size        , 64 , 1200)
-    GETOPT_NUMERIC(optstate, 'Y', Y_size        , 130, MAX_FFT_SIZE / 2 + 2)
+    GETOPT_NUMERIC(optstate, 'X', pixels_per_sec,  1 , MAX_X_SIZE)
+    GETOPT_NUMERIC(optstate, 'y', y_size        , 64 , MAX_Y_SIZE)
+    GETOPT_NUMERIC(optstate, 'Y', Y_size        , 130, MAX_Y_SIZE)
     GETOPT_NUMERIC(optstate, 'z', dB_range      , 20 , 180)
     GETOPT_NUMERIC(optstate, 'Z', gain          ,-100, 100)
     GETOPT_NUMERIC(optstate, 'q', spectrum_points, 0 , p->spectrum_points)
@@ -172,7 +178,7 @@ static double make_window(priv_t * p, int end)
   double sum = 0, * w = end < 0? p->window : p->window + end;
   int i, n = 1 + p->dft_size - abs(end);
 
-  if (end) memset(p->window, 0, sizeof(p->window));
+  if (end) memset(p->window, 0, sizeof(*(p->window)) * p->dft_size);
   for (i = 0; i < n; ++i) w[i] = 1;
   switch (p->win_type) {
     case Window_Hann: lsx_apply_hann(w, n); break;
@@ -190,10 +196,10 @@ static double make_window(priv_t * p, int end)
   return sum;
 }
 
-static double * rdft_init(int n)
+static double * rdft_init(size_t n)
 {
   double * q = lsx_malloc(2 * (n / 2 + 1) * n * sizeof(*q)), * p = q;
-  int i, j;
+  size_t i, j;
   for (j = 0; j <= n / 2; ++j) for (i = 0; i < n; ++i)
     *p++ = cos(2 * M_PI * j * i / n), *p++ = sin(2 * M_PI * j * i / n);
   return q;
@@ -260,11 +266,18 @@ static int start(sox_effect_t * effp)
   if (p->y_size) {
     p->dft_size = 2 * (p->y_size - 1);
     if (!is_p2(p->dft_size) && !effp->flow)
-      p->shared = rdft_init(p->dft_size);
+      p->shared = rdft_init((size_t)(p->dft_size));
   } else {
    int y = max(32, (p->Y_size? p->Y_size : 550) / effp->in_signal.channels - 2);
    for (p->dft_size = 128; p->dft_size <= y; p->dft_size <<= 1);
   }
+
+  /* Now that dft_size is set, allocate variable-sized elements of priv_t */
+  p->buf	= lsx_calloc(p->dft_size, sizeof(*(p->buf)));
+  p->dft_buf	= lsx_calloc(p->dft_size, sizeof(*(p->dft_buf)));
+  p->window	= lsx_calloc(p->dft_size + 1, sizeof(*(p->window)));
+  p->magnitudes = lsx_calloc((p->dft_size / 2) + 1, sizeof(*(p->magnitudes)));
+
   if (is_p2(p->dft_size) && !effp->flow)
     lsx_safe_rdft(p->dft_size, 1, p->dft_buf);
   lsx_debug("duration=%g x_size=%i pixels_per_sec=%g dft_size=%i", duration, p->x_size, pixels_per_sec, p->dft_size);
@@ -651,6 +664,10 @@ error: png_destroy_write_struct(&png, &png_info);
   free(png_rows);
   free(pixels);
   free(p->dBfs);
+  free(p->buf);
+  free(p->dft_buf);
+  free(p->window);
+  free(p->magnitudes);
   return SOX_SUCCESS;
 }
 


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel

[PATCH RESEND 8/9] Add spectrogram -n flag to normalise the output to maximum brightness

[Eric Wong]

From: Martin Guy <martinwguy@gmail.com>

This change adds a "normalize" flag, -n, to sox spectrogram to adjust
the spectrogram gain such that the highest values get the brightest
colours, allowing one to get uniform spectrograms regardless of
different volumes in music files.

Tested-by: Eric Wong <normalperson@yhbt.net>
---
 sox.1             |  4 ++++
 src/spectrogram.c | 23 ++++++++++++++++++++---
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/sox.1 b/sox.1
index 5a4766dd..04ab09b1 100644
--- a/sox.1
+++ b/sox.1
@@ -3235,6 +3235,10 @@ A negative
 .I num
 effectively increases the `brightness' of the spectrogram display,
 and vice versa.
+.IP \fB\-n\fR
+Sets the upper limit of the Z axis so that the loudest pixels
+are shown using the brightest colour in the palette - a kind of
+automatic \fB\-Z\fR flag.
 .IP \fB\-q\ \fInum\fR
 Sets the Z-axis quantisation, i.e. the number of different colours (or
 intensities) in which to render Z-axis
diff --git a/src/spectrogram.c b/src/spectrogram.c
index b38d2124..34457f21 100644
--- a/src/spectrogram.c
+++ b/src/spectrogram.c
@@ -59,7 +59,7 @@ typedef struct {
   double     pixels_per_sec, window_adjust;
   int        x_size0, y_size, Y_size, dB_range, gain, spectrum_points, perm;
   sox_bool   monochrome, light_background, high_colour, slack_overlap, no_axes;
-  sox_bool   raw, alt_palette, truncate;
+  sox_bool   normalize, raw, alt_palette, truncate;
   win_type_t win_type;
   char const * out_name, * title, * comment;
   char const *duration_str, *start_time_str;
@@ -113,7 +113,7 @@ static int getopts(sox_effect_t * effp, int argc, char **argv)
   char const * next;
   int c;
   lsx_getopt_t optstate;
-  lsx_getopt_init(argc, argv, "+S:d:x:X:y:Y:z:Z:q:p:W:w:st:c:AarmlhTo:", NULL, lsx_getopt_flag_none, 1, &optstate);
+  lsx_getopt_init(argc, argv, "+S:d:x:X:y:Y:z:Z:q:p:W:w:st:c:AarmnlhTo:", NULL, lsx_getopt_flag_none, 1, &optstate);
 
   p->dB_range = 120, p->spectrum_points = 249, p->perm = 1; /* Non-0 defaults */
   p->out_name = "spectrogram.png", p->comment = "Created by SoX";
@@ -134,6 +134,7 @@ static int getopts(sox_effect_t * effp, int argc, char **argv)
     case 'a': p->no_axes          = sox_true;   break;
     case 'r': p->raw              = sox_true;   break;
     case 'm': p->monochrome       = sox_true;   break;
+    case 'n': p->normalize        = sox_true;   break;
     case 'l': p->light_background = sox_true;   break;
     case 'h': p->high_colour      = sox_true;   break;
     case 'T': p->truncate         = sox_true;   break;
@@ -557,6 +558,7 @@ static int stop(sox_effect_t * effp) /* only called, by end(), on flow 0 */
   int         i, j, k, base, step, tick_len = 3 - p->no_axes;
   char        text[200], * prefix;
   double      limit;
+  float       autogain = 0.0;	/* Is changed if the -n flag was supplied */
 
   free(p->shared);
   if (p->using_stdout) {
@@ -583,8 +585,22 @@ static int stop(sox_effect_t * effp) /* only called, by end(), on flow 0 */
     png_rows[rows - 1 - j] = (png_bytep)(pixels + j * cols);
 
   /* Spectrogram */
+
+  if (p->normalize)
+    /* values are already in dB, so we subtract the maximum value
+     * (which will normally be negative) to raise the maximum to 0.0.
+     */
+    autogain = -p->max;
+
   for (k = 0; k < chans; ++k) {
     priv_t * q = (priv_t *)(effp - effp->flow + k)->priv;
+
+    if (p->normalize) {
+      float *fp;
+
+      for (i = p->rows * p->cols, fp = q->dBfs; i > 0 ; fp++, i--)
+	*fp += autogain;
+    }
     base = !p->raw * below + (chans - 1 - k) * (p->rows + 1);
     for (j = 0; j < p->rows; ++j) {
       for (i = 0; i < p->cols; ++i)
@@ -651,7 +667,7 @@ static int stop(sox_effect_t * effp) /* only called, by end(), on flow 0 */
     step = 10 * ceil(p->dB_range / 10. * (font_y + 2) / (k - 1));
     for (i = 0; i <= p->dB_range; i += step) {           /* (Tick) labels */
       int y = (double)i / p->dB_range * (k - 1) + .5;
-      sprintf(text, "%+i", i - p->gain - p->dB_range);
+      sprintf(text, "%+i", i - p->gain - p->dB_range - (int)(autogain+0.5));
       print_at(cols - right + 1, base + y + 5, Labels, text);
     }
   }
@@ -692,6 +708,7 @@ sox_effect_handler_t const * lsx_spectrogram_effect_fn(void)
     "\t-Y num\tY-height total (i.e. not per channel); default 550",
     "\t-z num\tZ-axis range in dB; default 120",
     "\t-Z num\tZ-axis maximum in dBFS; default 0",
+    "\t-n\tSet Z-axis maximum to the brightest pixel",
     "\t-q num\tZ-axis quantisation (0 - 249); default 249",
     "\t-w name\tWindow: Hann(default)/Hamming/Bartlett/Rectangular/Kaiser/Dolph",
     "\t-W num\tWindow adjust parameter (-10 - 10); applies only to Kaiser/Dolph",


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel

[PATCH RESEND 9/9] Added average power spectrum for stat -freq -a

[Eric Wong]

From: Pander <pander@users.sourceforge.net>

references:
https://sourceforge.net/p/sox/mailman/message/33186778/
https://sourceforge.net/p/sox/mailman/message/33175940/
https://sourceforge.net/p/sox/mailman/message/33625255/

Edited-by: Eric Wong <normalperson@yhbt.net>

[ew: removed extraneous whitespace changes,
 squashed re_average initialization fix from Pander,
 removed unnecessary check before free(3) call
 (ref: <5498A2F9.2010500@free.fr>),
 converted fft_average to bool]
---
 sox.1      |  6 ++++++
 src/stat.c | 27 ++++++++++++++++++++++++++-
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/sox.1 b/sox.1
index 04ab09b1..d64762af 100644
--- a/sox.1
+++ b/sox.1
@@ -3596,6 +3596,12 @@ audio in SoX's internal buffer.
 This is mainly used to help track down endian problems that
 sometimes occur in cross-platform versions of SoX.
 .SP
+The
+.B \-a
+option
+will output the average input's power spectrum instead of the default
+behavior in which the power spectrum is given for each 4096 point DFT.
+.SP
 See also the
 .B stats
 effect.
diff --git a/src/stat.c b/src/stat.c
index cdea27cc..0ca6c232 100644
--- a/src/stat.c
+++ b/src/stat.c
@@ -34,6 +34,7 @@ typedef struct {
   float *re_out;
   unsigned long fft_size;
   unsigned long fft_offset;
+  sox_bool fft_average;
 } priv_t;
 
 
@@ -69,6 +70,8 @@ static int sox_stat_getopts(sox_effect_t * effp, int argc, char **argv)
       stat->fft = 1;
     else if (!(strcmp(*argv, "-d")))
       stat->volume = 2;
+    else if (!(strcmp(*argv, "-a")))
+      stat->fft_average = sox_true;
     else {
       lsx_fail("Summary effect: unknown option");
       return SOX_EOF;
@@ -100,6 +103,7 @@ static int sox_stat_start(sox_effect_t * effp)
     stat->bin[i] = 0;
 
   stat->fft_size = 4096;
+  stat->fft_average = sox_false;
   stat->re_in = stat->re_out = NULL;
 
   if (stat->fft) {
@@ -134,6 +138,16 @@ static int sox_stat_flow(sox_effect_t * effp, const sox_sample_t *ibuf, sox_samp
   priv_t * stat = (priv_t *) effp->priv;
   int done, x, len = min(*isamp, *osamp);
   short count = 0;
+  float *re_average = NULL;
+  unsigned samples = 0;
+  float ffa = 0.0;
+  unsigned i;
+
+  if (stat->fft_average) {
+      samples = (stat->fft_size / 2);
+      ffa = effp->in_signal.rate / samples;
+      re_average = lsx_malloc(sizeof(float) * (int)samples);
+  }
 
   if (len) {
     if (stat->read == 0)          /* 1st sample */
@@ -146,10 +160,20 @@ static int sox_stat_flow(sox_effect_t * effp, const sox_sample_t *ibuf, sox_samp
 
         if (stat->fft_offset >= stat->fft_size) {
           stat->fft_offset = 0;
+          if (stat->fft_average) {
+              lsx_power_spectrum_f((int)samples, stat->re_in, stat->re_out);
+              for (i = 0; i < samples / 2; i++) /* FIXME: should be <= samples / 2 */
+                  re_average[i] += stat->re_out[i];
+          } else {
           print_power_spectrum((unsigned) stat->fft_size, effp->in_signal.rate, stat->re_in, stat->re_out);
+          }
         }
 
       }
+      if (stat->fft_average) {
+          for (i = 0; i < samples / 2; i++) /* FIXME: should be <= samples / 2 */
+              fprintf(stderr, " %f  %f\n", ffa * i, re_average[i] / len);
+      }
     }
 
     for (done = 0; done < len; done++) {
@@ -192,6 +216,7 @@ static int sox_stat_flow(sox_effect_t * effp, const sox_sample_t *ibuf, sox_samp
     stat->read += len;
   }
 
+  free(re_average);
   *isamp = *osamp = len;
   /* Process all samples */
 
@@ -320,7 +345,7 @@ static int sox_stat_stop(sox_effect_t * effp)
 
 static sox_effect_handler_t sox_stat_effect = {
   "stat",
-  "[ -s N ] [ -rms ] [-freq] [ -v ] [ -d ]",
+  "[ -s N ] [ -rms ] [-freq] [ -v ] [ -d ] [ -a ]",
   SOX_EFF_MCHAN | SOX_EFF_MODIFY,
   sox_stat_getopts,
   sox_stat_start,


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel

Re: [PATCH RESEND 0/9] some old accumulated patches

[Måns Rullgård]

Eric Wong <normalperson@yhbt.net> writes:

> Hi Måns, I guess you're merging stuff these days?
>
> I still use sox every day, but I've mostly stopped hacking C
> since gcc and clang takes too long to compile.

Get a faster computer.  AMD makes some nice ones.

> Here's a bunch of non-critical patches I had queued up from years ago.
> (I mailed separately about the wavpack issue).

Thanks.  I'll check them over and push them unless I spot any problems.

-- 
Måns Rullgård


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel

Re: [PATCH RESEND 2/9] speed up "|program" inputs on Linux 2.6.35+

[Måns Rullgård]

Eric Wong <normalperson@yhbt.net> writes:

> Linux 2.6.35+ allows pipe buffer resizing via fcntl(2).   When
> running multi-threaded SoX invocations with large buffers, the
> default pipe size (64K) can be too small and become a bottleneck
> for IPC.  Increasing the pipe to the maximum allowed size
> reduces the amount of stalls in data flow between processes (SoX
> or otherwise).
>
> When using SOX_OPTS="--multi-thread --buffer 131072" on a 4-core
> system, a command like:
>
> 	sox -M "|sox $< -p $(lft_fx)" "|sox $< -p $(rgt_fx)" $@
>
> ..can run significantly faster (10-80%) depending on the
> processing chain, file sizes/formats and effects in use.
>
> Before this patch, using "--buffer 131072" could be hugely
> detrimental to performance due to the pipe being only half
> the size (64K) of the SoX buffer.
> ---
>  src/formats.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
>
> diff --git a/src/formats.c b/src/formats.c
> index f3efe764..1da8489c 100644
> --- a/src/formats.c
> +++ b/src/formats.c
> @@ -18,6 +18,7 @@
>   * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
>   */
>
> +#define _GNU_SOURCE
>  #include "sox_i.h"
>
>  #include <assert.h>
> @@ -37,6 +38,10 @@
>    #include <magic.h>
>  #endif
>
> +#ifdef HAVE_UNISTD_H
> +#  include <unistd.h>
> +#endif
> +
>  #define PIPE_AUTO_DETECT_SIZE 256 /* Only as much as we can rewind a pipe */
>  #define AUTO_DETECT_SIZE 4096     /* For seekable file, so no restriction */
>
> @@ -370,6 +375,50 @@ static int xfclose(FILE * file, lsx_io_type io_type)
>      fclose(file);
>  }
>
> +static void incr_pipe_size(FILE *f)
> +{
> +/*
> + * Linux 2.6.35 and later has the ability to expand the pipe buffer
> + * Try to get it as big as possible to avoid stalls when SoX itself
> + * is using big buffers
> + */
> +#if defined(F_GETPIPE_SZ) && defined(F_SETPIPE_SZ)
> +  static long max_pipe_size;
> +
> +  /* read the maximum size of the pipe the first time this is called */
> +  if (max_pipe_size == 0) {
> +    const char path[] = "/proc/sys/fs/pipe-max-size";
> +    int fd = open(path, O_RDONLY);
> +
> +    max_pipe_size = -1;
> +    if (fd >= 0) {
> +      char buf[80];
> +      ssize_t r = read(fd, buf, sizeof(buf));
> +
> +      if (r > 0) {
> +        buf[r] = 0;

In the (bizarre) event that read() actually reads 80 bytes, that will
write outside the buffer.  I'll fix it, just to be sure.

> +        max_pipe_size = strtol(buf, NULL, 10);
> +
> +        /* guard against obviously wrong values on messed up systems */
> +        if (max_pipe_size <= PIPE_BUF || max_pipe_size > INT_MAX)
> +          max_pipe_size = -1;
> +      }
> +      close(fd);
> +    }
> +  }
> +
> +  if (max_pipe_size > PIPE_BUF) {
> +    int fd = fileno(f);
> +
> +    if (fcntl(fd, F_SETPIPE_SZ, max_pipe_size) >= 0)
> +      lsx_debug("got pipe %ld bytes\n", max_pipe_size);
> +    else
> +      lsx_warn("couldn't set pipe size to %ld bytes: %s\n",
> +               max_pipe_size, strerror(errno));
> +  }
> +#endif /* do nothing for platforms without F_{GET,SET}PIPE_SZ */
> +}
> +
>  static FILE * xfopen(char const * identifier, char const * mode, lsx_io_type * io_type)
>  {
>    *io_type = lsx_io_file;
> @@ -382,6 +431,7 @@ static FILE * xfopen(char const * identifier, char const * mode, lsx_io_type * i
>  #endif
>      f = popen(identifier + 1, POPEN_MODE);
>      *io_type = lsx_io_pipe;
> +    incr_pipe_size(f);
>  #else
>      lsx_fail("this build of SoX cannot open pipes");
>  #endif
> @@ -394,6 +444,7 @@ static FILE * xfopen(char const * identifier, char const * mode, lsx_io_type * i
>      char * command = lsx_malloc(strlen(command_format) + strlen(identifier));
>      sprintf(command, command_format, identifier);
>      f = popen(command, POPEN_MODE);
> +    incr_pipe_size(f);
>      free(command);
>      *io_type = lsx_io_url;
>  #else

-- 
Måns Rullgård


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel

Re: [PATCH RESEND 4/9] sndio: handle 24-bit samples properly on OpenBSD

[Måns Rullgård]

Eric Wong <normalperson@yhbt.net> writes:

> From: Alexandre Ratchov <alex@caoua.org>
>
> Reported-by: Jan Stary <hans@stare.cz>
> cf. http://marc.info/?l=openbsd-ports&m=147395657332262&w=2
> ---
>  src/sndio.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/src/sndio.c b/src/sndio.c
> index e031b5bb..77785ce4 100644
> --- a/src/sndio.c
> +++ b/src/sndio.c
> @@ -113,8 +113,6 @@ static int startany(sox_format_t *ft, unsigned mode)
>      else
>        reqpar.rchan = ft->signal.channels;
>    }
> -  if (ft->signal.precision > 0)
> -    reqpar.bits = ft->signal.precision;
>    switch (ft->encoding.encoding) {
>    case SOX_ENCODING_SIGN2:
>      reqpar.sig = 1;
> @@ -127,6 +125,12 @@ static int startany(sox_format_t *ft, unsigned mode)
>    }
>    if (ft->encoding.bits_per_sample > 0)
>      reqpar.bits = ft->encoding.bits_per_sample;
> +  else if (ft->signal.precision > 0)
> +    reqpar.bits = ft->signal.precision;
> +  else
> +    reqpar.bits = SOX_DEFAULT_PRECISION;
> +  reqpar.bps = (reqpar.bits + 7) / 8;
> +  reqpar.msb = 1;
>    if (ft->encoding.reverse_bytes != sox_option_default) {
>      reqpar.le = SIO_LE_NATIVE;
>      if (ft->encoding.reverse_bytes)

I'm not familiar with OpenBSD, so I'll trust you guys that this is correct.

-- 
Måns Rullgård


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel

Re: [PATCH RESEND 7/9] spectrogram: remove arbitrary limit on height of spectrogram

[Måns Rullgård]

Eric Wong <normalperson@yhbt.net> writes:

> From: Martin Guy <martinwguy@gmail.com>
>
> Sox had an arbitrary limit of 8193 on the vertical axis size
> based on MAX_FFT_SIZE=4096 and had fixed-size arrays for its data.
> This is both wasteful of memory for smaller FFTs and stops us producing
> more detailed output for no obvious reason.
>
> This patch removes the size limit on Y-axis-height by making array
> allocation dynamic. In practice, you can't remove the limit as getopt
> insists on minimum and maximum values for numeric arguments, so we
> copy the similarly arbitrary limit of 200000 from MAX_X_SIZE.
>
> Tested-by: Eric Wong <normalperson@yhbt.net>
> ---
>  src/spectrogram.c | 37 +++++++++++++++++++++++++++----------
>  1 file changed, 27 insertions(+), 10 deletions(-)
>
> diff --git a/src/spectrogram.c b/src/spectrogram.c
> index afb0b0e8..b38d2124 100644
> --- a/src/spectrogram.c
> +++ b/src/spectrogram.c
> @@ -36,10 +36,13 @@
>    #include <io.h>
>  #endif
>
> -#define MAX_FFT_SIZE 4096
>  #define is_p2(x) !(x & (x - 1))
>
> +/* These are arbitrary as there is no upper limit, but
> + * sox's getopt() needs an upper limit for each option, so...
> + */
>  #define MAX_X_SIZE 200000
> +#define MAX_Y_SIZE 200000
>
>  typedef enum {Window_Hann, Window_Hamming, Window_Bartlett, Window_Rectangular, Window_Kaiser, Window_Dolph} win_type_t;
>  static lsx_enum_item const window_options[] = {
> @@ -71,8 +74,11 @@ typedef struct {
>    int        dft_size, step_size, block_steps, block_num, rows, cols, read;
>    int        x_size, end, end_min, last_end;
>    sox_bool   truncated;
> -  double     buf[MAX_FFT_SIZE], dft_buf[MAX_FFT_SIZE], window[MAX_FFT_SIZE+1];
> -  double     block_norm, max, magnitudes[(MAX_FFT_SIZE>>1) + 1];
> +  double     * buf;		/* [dft_size] */
> +  double     * dft_buf;		/* [dft_size] */
> +  double     * window;		/* [dft_size + 1] */
> +  double     block_norm, max;
> +  double     * magnitudes;	/* [(dft_size / 2) + 1] */
>    float      * dBfs;
>  } priv_t;
>
> @@ -114,9 +120,9 @@ static int getopts(sox_effect_t * effp, int argc, char **argv)
>
>    while ((c = lsx_getopt(&optstate)) != -1) switch (c) {
>      GETOPT_NUMERIC(optstate, 'x', x_size0       , 100, MAX_X_SIZE)
> -    GETOPT_NUMERIC(optstate, 'X', pixels_per_sec,  1 , 5000)
> -    GETOPT_NUMERIC(optstate, 'y', y_size        , 64 , 1200)
> -    GETOPT_NUMERIC(optstate, 'Y', Y_size        , 130, MAX_FFT_SIZE / 2 + 2)
> +    GETOPT_NUMERIC(optstate, 'X', pixels_per_sec,  1 , MAX_X_SIZE)
> +    GETOPT_NUMERIC(optstate, 'y', y_size        , 64 , MAX_Y_SIZE)
> +    GETOPT_NUMERIC(optstate, 'Y', Y_size        , 130, MAX_Y_SIZE)
>      GETOPT_NUMERIC(optstate, 'z', dB_range      , 20 , 180)
>      GETOPT_NUMERIC(optstate, 'Z', gain          ,-100, 100)
>      GETOPT_NUMERIC(optstate, 'q', spectrum_points, 0 , p->spectrum_points)
> @@ -172,7 +178,7 @@ static double make_window(priv_t * p, int end)
>    double sum = 0, * w = end < 0? p->window : p->window + end;
>    int i, n = 1 + p->dft_size - abs(end);
>
> -  if (end) memset(p->window, 0, sizeof(p->window));
> +  if (end) memset(p->window, 0, sizeof(*(p->window)) * p->dft_size);

This memset is short by one element.

>    for (i = 0; i < n; ++i) w[i] = 1;
>    switch (p->win_type) {
>      case Window_Hann: lsx_apply_hann(w, n); break;
> @@ -190,10 +196,10 @@ static double make_window(priv_t * p, int end)
>    return sum;
>  }
>
> -static double * rdft_init(int n)
> +static double * rdft_init(size_t n)
>  {
>    double * q = lsx_malloc(2 * (n / 2 + 1) * n * sizeof(*q)), * p = q;
> -  int i, j;
> +  size_t i, j;
>    for (j = 0; j <= n / 2; ++j) for (i = 0; i < n; ++i)
>      *p++ = cos(2 * M_PI * j * i / n), *p++ = sin(2 * M_PI * j * i / n);
>    return q;
> @@ -260,11 +266,18 @@ static int start(sox_effect_t * effp)
>    if (p->y_size) {
>      p->dft_size = 2 * (p->y_size - 1);
>      if (!is_p2(p->dft_size) && !effp->flow)
> -      p->shared = rdft_init(p->dft_size);
> +      p->shared = rdft_init((size_t)(p->dft_size));
>    } else {
>     int y = max(32, (p->Y_size? p->Y_size : 550) / effp->in_signal.channels - 2);
>     for (p->dft_size = 128; p->dft_size <= y; p->dft_size <<= 1);
>    }
> +
> +  /* Now that dft_size is set, allocate variable-sized elements of priv_t */
> +  p->buf	= lsx_calloc(p->dft_size, sizeof(*(p->buf)));
> +  p->dft_buf	= lsx_calloc(p->dft_size, sizeof(*(p->dft_buf)));
> +  p->window	= lsx_calloc(p->dft_size + 1, sizeof(*(p->window)));
> +  p->magnitudes = lsx_calloc((p->dft_size / 2) + 1, sizeof(*(p->magnitudes)));
> +
>    if (is_p2(p->dft_size) && !effp->flow)
>      lsx_safe_rdft(p->dft_size, 1, p->dft_buf);
>    lsx_debug("duration=%g x_size=%i pixels_per_sec=%g dft_size=%i", duration, p->x_size, pixels_per_sec, p->dft_size);
> @@ -651,6 +664,10 @@ error: png_destroy_write_struct(&png, &png_info);
>    free(png_rows);
>    free(pixels);
>    free(p->dBfs);
> +  free(p->buf);
> +  free(p->dft_buf);
> +  free(p->window);
> +  free(p->magnitudes);
>    return SOX_SUCCESS;
>  }

-- 
Måns Rullgård


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel

Re: [PATCH RESEND 0/9] some old accumulated patches

[Eric Wong]

Måns Rullgård <mans@mansr.com> wrote:
> Eric Wong <normalperson@yhbt.net> writes:
> 
> > Hi Måns, I guess you're merging stuff these days?
> >
> > I still use sox every day, but I've mostly stopped hacking C
> > since gcc and clang takes too long to compile.
> 
> Get a faster computer.  AMD makes some nice ones.

Yeah, I hear nice things.  But over the past few decades, I've
also become totally disillusioned with software getting more
bloated and complex and HW getting outdated every few years.

That and privacy + software freedom problems with proprietary
blobs (PSP, ME); along with a never-ending stream of CPU bugs
leading to performance regressions :<

Money's not exactly flowing these days, either...

> > Here's a bunch of non-critical patches I had queued up from years ago.
> > (I mailed separately about the wavpack issue).
> 
> Thanks.  I'll check them over and push them unless I spot any problems.

Thanks for catching some of my embarrassing oversights :x


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel

Re: [PATCH RESEND 0/9] some old accumulated patches

[Måns Rullgård]

Eric Wong <normalperson@yhbt.net> writes:

> Måns Rullgård <mans@mansr.com> wrote:
>> Eric Wong <normalperson@yhbt.net> writes:
>> 
>> > Hi Måns, I guess you're merging stuff these days?
>> >
>> > I still use sox every day, but I've mostly stopped hacking C
>> > since gcc and clang takes too long to compile.
>> 
>> Get a faster computer.  AMD makes some nice ones.
>
> Yeah, I hear nice things.  But over the past few decades, I've
> also become totally disillusioned with software getting more
> bloated and complex and HW getting outdated every few years.
>
> That and privacy + software freedom problems with proprietary
> blobs (PSP, ME); along with a never-ending stream of CPU bugs
> leading to performance regressions :<

I got myself a 16-core EPYC recently as an upgrade for the 10-year-old
Xeon.  It's about 3-4 times faster at the things I care about while
using less power.

> Money's not exactly flowing these days, either...

Yeah, that can be a problem.

>> > Here's a bunch of non-critical patches I had queued up from years ago.
>> > (I mailed separately about the wavpack issue).
>> 
>> Thanks.  I'll check them over and push them unless I spot any problems.
>
> Thanks for catching some of my embarrassing oversights :x

That's what code review is for.

-- 
Måns Rullgård


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel

Re: [PATCH RESEND 9/9] Added average power spectrum for stat -freq -a

[Måns Rullgård]

Eric Wong <normalperson@yhbt.net> writes:

> From: Pander <pander@users.sourceforge.net>

Does "Pander" have a real name?

> references:
> https://sourceforge.net/p/sox/mailman/message/33186778/
> https://sourceforge.net/p/sox/mailman/message/33175940/
> https://sourceforge.net/p/sox/mailman/message/33625255/
>
> Edited-by: Eric Wong <normalperson@yhbt.net>
>
> [ew: removed extraneous whitespace changes,
>  squashed re_average initialization fix from Pander,
>  removed unnecessary check before free(3) call
>  (ref: <5498A2F9.2010500@free.fr>),
>  converted fft_average to bool]
> ---
>  sox.1      |  6 ++++++
>  src/stat.c | 27 ++++++++++++++++++++++++++-
>  2 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/sox.1 b/sox.1
> index 04ab09b1..d64762af 100644
> --- a/sox.1
> +++ b/sox.1
> @@ -3596,6 +3596,12 @@ audio in SoX's internal buffer.
>  This is mainly used to help track down endian problems that
>  sometimes occur in cross-platform versions of SoX.
>  .SP
> +The
> +.B \-a
> +option
> +will output the average input's power spectrum instead of the default
> +behavior in which the power spectrum is given for each 4096 point DFT.
> +.SP
>  See also the
>  .B stats
>  effect.
> diff --git a/src/stat.c b/src/stat.c
> index cdea27cc..0ca6c232 100644
> --- a/src/stat.c
> +++ b/src/stat.c
> @@ -34,6 +34,7 @@ typedef struct {
>    float *re_out;
>    unsigned long fft_size;
>    unsigned long fft_offset;
> +  sox_bool fft_average;
>  } priv_t;
>
> @@ -69,6 +70,8 @@ static int sox_stat_getopts(sox_effect_t * effp, int argc, char **argv)
>        stat->fft = 1;
>      else if (!(strcmp(*argv, "-d")))
>        stat->volume = 2;
> +    else if (!(strcmp(*argv, "-a")))
> +      stat->fft_average = sox_true;
>      else {
>        lsx_fail("Summary effect: unknown option");
>        return SOX_EOF;
> @@ -100,6 +103,7 @@ static int sox_stat_start(sox_effect_t * effp)
>      stat->bin[i] = 0;
>
>    stat->fft_size = 4096;
> +  stat->fft_average = sox_false;
>    stat->re_in = stat->re_out = NULL;
>
>    if (stat->fft) {
> @@ -134,6 +138,16 @@ static int sox_stat_flow(sox_effect_t * effp, const sox_sample_t *ibuf, sox_samp
>    priv_t * stat = (priv_t *) effp->priv;
>    int done, x, len = min(*isamp, *osamp);
>    short count = 0;
> +  float *re_average = NULL;
> +  unsigned samples = 0;
> +  float ffa = 0.0;
> +  unsigned i;
> +
> +  if (stat->fft_average) {
> +      samples = (stat->fft_size / 2);
> +      ffa = effp->in_signal.rate / samples;
> +      re_average = lsx_malloc(sizeof(float) * (int)samples);
> +  }
>
>    if (len) {
>      if (stat->read == 0)          /* 1st sample */
> @@ -146,10 +160,20 @@ static int sox_stat_flow(sox_effect_t * effp, const sox_sample_t *ibuf, sox_samp
>
>          if (stat->fft_offset >= stat->fft_size) {
>            stat->fft_offset = 0;
> +          if (stat->fft_average) {
> +              lsx_power_spectrum_f((int)samples, stat->re_in, stat->re_out);
> +              for (i = 0; i < samples / 2; i++) /* FIXME: should be <= samples / 2 */

What's this about?  If it should be <=, why isn't it?

> +                  re_average[i] += stat->re_out[i];

re_average is never initialised.  This will produce garbage.

> +          } else {
>            print_power_spectrum((unsigned) stat->fft_size, effp->in_signal.rate, stat->re_in, stat->re_out);
> +          }
>          }
>
>        }
> +      if (stat->fft_average) {
> +          for (i = 0; i < samples / 2; i++) /* FIXME: should be <= samples / 2 */
> +              fprintf(stderr, " %f  %f\n", ffa * i, re_average[i] / len);
> +      }

I don't understand the idea here.  Unless I'm mistaken, this prints the
power spectrum averaged over however many FFT blocks happen to fit in
the input chunk, which could be none.

>      }
>
>      for (done = 0; done < len; done++) {
> @@ -192,6 +216,7 @@ static int sox_stat_flow(sox_effect_t * effp, const sox_sample_t *ibuf, sox_samp
>      stat->read += len;
>    }
>
> +  free(re_average);
>    *isamp = *osamp = len;
>    /* Process all samples */
>
> @@ -320,7 +345,7 @@ static int sox_stat_stop(sox_effect_t * effp)
>
>  static sox_effect_handler_t sox_stat_effect = {
>    "stat",
> -  "[ -s N ] [ -rms ] [-freq] [ -v ] [ -d ]",
> +  "[ -s N ] [ -rms ] [-freq] [ -v ] [ -d ] [ -a ]",
>    SOX_EFF_MCHAN | SOX_EFF_MODIFY,
>    sox_stat_getopts,
>    sox_stat_start,

I'm skipping this patch unless someone can explain what the intent is.
Then I might try to fix it.

-- 
Måns Rullgård


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel

Re: [PATCH RESEND 0/9] some old accumulated patches

[Måns Rullgård]

Eric Wong <normalperson@yhbt.net> writes:

> Hi Måns, I guess you're merging stuff these days?
>
> I still use sox every day, but I've mostly stopped hacking C
> since gcc and clang takes too long to compile.  Here's a bunch
> of non-critical patches I had queued up from years ago.  (I
> mailed separately about the wavpack issue).
>
> In particular, I'm really happy with the "|program" speedup
> on Linux with F_SETPIPE_SZ and use it all the time.
>
> Anyways, thanks for keeping this project going!
>
> The following changes since commit 9f2d60f9cd52a1cc5ae92e02bd862c8cb4b85f13:
>
>   sox: silence gcc warning in headroom() function (2020-07-17 18:52:20 +0100)
>
> are available in the Git repository at:
>
>   https://80x24.org/sox.git for-mans_20200731
>
> for you to fetch changes up to f5f7a3f32ac2d03fde04ac7433514666dd4e0700:
>
>   Added average power spectrum for stat -freq -a (2020-07-31 09:14:53 +0000)
>
> ----------------------------------------------------------------
> Alexandre Ratchov (1):
>       sndio: handle 24-bit samples properly on OpenBSD
>
> Eric Wong (2):
>       use non-blocking stdin for interactive mode
>       speed up "|program" inputs on Linux 2.6.35+
>
> Guido Günther (1):
>       Handle vorbis_analysis_headerout errors
>
> Jan Stary (1):
>       sox.1: fix section name
>
> Martin Guy (2):
>       spectrogram: remove arbitrary limit on height of spectrogram
>       Add spectrogram -n flag to normalise the output to maximum brightness
>
> Pander (1):
>       Added average power spectrum for stat -freq -a
>
> gabor.karsay@gmx.at (1):
>       fix manpage warning: "table wider than line width"

I have pushed (with some minor fixes) all except the 'stat' patch.  That
one doesn't make sense, as detailed in a separate reply.

-- 
Måns Rullgård


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel

Re: [PATCH RESEND 9/9] Added average power spectrum for stat -freq -a

[Eric Wong]

Måns Rullgård <mans@mansr.com> wrote:
> Eric Wong <normalperson@yhbt.net> writes:
> 
> > From: Pander <pander@users.sourceforge.net>
> 
> Does "Pander" have a real name?

Does it matter for this project?

Fwiw, I'm against real name policies; and there's no
copyright assignment or DCO here.

I don't see an acceptable way to enforce such a thing.

Speaking for myself, I have too much social anxiety to attend
keysigning parties pre-COVID; and refuse to video conference
because of facial/voice recognition privacy issues.  Proving
my real name isn't feasible.

<snip>

> > +  if (stat->fft_average) {
> > +      samples = (stat->fft_size / 2);
> > +      ffa = effp->in_signal.rate / samples;
> > +      re_average = lsx_malloc(sizeof(float) * (int)samples);
> > +  }
> >
> >    if (len) {
> >      if (stat->read == 0)          /* 1st sample */
> > @@ -146,10 +160,20 @@ static int sox_stat_flow(sox_effect_t * effp, const sox_sample_t *ibuf, sox_samp
> >
> >          if (stat->fft_offset >= stat->fft_size) {
> >            stat->fft_offset = 0;
> > +          if (stat->fft_average) {
> > +              lsx_power_spectrum_f((int)samples, stat->re_in, stat->re_out);
> > +              for (i = 0; i < samples / 2; i++) /* FIXME: should be <= samples / 2 */
> 
> What's this about?  If it should be <=, why isn't it?
> 
> > +                  re_average[i] += stat->re_out[i];
> 
> re_average is never initialised.  This will produce garbage.

<snip>

> I'm skipping this patch unless someone can explain what the intent is.
> Then I might try to fix it.

I do agree this patch is buggy and I don't have the math knowledge
to comment on that part.  But I should've caught the uninitialized
memory, at least :x


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel

Re: [PATCH RESEND 9/9] Added average power spectrum for stat -freq -a

[Måns Rullgård]

Eric Wong <normalperson@yhbt.net> writes:

> Måns Rullgård <mans@mansr.com> wrote:
>> Eric Wong <normalperson@yhbt.net> writes:
>> 
>> > From: Pander <pander@users.sourceforge.net>
>> 
>> Does "Pander" have a real name?
>
> Does it matter for this project?
>
> Fwiw, I'm against real name policies; and there's no
> copyright assignment or DCO here.
>
> I don't see an acceptable way to enforce such a thing.
>
> Speaking for myself, I have too much social anxiety to attend
> keysigning parties pre-COVID; and refuse to video conference
> because of facial/voice recognition privacy issues.  Proving
> my real name isn't feasible.

I'm not asking anyone to prove their identity, only that they don't
actively hide it.  It might matter some time.  The Linux kernel has
the same rule.

-- 
Måns Rullgård


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel

Re: [PATCH RESEND 9/9] Added average power spectrum for stat -freq -a

[Måns Rullgård]

Måns Rullgård <mans@mansr.com> writes:

>> +      if (stat->fft_average) {
>> +          for (i = 0; i < samples / 2; i++) /* FIXME: should be <= samples / 2 */
>> +              fprintf(stderr, " %f  %f\n", ffa * i, re_average[i] / len);
>> +      }
>
> I don't understand the idea here.  Unless I'm mistaken, this prints the
> power spectrum averaged over however many FFT blocks happen to fit in
> the input chunk, which could be none.

Oh, and the "average" it calculates is bonkers too.

-- 
Måns Rullgård


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel

Re: [PATCH RESEND 9/9] Added average power spectrum for stat -freq -a

[Pander via SoX-devel]

On 8/2/20 12:52, Måns Rullgård wrote:
> Måns Rullgård <mans@mansr.com> writes:
>
>>> +      if (stat->fft_average) {
>>> +          for (i = 0; i < samples / 2; i++) /* FIXME: should be <= samples / 2 */
>>> +              fprintf(stderr, " %f  %f\n", ffa * i, re_average[i] / len);
>>> +      }
>> I don't understand the idea here.  Unless I'm mistaken, this prints the
>> power spectrum averaged over however many FFT blocks happen to fit in
>> the input chunk, which could be none.
> Oh, and the "average" it calculates is bonkers too.
It provides an average of only full blocks, as partly filled blocks mess
up the average. This is still in development and I plan to continue with
it soon, especially providing a test set demonstrating its value. Can
you please elaborate on what you mean with bonkers here?
>


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel

Re: [PATCH RESEND 9/9] Added average power spectrum for stat -freq -a

[Måns Rullgård]

Pander <pander@users.sourceforge.net> writes:

> On 8/2/20 12:52, Måns Rullgård wrote:
>> Måns Rullgård <mans@mansr.com> writes:
>>
>>>> +      if (stat->fft_average) {
>>>> +          for (i = 0; i < samples / 2; i++) /* FIXME: should be <= samples / 2 */
>>>> +              fprintf(stderr, " %f  %f\n", ffa * i, re_average[i] / len);
>>>> +      }
>>> I don't understand the idea here.  Unless I'm mistaken, this prints the
>>> power spectrum averaged over however many FFT blocks happen to fit in
>>> the input chunk, which could be none.
>> Oh, and the "average" it calculates is bonkers too.
> It provides an average of only full blocks, as partly filled blocks mess
> up the average.

Yes, of course, but I can't see the value of printing the average (even
if calculated correctly) of some unspecified number of blocks.  If audio
data is supplied in small chucks, most calls won't fill even one FFT
block, so you'll be printing a lot of zeros.

> This is still in development and I plan to continue with it soon,
> especially providing a test set demonstrating its value. Can you
> please elaborate on what you mean with bonkers here?

You're dividing the spectrum by the number of samples processed.  That
doesn't make sense.

-- 
Måns Rullgård


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel

Re: [PATCH RESEND 7/9] spectrogram: remove arbitrary limit on height of spectrogram

[Måns Rullgård]

Martin Guy <martinwguy@gmail.com> writes:

> On 31/07/2020, Måns Rullgård <mans@mansr.com> wrote:
>>> -  if (end) memset(p->window, 0, sizeof(p->window));
>>> +  if (end) memset(p->window, 0, sizeof(*(p->window)) * p->dft_size);
>>
>> This memset is short by one element.
>
> It is. Many thanks. Do I need to do anything to action this?

No, I fixed that and a couple of other problems.  It's in the git master
branch now.

-- 
Måns Rullgård


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel

Re: [PATCH RESEND 7/9] spectrogram: remove arbitrary limit on height of spectrogram

[Pander via SoX-devel]

On 8/13/20 18:20, Måns Rullgård wrote:
> Martin Guy <martinwguy@gmail.com> writes:
>
>> On 31/07/2020, Måns Rullgård <mans@mansr.com> wrote:
>>>> -  if (end) memset(p->window, 0, sizeof(p->window));
>>>> +  if (end) memset(p->window, 0, sizeof(*(p->window)) * p->dft_size);
>>> This memset is short by one element.
>> It is. Many thanks. Do I need to do anything to action this?
> No, I fixed that and a couple of other problems.  It's in the git master
> branch now.
>
Thanks for this. Slightly related question: the x-axis is in seconds,
can an option be added to get in hh:mm:ss and other date time formats?
This will probably also have influence on the interval of the number of
labels on the x axis.


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel

Re: [PATCH RESEND 7/9] spectrogram: remove arbitrary limit on height of spectrogram

[Måns Rullgård]

Pander via SoX-devel <sox-devel@lists.sourceforge.net> writes:

> On 8/13/20 18:20, Måns Rullgård wrote:
>> Martin Guy <martinwguy@gmail.com> writes:
>>
>>> On 31/07/2020, Måns Rullgård <mans@mansr.com> wrote:
>>>>> -  if (end) memset(p->window, 0, sizeof(p->window));
>>>>> +  if (end) memset(p->window, 0, sizeof(*(p->window)) * p->dft_size);
>>>> This memset is short by one element.
>>> It is. Many thanks. Do I need to do anything to action this?
>> No, I fixed that and a couple of other problems.  It's in the git master
>> branch now.
>>
> Thanks for this. Slightly related question: the x-axis is in seconds,
> can an option be added to get in hh:mm:ss and other date time formats?
> This will probably also have influence on the interval of the number of
> labels on the x axis.

I suppose it could be done.

-- 
Måns Rullgård


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel