From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Eric Wong Newsgroups: gmane.comp.audio.sox.devel Subject: Re: sox spectrogram patches Date: Sat, 26 Dec 2015 23:15:53 +0000 Message-ID: <20151226231553.GA9253@dcvr.yhbt.net> References: <20151226104316.GA5868@dcvr.yhbt.net> <20151226211813.GA13684@dcvr.yhbt.net> Reply-To: sox-devel@lists.sourceforge.net NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit X-Trace: ger.gmane.org 1451171781 7774 80.91.229.3 (26 Dec 2015 23:16:21 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sat, 26 Dec 2015 23:16:21 +0000 (UTC) To: sox-devel@lists.sourceforge.net Original-X-From: sox-devel-bounces@lists.sourceforge.net Sun Dec 27 00:16:19 2015 Return-path: Envelope-to: gcasd-sox-devel@m.gmane.org X-ACL-Warn: Content-Disposition: inline In-Reply-To: X-Spam-Score: -0.0 (/) X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. -0.0 RP_MATCHES_RCVD Envelope sender domain matches handover relay domain X-Headers-End: 1aCy46-0001ky-T4 X-BeenThere: sox-devel@lists.sourceforge.net X-Mailman-Version: 2.1.9 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: sox-devel-bounces@lists.sourceforge.net Xref: news.gmane.org gmane.comp.audio.sox.devel:459 Archived-At: Received: from lists.sourceforge.net ([216.34.181.88]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1aCy4P-0004aj-Tz for gcasd-sox-devel@m.gmane.org; Sun, 27 Dec 2015 00:16:18 +0100 Received: from localhost ([127.0.0.1] helo=sfs-ml-2.v29.ch3.sourceforge.com) by sfs-ml-2.v29.ch3.sourceforge.com with esmtp (Exim 4.76) (envelope-from ) id 1aCy49-00033u-MH; Sat, 26 Dec 2015 23:16:01 +0000 Received: from sog-mx-4.v43.ch3.sourceforge.com ([172.29.43.194] helo=mx.sourceforge.net) by sfs-ml-2.v29.ch3.sourceforge.com with esmtp (Exim 4.76) (envelope-from ) id 1aCy48-00033m-EC for sox-devel@lists.sourceforge.net; Sat, 26 Dec 2015 23:16:00 +0000 Received: from dcvr.yhbt.net ([64.71.152.64]) by sog-mx-4.v43.ch3.sourceforge.com with esmtp (Exim 4.76) id 1aCy46-0001ky-T4 for sox-devel@lists.sourceforge.net; Sat, 26 Dec 2015 23:16:00 +0000 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 5B0A71F724; Sat, 26 Dec 2015 23:15:53 +0000 (UTC) Martin Guy wrote: > On 26/12/2015, Eric Wong 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'm not entirely sure what you mean, here. Basically the shared_start function doesn't change anything for FFTW users; and I've verified the PNGs of using FFTW with/without my work-in-progress patch via cmp(1) with identical results. Or did you mean the location of fftw_plan field inside priv_t? That doesn't seem to matter, does it? I don't know the FFT stuff and math stuff well at all, so I could be wrong. > 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... Heh, I did break it --without-fftw :x I wasn't even thinking about the RAM usage, but about documenting how the structure would be used. Here's my work-in-progress fix, output now matches the FFTW-using PNG: diff --git a/src/spectrogram.c b/src/spectrogram.c index 1b82aad..d1e98c5 100644 --- a/src/spectrogram.c +++ b/src/spectrogram.c @@ -72,9 +72,8 @@ typedef struct { /* Shared work area */ #if HAVE_FFTW fftw_plan fftw_plan; /* Used if FFT_type == FFT_fftw */ -#else - double * shared, * * shared_ptr; #endif + double * shared, * * shared_ptr; /* unused for FFTW */ /* Per-channel work area */ int WORK; /* Start of work area is marked by this dummy variable. */ @@ -170,6 +169,7 @@ static int getopts(sox_effect_t * effp, int argc, char **argv) p->spectrum_points += 2; if (p->alt_palette) p->spectrum_points = min(p->spectrum_points, (int)alt_palette_len); + p->shared_ptr = &p->shared; if (!strcmp(p->out_name, "-")) { if (effp->global_info->global_info->stdout_in_use_by) { lsx_fail("stdout already in use by `%s'", effp->global_info->global_info->stdout_in_use_by); @@ -244,12 +244,14 @@ static void shared_start(sox_effect_t * effp) { priv_t * p = (priv_t *)effp->priv; - p->shared_ptr = &p->shared; - if (!is_p2(p->dft_size) && !effp->flow) { - if (p->y_size) { - p->shared = rdft_init((size_t)(p->dft_size)); + if (!effp->flow) { + if (!is_p2(p->dft_size)) { + if (p->y_size) { + p->shared = rdft_init((size_t)(p->dft_size)); + } + } else { + lsx_safe_rdft(p->dft_size, 1, p->dft_buf); } - lsx_safe_rdft(p->dft_size, 1, p->dft_buf); } } > 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... No worries. Better we catch our mistakes now instead of throwing a nasty heap at Chris and the gang when they decide to return :) ------------------------------------------------------------------------------