Re: sox spectrogram patches

[Martin Guy <martinwguy@gmail.com>]

On 26/12/2015, Eric Wong <normalperson@yhbt.net> wrote:
> --- a/src/spectrogram.c
> +++ b/src/spectrogram.c
> @@ -70,7 +70,11 @@ typedef struct {
>    sox_bool   using_stdout; /* output image to stdout */
>
>    /* Shared work area */
> +#if HAVE_FFTW
> +  fftw_plan  fftw_plan;		/* Used if FFT_type == FFT_fftw */
> +#else
>    double     * shared, * * shared_ptr;
> +#endif
>
>    /* Per-channel work area */
>    int        WORK;  /* Start of work area is marked by this dummy variable.
> */
> @@ -84,9 +88,6 @@ typedef struct {
>    double     block_norm, max;
>    double     * magnitudes;	/* [(dft_size / 2) + 1] */
>    float      * dBfs;
> -#if HAVE_FFTW
> -  fftw_plan  fftw_plan;		/* Used if FFT_type == FFT_fftw */
> -#endif
>  } priv_t;
>
>  #define secs(cols) \
> +  /* We have one FFT plan per flow because the input/output arrays differ.
> */
> +  p->fftw_plan = fftw_plan_r2r_1d(p->dft_size, p->dft_buf, p->dft_buf,
> +                      FFTW_R2HC, FFTW_MEASURE);
> +}

This may not be right. FFTW3 plans depend on the memory addresses of
the input and output vectors, so if you have two FFTs that are exactly
the same except for the input and output buffer addresses, they need
separate plans.
In this case, the plan depends on dft_buf, which seems to be specific
to each flow, so I think you should have a separare plan for each
flow.

I don't know what "shared" is used for, so I just left it alone. AFter
all, it's ore important that the code work, than to save four bytes of
RAM and risk breaking it...

By the way, that "/* Used if FFT_type == FFT_fftw */" comment is
stale, dating back to when I had an option to choose the FFT algorithm
at runtime - sorry about that...

Thanks

    M

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