Review patch for average power spectrum

Review patch for average power spectrum

[Pander]

[-- Attachment #1: Type: text/plain, Size: 215 bytes --]

Hi all,

Please review the attached patch for outputting the average power spectrum.

For many applications I need to calculate this with additional tools.
This patch will make SoX do it directly.

Regards,

Pander

[-- Attachment #2: 0001-Added-average-power-spectrum-for-stat-freq-a.patch --]
[-- Type: text/x-patch, Size: 6649 bytes --]

>From 261b4fb8cc9f8f98e0044e973a81e7a367349339 Mon Sep 17 00:00:00 2001
From: Pander <pander@users.sourceforge.net>
Date: Mon, 22 Dec 2014 22:45:34 +0100
Subject: [PATCH] Added average power spectrum for stat -freq -a

---
 sox.1      | 42 ++++++++++++++++++++++++------------------
 src/stat.c | 28 +++++++++++++++++++++++++++-
 2 files changed, 51 insertions(+), 19 deletions(-)

diff --git a/sox.1 b/sox.1
index 2c4ca47..30c1c8a 100644
--- a/sox.1
+++ b/sox.1
@@ -8,7 +8,7 @@
 ..
 '\"
 '\" Replacement em-dash for nroff (default is too short).
-.ie n .ds m " - 
+.ie n .ds m " -
 .el .ds m \(em
 '\"
 '\" Placeholder macro for if longer nroff arrow is needed.
@@ -1204,7 +1204,7 @@ SoX has support for GSM's original 13kbps `Full Rate' audio format.
 It is usually CPU-intensive to work with GSM audio.
 .RE
 .TP
-\ 
+\
 Encoding names can be abbreviated where this would not be ambiguous;
 e.g. `unsigned-integer' can be given as `un', but not `u' (ambiguous
 with `u-law').
@@ -1306,7 +1306,7 @@ for a list of supported file types.
 .if t .sp -.5
 .if n .sp -1
 .TP
-\ 
+\
 These options specify whether the byte-order of the audio data is,
 respectively, `little endian', `big endian', or the opposite to that of
 the system on which SoX is being used.  Endianness applies only to data
@@ -1489,9 +1489,9 @@ center;
 cI cI lI
 cB c l.
 \ 	Method	Notes
-h	Hz	\ 
-k	kHz	\ 
-o	Octaves	\ 
+h	Hz	\
+k	kHz	\
+o	Octaves	\
 q	Q-factor	See [2]
 .TE
 .DT
@@ -3301,7 +3301,7 @@ If `-' is given, the spectrogram will be sent to standard output
 (stdout).
 .RE
 .TP
-\ 
+\
 .I Advanced Options:
 .br
 In order to process a smaller section of audio without affecting other
@@ -3336,7 +3336,7 @@ creates a spectrogram showing all but the first minute of the audio
 (the output file, however, receives the entire audio stream).
 .RE
 .TP
-\ 
+\
 For the ability to perform off-line processing of spectral data, see the
 .B stat
 effect.
@@ -3412,12 +3412,12 @@ operation.  The effect simulates the diagonal cuts and joins the two pieces:
       length1   excess
     -----------><--->
     _________   :   :  _________________
-             \\  :   : :\\     `         
-              \\ :   : : \\     `        
-               \\:   : :  \\     `       
-                *   : :   * - - *      
-                 \\  : :   :\\     `     
-                  \\ : :   : \\     `    
+             \\  :   : :\\     `
+              \\ :   : : \\     `
+               \\:   : :  \\     `
+                *   : :   * - - *
+                 \\  : :   :\\     `
+                  \\ : :   : \\     `
     _______________\\: :   :  \\_____`____
                       :   :   :     :
                       <--->   <----->
@@ -3518,7 +3518,7 @@ as follows:
 .TS
 center;
 lI l l.
-Samples read	\fIn\fR\^\(mu\^\fIc\fR	\ 
+Samples read	\fIn\fR\^\(mu\^\fIc\fR	\
 Length (seconds)	\fIn\fR\^\(di\^\fIr\fR
 Scaled by	\ 	See \-s below.
 Maximum amplitude	max(\fIx\s-2\dk\u\s0\fR)	T{
@@ -3592,6 +3592,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.
@@ -3877,7 +3883,7 @@ Exponential: as `/', but initial phase always zero, and stepped (less
 smooth) frequency changes.
 .RE
 .TP
-\ 
+\
 Not used for noise.
 .SP
 \fIoff\fR is the bias (DC-offset) of the signal in percent; default=0.
@@ -4070,7 +4076,7 @@ The amount of audio (in seconds) to preserve before the trigger point
 and any found quieter/shorter bursts.
 .RE
 .TP
-\ 
+\
 .I Advanced Options:
 .br
 These allow fine tuning of the algorithm's internal parameters.
@@ -4109,7 +4115,7 @@ algorithm.
 algorithm.
 .RE
 .TP
-\ 
+\
 See also the
 .B silence
 effect.
diff --git a/src/stat.c b/src/stat.c
index cdea27c..63d1741 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;
+  int 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 = 1;
     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 = 0;
   stat->re_in = stat->re_out = NULL;
 
   if (stat->fft) {
@@ -134,6 +138,15 @@ 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;
+  unsigned samples = 0;
+  float ffa = 0.0;
+  unsigned i;
+  if (stat->fft_average == 1) {
+      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 +159,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 == 1) {
+              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 == 1) {
+          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 +215,9 @@ static int sox_stat_flow(sox_effect_t * effp, const sox_sample_t *ibuf, sox_samp
     stat->read += len;
   }
 
+  if (re_average) {
+    free(re_average);
+  }
   *isamp = *osamp = len;
   /* Process all samples */
 
@@ -320,7 +346,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,
-- 
2.1.0


[-- Attachment #3: Type: text/plain, Size: 440 bytes --]

------------------------------------------------------------------------------
Dive into the World of Parallel Programming! The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net

[-- Attachment #4: Type: text/plain, Size: 158 bytes --]

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

Re: Review patch for average power spectrum

[Eric Wong]

Pander <pander@users.sourceforge.net> wrote:
> Please review the attached patch for outputting the average power spectrum.

I am not knowledgable in the math behind this, but I noticed one
memory management error:

> @@ -134,6 +138,15 @@ 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;

re_average needs to be initialized for the "stat->fft_average != 1" case

  float *re_average = NULL;

> +  if (stat->fft_average == 1) {
> +      samples = (stat->fft_size / 2);
> +      ffa = effp->in_signal.rate / samples;
> +      re_average = lsx_malloc(sizeof(float) * (int)samples);
> +  }

...otherwise, free may be called on an uninitialized value:

> +  if (re_average) {
> +    free(re_average);
> +  }
>    *isamp = *osamp = len;
>    /* Process all samples */

There's also a bunch of unrelated whitespace eliminations
in the manpage which should probably be a separate commit
(but I'm not a sox maintainer, just another user + occasional patcher).

------------------------------------------------------------------------------
Dive into the World of Parallel Programming! The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net

Re: Review patch for average power spectrum

[Pander]

[-- Attachment #1: Type: text/plain, Size: 2107 bytes --]

On 12/22/2014 11:14 PM, Eric Wong wrote:
> Pander <pander@users.sourceforge.net> wrote:
>> Please review the attached patch for outputting the average power spectrum.
> 
> I am not knowledgable in the math behind this, but I noticed one
> memory management error:
> 
>> @@ -134,6 +138,15 @@ 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;
> 
> re_average needs to be initialized for the "stat->fft_average != 1" case
> 
>   float *re_average = NULL;

Thanks, that will also remove a warning.

Please see the attachement for the incremental patch.

The first patch also included some of the changes coming in with the
promoted git pull in http://sox.sourceforge.net/ but git can split them
in the patch for the maintainer.

> 
>> +  if (stat->fft_average == 1) {
>> +      samples = (stat->fft_size / 2);
>> +      ffa = effp->in_signal.rate / samples;
>> +      re_average = lsx_malloc(sizeof(float) * (int)samples);
>> +  }
> 
> ...otherwise, free may be called on an uninitialized value:
> 
>> +  if (re_average) {
>> +    free(re_average);
>> +  }
>>    *isamp = *osamp = len;
>>    /* Process all samples */
> 
> There's also a bunch of unrelated whitespace eliminations
> in the manpage which should probably be a separate commit
> (but I'm not a sox maintainer, just another user + occasional patcher).
> 
> ------------------------------------------------------------------------------
> Dive into the World of Parallel Programming! The Go Parallel Website,
> sponsored by Intel and developed in partnership with Slashdot Media, is your
> hub for all things parallel software development, from weekly thought
> leadership blogs to news, videos, case studies, tutorials and more. Take a
> look and join the conversation now. http://goparallel.sourceforge.net
> _______________________________________________
> SoX-devel mailing list
> SoX-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/sox-devel
> 


[-- Attachment #2: 0001-Im-proved-memory-management.patch --]
[-- Type: text/x-patch, Size: 700 bytes --]

>From 94c0f9125b77f0f9a678c9766330e7e55b5ec546 Mon Sep 17 00:00:00 2001
From: Pander <pander@users.sourceforge.net>
Date: Mon, 22 Dec 2014 23:56:37 +0100
Subject: [PATCH] Im proved memory management

---
 src/stat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/stat.c b/src/stat.c
index 63d1741..0ce8415 100644
--- a/src/stat.c
+++ b/src/stat.c
@@ -138,7 +138,7 @@ 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;
+  float *re_average = NULL;
   unsigned samples = 0;
   float ffa = 0.0;
   unsigned i;
-- 
2.1.0


[-- Attachment #3: Type: text/plain, Size: 440 bytes --]

------------------------------------------------------------------------------
Dive into the World of Parallel Programming! The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net

[-- Attachment #4: Type: text/plain, Size: 158 bytes --]

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

Re: Review patch for average power spectrum

[Cedric Roux]

On 12/22/2014 11:14 PM, Eric Wong wrote:
> ...otherwise, free may be called on an uninitialized value:
>
>> +  if (re_average) {
>> +    free(re_average);
>> +  }

Just for you information,
if a pointer is null free(p) is fine and does nothing.
The "if" is not required.


------------------------------------------------------------------------------
Dive into the World of Parallel Programming! The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net

Re: Review patch for average power spectrum

[Pander]

On 12/23/2014 12:02 AM, Cedric Roux wrote:
> On 12/22/2014 11:14 PM, Eric Wong wrote:
>> ...otherwise, free may be called on an uninitialized value:
>>
>>> +  if (re_average) {
>>> +    free(re_average);
>>> +  }
> 
> Just for you information,
> if a pointer is null free(p) is fine and does nothing.
> The "if" is not required.

Thanks. Could the person implementing or applying the patch take this
into account?

Question for another unrelated patch, is non-linear interpolation
available in SoX via the C libraries which are already included?

> 
> 
> ------------------------------------------------------------------------------
> Dive into the World of Parallel Programming! The Go Parallel Website,
> sponsored by Intel and developed in partnership with Slashdot Media, is your
> hub for all things parallel software development, from weekly thought
> leadership blogs to news, videos, case studies, tutorials and more. Take a
> look and join the conversation now. http://goparallel.sourceforge.net
> _______________________________________________
> SoX-devel mailing list
> SoX-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/sox-devel
> 


------------------------------------------------------------------------------
Dive into the World of Parallel Programming! The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net