* [PATCH RESEND 1/9] use non-blocking stdin for interactive mode
2020-07-31 9:37 [PATCH RESEND 0/9] some old accumulated patches Eric Wong
@ 2020-07-31 9:37 ` Eric Wong
2020-07-31 9:37 ` [PATCH RESEND 2/9] speed up "|program" inputs on Linux 2.6.35+ Eric Wong
` (9 subsequent siblings)
10 siblings, 0 replies; 26+ messages in thread
From: Eric Wong @ 2020-07-31 9:37 UTC (permalink / raw)
To: Måns Rullgård; +Cc: Eric Wong, sox-devel
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
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH RESEND 2/9] speed up "|program" inputs on Linux 2.6.35+
2020-07-31 9:37 [PATCH RESEND 0/9] some old accumulated patches Eric Wong
2020-07-31 9:37 ` [PATCH RESEND 1/9] use non-blocking stdin for interactive mode Eric Wong
@ 2020-07-31 9:37 ` Eric Wong
2020-07-31 10:53 ` Måns Rullgård
2020-07-31 9:37 ` [PATCH RESEND 3/9] sox.1: fix section name Eric Wong
` (8 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: Eric Wong @ 2020-07-31 9:37 UTC (permalink / raw)
To: Måns Rullgård; +Cc: sox-devel
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
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH RESEND 2/9] speed up "|program" inputs on Linux 2.6.35+
2020-07-31 9:37 ` [PATCH RESEND 2/9] speed up "|program" inputs on Linux 2.6.35+ Eric Wong
@ 2020-07-31 10:53 ` Måns Rullgård
0 siblings, 0 replies; 26+ messages in thread
From: Måns Rullgård @ 2020-07-31 10:53 UTC (permalink / raw)
To: Eric Wong; +Cc: sox-devel
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
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH RESEND 3/9] sox.1: fix section name
2020-07-31 9:37 [PATCH RESEND 0/9] some old accumulated patches Eric Wong
2020-07-31 9:37 ` [PATCH RESEND 1/9] use non-blocking stdin for interactive mode Eric Wong
2020-07-31 9:37 ` [PATCH RESEND 2/9] speed up "|program" inputs on Linux 2.6.35+ Eric Wong
@ 2020-07-31 9:37 ` Eric Wong
2020-07-31 9:37 ` [PATCH RESEND 4/9] sndio: handle 24-bit samples properly on OpenBSD Eric Wong
` (7 subsequent siblings)
10 siblings, 0 replies; 26+ messages in thread
From: Eric Wong @ 2020-07-31 9:37 UTC (permalink / raw)
To: Måns Rullgård; +Cc: sox-devel
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
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH RESEND 4/9] sndio: handle 24-bit samples properly on OpenBSD
2020-07-31 9:37 [PATCH RESEND 0/9] some old accumulated patches Eric Wong
` (2 preceding siblings ...)
2020-07-31 9:37 ` [PATCH RESEND 3/9] sox.1: fix section name Eric Wong
@ 2020-07-31 9:37 ` Eric Wong
2020-07-31 10:59 ` Måns Rullgård
2020-07-31 9:37 ` [PATCH RESEND 5/9] Handle vorbis_analysis_headerout errors Eric Wong
` (6 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: Eric Wong @ 2020-07-31 9:37 UTC (permalink / raw)
To: Måns Rullgård; +Cc: sox-devel, Alexandre Ratchov
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
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH RESEND 4/9] sndio: handle 24-bit samples properly on OpenBSD
2020-07-31 9:37 ` [PATCH RESEND 4/9] sndio: handle 24-bit samples properly on OpenBSD Eric Wong
@ 2020-07-31 10:59 ` Måns Rullgård
0 siblings, 0 replies; 26+ messages in thread
From: Måns Rullgård @ 2020-07-31 10:59 UTC (permalink / raw)
To: Eric Wong; +Cc: sox-devel, Alexandre Ratchov
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
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH RESEND 5/9] Handle vorbis_analysis_headerout errors
2020-07-31 9:37 [PATCH RESEND 0/9] some old accumulated patches Eric Wong
` (3 preceding siblings ...)
2020-07-31 9:37 ` [PATCH RESEND 4/9] sndio: handle 24-bit samples properly on OpenBSD Eric Wong
@ 2020-07-31 9:37 ` Eric Wong
2020-07-31 9:37 ` [PATCH RESEND 6/9] fix manpage warning: "table wider than line width" Eric Wong
` (5 subsequent siblings)
10 siblings, 0 replies; 26+ messages in thread
From: Eric Wong @ 2020-07-31 9:37 UTC (permalink / raw)
To: Måns Rullgård; +Cc: Guido Günther, sox-devel
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
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH RESEND 6/9] fix manpage warning: "table wider than line width"
2020-07-31 9:37 [PATCH RESEND 0/9] some old accumulated patches Eric Wong
` (4 preceding siblings ...)
2020-07-31 9:37 ` [PATCH RESEND 5/9] Handle vorbis_analysis_headerout errors Eric Wong
@ 2020-07-31 9:37 ` Eric Wong
2020-07-31 9:37 ` [PATCH RESEND 7/9] spectrogram: remove arbitrary limit on height of spectrogram Eric Wong
` (4 subsequent siblings)
10 siblings, 0 replies; 26+ messages in thread
From: Eric Wong @ 2020-07-31 9:37 UTC (permalink / raw)
To: Måns Rullgård
Cc: gabor.karsay@gmx.at, Jaromír Mikeš, sox-devel
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
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH RESEND 7/9] spectrogram: remove arbitrary limit on height of spectrogram
2020-07-31 9:37 [PATCH RESEND 0/9] some old accumulated patches Eric Wong
` (5 preceding siblings ...)
2020-07-31 9:37 ` [PATCH RESEND 6/9] fix manpage warning: "table wider than line width" Eric Wong
@ 2020-07-31 9:37 ` Eric Wong
2020-07-31 14:34 ` Måns Rullgård
2020-07-31 9:38 ` [PATCH RESEND 8/9] Add spectrogram -n flag to normalise the output to maximum brightness Eric Wong
` (3 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: Eric Wong @ 2020-07-31 9:37 UTC (permalink / raw)
To: Måns Rullgård; +Cc: Martin Guy, sox-devel
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
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH RESEND 7/9] spectrogram: remove arbitrary limit on height of spectrogram
2020-07-31 9:37 ` [PATCH RESEND 7/9] spectrogram: remove arbitrary limit on height of spectrogram Eric Wong
@ 2020-07-31 14:34 ` Måns Rullgård
[not found] ` <CAL4-wQpF-qOD=BRVPhgZFC7fjvFDV-rQx1stvwY_xCTyj5uooA@mail.gmail.com>
0 siblings, 1 reply; 26+ messages in thread
From: Måns Rullgård @ 2020-07-31 14:34 UTC (permalink / raw)
To: Eric Wong; +Cc: Martin Guy, sox-devel
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
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH RESEND 8/9] Add spectrogram -n flag to normalise the output to maximum brightness
2020-07-31 9:37 [PATCH RESEND 0/9] some old accumulated patches Eric Wong
` (6 preceding siblings ...)
2020-07-31 9:37 ` [PATCH RESEND 7/9] spectrogram: remove arbitrary limit on height of spectrogram Eric Wong
@ 2020-07-31 9:38 ` Eric Wong
2020-07-31 9:38 ` [PATCH RESEND 9/9] Added average power spectrum for stat -freq -a Eric Wong
` (2 subsequent siblings)
10 siblings, 0 replies; 26+ messages in thread
From: Eric Wong @ 2020-07-31 9:38 UTC (permalink / raw)
To: Måns Rullgård; +Cc: Martin Guy, sox-devel
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
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH RESEND 9/9] Added average power spectrum for stat -freq -a
2020-07-31 9:37 [PATCH RESEND 0/9] some old accumulated patches Eric Wong
` (7 preceding siblings ...)
2020-07-31 9:38 ` [PATCH RESEND 8/9] Add spectrogram -n flag to normalise the output to maximum brightness Eric Wong
@ 2020-07-31 9:38 ` Eric Wong
2020-08-01 11:52 ` Måns Rullgård
2020-07-31 10:13 ` [PATCH RESEND 0/9] some old accumulated patches Måns Rullgård
2020-08-01 16:57 ` Måns Rullgård
10 siblings, 1 reply; 26+ messages in thread
From: Eric Wong @ 2020-07-31 9:38 UTC (permalink / raw)
To: Måns Rullgård; +Cc: Pander, sox-devel
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
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH RESEND 9/9] Added average power spectrum for stat -freq -a
2020-07-31 9:38 ` [PATCH RESEND 9/9] Added average power spectrum for stat -freq -a Eric Wong
@ 2020-08-01 11:52 ` Måns Rullgård
2020-08-02 1:03 ` Eric Wong
2020-08-02 10:52 ` Måns Rullgård
0 siblings, 2 replies; 26+ messages in thread
From: Måns Rullgård @ 2020-08-01 11:52 UTC (permalink / raw)
To: Eric Wong; +Cc: Pander, sox-devel
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
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH RESEND 9/9] Added average power spectrum for stat -freq -a
2020-08-01 11:52 ` Måns Rullgård
@ 2020-08-02 1:03 ` Eric Wong
2020-08-02 10:43 ` Måns Rullgård
2020-08-02 10:52 ` Måns Rullgård
1 sibling, 1 reply; 26+ messages in thread
From: Eric Wong @ 2020-08-02 1:03 UTC (permalink / raw)
To: Måns Rullgård; +Cc: Pander, sox-devel
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
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH RESEND 9/9] Added average power spectrum for stat -freq -a
2020-08-02 1:03 ` Eric Wong
@ 2020-08-02 10:43 ` Måns Rullgård
0 siblings, 0 replies; 26+ messages in thread
From: Måns Rullgård @ 2020-08-02 10:43 UTC (permalink / raw)
To: Eric Wong; +Cc: Pander, sox-devel
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
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH RESEND 9/9] Added average power spectrum for stat -freq -a
2020-08-01 11:52 ` Måns Rullgård
2020-08-02 1:03 ` Eric Wong
@ 2020-08-02 10:52 ` Måns Rullgård
2020-08-03 12:44 ` Pander via SoX-devel
1 sibling, 1 reply; 26+ messages in thread
From: Måns Rullgård @ 2020-08-02 10:52 UTC (permalink / raw)
To: Eric Wong; +Cc: Pander, sox-devel
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
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH RESEND 9/9] Added average power spectrum for stat -freq -a
2020-08-02 10:52 ` Måns Rullgård
@ 2020-08-03 12:44 ` Pander via SoX-devel
2020-08-03 13:41 ` Måns Rullgård
0 siblings, 1 reply; 26+ messages in thread
From: Pander via SoX-devel @ 2020-08-03 12:44 UTC (permalink / raw)
To: Måns Rullgård, Eric Wong; +Cc: Pander, 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
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH RESEND 9/9] Added average power spectrum for stat -freq -a
2020-08-03 12:44 ` Pander via SoX-devel
@ 2020-08-03 13:41 ` Måns Rullgård
0 siblings, 0 replies; 26+ messages in thread
From: Måns Rullgård @ 2020-08-03 13:41 UTC (permalink / raw)
To: Pander; +Cc: Eric Wong, sox-devel
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
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH RESEND 0/9] some old accumulated patches
2020-07-31 9:37 [PATCH RESEND 0/9] some old accumulated patches Eric Wong
` (8 preceding siblings ...)
2020-07-31 9:38 ` [PATCH RESEND 9/9] Added average power spectrum for stat -freq -a Eric Wong
@ 2020-07-31 10:13 ` Måns Rullgård
2020-07-31 21:16 ` Eric Wong
2020-08-01 16:57 ` Måns Rullgård
10 siblings, 1 reply; 26+ messages in thread
From: Måns Rullgård @ 2020-07-31 10:13 UTC (permalink / raw)
To: Eric Wong; +Cc: sox-devel
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
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH RESEND 0/9] some old accumulated patches
2020-07-31 10:13 ` [PATCH RESEND 0/9] some old accumulated patches Måns Rullgård
@ 2020-07-31 21:16 ` Eric Wong
2020-07-31 21:44 ` Måns Rullgård
0 siblings, 1 reply; 26+ messages in thread
From: Eric Wong @ 2020-07-31 21:16 UTC (permalink / raw)
To: sox-devel
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
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH RESEND 0/9] some old accumulated patches
2020-07-31 21:16 ` Eric Wong
@ 2020-07-31 21:44 ` Måns Rullgård
0 siblings, 0 replies; 26+ messages in thread
From: Måns Rullgård @ 2020-07-31 21:44 UTC (permalink / raw)
To: Eric Wong; +Cc: sox-devel
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
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH RESEND 0/9] some old accumulated patches
2020-07-31 9:37 [PATCH RESEND 0/9] some old accumulated patches Eric Wong
` (9 preceding siblings ...)
2020-07-31 10:13 ` [PATCH RESEND 0/9] some old accumulated patches Måns Rullgård
@ 2020-08-01 16:57 ` Måns Rullgård
10 siblings, 0 replies; 26+ messages in thread
From: Måns Rullgård @ 2020-08-01 16:57 UTC (permalink / raw)
To: Eric Wong; +Cc: sox-devel
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
^ permalink raw reply [flat|nested] 26+ messages in thread