*Follow up on Average power spectrum path@ 2015-12-17 14:21 Pander2015-12-19 11:12 ` Eric Wong 0 siblings, 1 reply; 4+ messages in thread From: Pander @ 2015-12-17 14:21 UTC (permalink / raw) To: sox-devel 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 ------------------------------------------------------------------------------ ^ permalink raw reply [flat|nested] 4+ messages in thread

*Re: Follow up on Average power spectrum path2015-12-17 14:21 Follow up on Average power spectrum path Pander@ 2015-12-19 11:12 ` Eric Wong2015-12-19 11:20 ` Måns Rullgård 2015-12-28 17:38 ` Pander 0 siblings, 2 replies; 4+ messages in thread From: Eric Wong @ 2015-12-19 11:12 UTC (permalink / raw) To: sox-devel Pander <pander@users.sourceforge.net> wrote: > 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. ------------------------------------------------------------------------------ ^ permalink raw reply related [flat|nested] 4+ messages in thread

*Re: Follow up on Average power spectrum path2015-12-19 11:12 ` Eric Wong@ 2015-12-19 11:20 ` Måns Rullgård2015-12-28 17:38 ` Pander 1 sibling, 0 replies; 4+ messages in thread From: Måns Rullgård @ 2015-12-19 11:20 UTC (permalink / raw) To: Eric Wong;+Cc:sox-devel Eric Wong <normalperson@yhbt.net> writes: > Pander <pander@users.sourceforge.net> wrote: >> 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. -- Måns Rullgård ------------------------------------------------------------------------------ ^ permalink raw reply [flat|nested] 4+ messages in thread

*2015-12-19 11:12 ` Eric Wong 2015-12-19 11:20 ` Måns RullgårdRe: Follow up on Average power spectrum path@ 2015-12-28 17:38 ` Pander1 sibling, 0 replies; 4+ messages in thread From: Pander @ 2015-12-28 17:38 UTC (permalink / raw) To: sox-devel On 12/19/2015 12:12 PM, Eric Wong wrote: > Pander <pander@users.sourceforge.net> wrote: >> 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. > > 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? 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) > > 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. > > ------------------------------------------------------------------------------ > _______________________________________________ > SoX-devel mailing list > SoX-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/sox-devel > ------------------------------------------------------------------------------ ^ permalink raw reply [flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-12-28 17:39 UTC | newest]Thread overview:4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-12-17 14:21 Follow up on Average power spectrum path Pander 2015-12-19 11:12 ` Eric Wong 2015-12-19 11:20 ` Måns Rullgård 2015-12-28 17:38 ` Pander

Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/sox.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).