[SoX-devel] Follow up on Average power spectrum path

[Pander]

Hi all,

Because of a question on the users list, I would like to ask how to move
forward the following patch and documentation?

Average power spectrum patch:
https://sourceforge.net/p/sox/mailman/message/33186778/
and
https://sourceforge.net/p/sox/mailman/message/33175940/

Extra documentation:
https://sourceforge.net/p/sox/mailman/message/33625255/

Please let me know how I can help to move this forward.

Thanks,

Pander

------------------------------------------------------------------------------

[Eric Wong]

Post by Pander
Because of a question on the users list, I would like to ask how to move
forward the following patch and documentation?

I guess the real sox developers are still busy...
I suppose I'll start publishing things I've reviewed/tested
and push things I've reviewed to branches my server (see below)[1]
so it'll hopefully be easier for them when they return.

I'm not really qualified to review the maths parts of this;
as I am only a simple Unix/C plumber and I already commented
some last year on this patch...

I wonder if "-a" should imply "-freq", since it's "-a"
is a no-op without "-freq".

And is this option is intended to be the average of each hunk in the
flow and not for the entire file? In other words, it outputs a LOT of
lines with 2 floating point numbers in them when I run:
"sox foo.flac -n stat -freq -a" on an arbitrary file.

Most of the normal "stat" output is printed in the sox_stat_stop
function instead of sox_stat_flow; the lone exception being '-d'
which seems mainly for troubleshooting.

Also, did you intend fft_average to be extended for non-boolean
purposes?

I don't mind using "int" as a bool for other pre-C99 projects, but the
"== 1" checks in conditionals looked a bit odd to me and sox already
defines a sox_bool type, so I figure squashing in the following would
improve readability:

diff --git a/src/stat.c b/src/stat.c
index 63d1741..89bed55 100644
--- a/src/stat.c
+++ b/src/stat.c
@@ -34,7 +34,7 @@ typedef struct {
float *re_out;
unsigned long fft_size;
unsigned long fft_offset;
- int fft_average;
+ sox_bool fft_average;
} priv_t;


@@ -71,7 +71,7 @@ static int sox_stat_getopts(sox_effect_t * effp, int argc, char **argv)
else if (!(strcmp(*argv, "-d")))
stat->volume = 2;
else if (!(strcmp(*argv, "-a")))
- stat->fft_average = 1;
+ stat->fft_average = sox_true;
else {
lsx_fail("Summary effect: unknown option");
return SOX_EOF;
@@ -103,7 +103,7 @@ static int sox_stat_start(sox_effect_t * effp)
stat->bin[i] = 0;

stat->fft_size = 4096;
- stat->fft_average = 0;
+ stat->fft_average = sox_false;
stat->re_in = stat->re_out = NULL;

if (stat->fft) {
@@ -142,7 +142,8 @@ static int sox_stat_flow(sox_effect_t * effp, const sox_sample_t *ibuf, sox_samp
unsigned samples = 0;
float ffa = 0.0;
unsigned i;
- if (stat->fft_average == 1) {
+
+ if (stat->fft_average) {
samples = (stat->fft_size / 2);
ffa = effp->in_signal.rate / samples;
re_average = lsx_malloc(sizeof(float) * (int)samples);
@@ -159,7 +160,7 @@ 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 == 1) {
+ 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];
@@ -169,7 +170,7 @@ static int sox_stat_flow(sox_effect_t * effp, const sox_sample_t *ibuf, sox_samp
}

}
- if (stat->fft_average == 1) {
+ 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);
}

---
[1] you can add my repo to your existing sox repository:
git remote add 80x24 git://80x24.org/sox.git
git fetch 80x24
(or a web viewer if you prefer: http://bogomips.org/sox.git )
These are work-in-progress dev branches, so I will squash
and rebase branches as needed with forced pushes.

------------------------------------------------------------------------------

[Måns Rullgård]

Post by Eric Wong

Post by Pander
Because of a question on the users list, I would like to ask how to move
forward the following patch and documentation?

I guess the real sox developers are still busy...

Or missing.

[Pander]

Post by Eric Wong

Post by Pander
Because of a question on the users list, I would like to ask how to move
forward the following patch and documentation?

I guess the real sox developers are still busy...
I suppose I'll start publishing things I've reviewed/tested
and push things I've reviewed to branches my server (see below)[1]
so it'll hopefully be easier for them when they return.
I'm not really qualified to review the maths parts of this;
as I am only a simple Unix/C plumber and I already commented
some last year on this patch...
I wonder if "-a" should imply "-freq", since it's "-a"
is a no-op without "-freq".

It is only allowed/used after -freq in this context.

After gaining some more experience with sox this year, I have learned
that calculating the average power spectrum is not straightforward. I
have also posted about this to one of the sox mailing lists where I
illustrated the problem with some graphs.

Only when the last sample is filled completely, an average can be
calculated over all samples. Usually, this is not the case and should
the average be calculated over over all samples except the last one.

The safest way to calculate the average, is to always omit the last
sample. However, it is more correct to include the last sample, but only
when can be detected that the last sample is filled exactly as all the
other samples are. If it is only partially filled, it (highly probably)
will distort the average too much.

Since I only work with a lot of samples, for me you can always omit the
last one. But if someone could let you know how to detect that the last
samples is used to the full and can be included, please do so.

Of course, when omitting the last sample, divide by the number of
samples - 1. Either way, how this is implemented, it should be
documented clearly when the last sample is omitted.

Post by Eric Wong
And is this option is intended to be the average of each hunk in the
flow and not for the entire file? In other words, it outputs a LOT of
"sox foo.flac -n stat -freq -a" on an arbitrary file.
Most of the normal "stat" output is printed in the sox_stat_stop
function instead of sox_stat_flow; the lone exception being '-d'
which seems mainly for troubleshooting.
Also, did you intend fft_average to be extended for non-boolean
purposes?

Initially not, but see above. It could be desirable to offer different
options:
nothing: do as now
-a average over all samples
-ao average over all samples but omit last sample
-as average over all samples but omit last sample only when needed
(often needed because the last sample is partially filled)

Post by Eric Wong
I don't mind using "int" as a bool for other pre-C99 projects, but the
"== 1" checks in conditionals looked a bit odd to me and sox already
defines a sox_bool type, so I figure squashing in the following would
diff --git a/src/stat.c b/src/stat.c
index 63d1741..89bed55 100644
--- a/src/stat.c
+++ b/src/stat.c
@@ -34,7 +34,7 @@ typedef struct {
float *re_out;
unsigned long fft_size;
unsigned long fft_offset;
- int fft_average;
+ sox_bool fft_average;
} priv_t;
@@ -71,7 +71,7 @@ static int sox_stat_getopts(sox_effect_t * effp, int argc, char **argv)
else if (!(strcmp(*argv, "-d")))
stat->volume = 2;
else if (!(strcmp(*argv, "-a")))
- stat->fft_average = 1;
+ stat->fft_average = sox_true;
else {
lsx_fail("Summary effect: unknown option");
return SOX_EOF;
@@ -103,7 +103,7 @@ static int sox_stat_start(sox_effect_t * effp)
stat->bin[i] = 0;
stat->fft_size = 4096;
- stat->fft_average = 0;
+ stat->fft_average = sox_false;
stat->re_in = stat->re_out = NULL;
if (stat->fft) {
@@ -142,7 +142,8 @@ static int sox_stat_flow(sox_effect_t * effp, const sox_sample_t *ibuf, sox_samp
unsigned samples = 0;
float ffa = 0.0;
unsigned i;
- if (stat->fft_average == 1) {
+
+ if (stat->fft_average) {
samples = (stat->fft_size / 2);
ffa = effp->in_signal.rate / samples;
re_average = lsx_malloc(sizeof(float) * (int)samples);
@@ -159,7 +160,7 @@ 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 == 1) {
+ 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];
@@ -169,7 +170,7 @@ static int sox_stat_flow(sox_effect_t * effp, const sox_sample_t *ibuf, sox_samp
}
}
- if (stat->fft_average == 1) {
+ 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);
}
---
git remote add 80x24 git://80x24.org/sox.git
git fetch 80x24
(or a web viewer if you prefer: http://bogomips.org/sox.git )
These are work-in-progress dev branches, so I will squash
and rebase branches as needed with forced pushes.
------------------------------------------------------------------------------
_______________________________________________
SoX-devel mailing list
https://lists.sourceforge.net/lists/listinfo/sox-devel

------------------------------------------------------------------------------