After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 567642 - spectrum element has undocumented arbitrary limitation on interval
spectrum element has undocumented arbitrary limitation on interval
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
0.10.8
Other All
: Normal blocker
: 0.10.14
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2009-01-13 18:31 UTC by Chris Howie
Modified: 2009-01-23 08:38 UTC
See Also:
GNOME target: ---
GNOME version: 2.21/2.22


Attachments
spectrum.diff (27.25 KB, patch)
2009-01-14 10:47 UTC, Sebastian Dröge (slomo)
committed Details | Review
spectrum-crash.diff (683 bytes, patch)
2009-01-15 11:38 UTC, Sebastian Dröge (slomo)
committed Details | Review

Description Chris Howie 2009-01-13 18:31:17 UTC
Please describe the problem:
I've been working on a project to bring visualization to Banshee, and the spectrum element has been giving me headaches lately.  I have interval set to GST_SECOND/60, expecting 60 messages to be posted to the bus per second, so I can multiplex that with the audio stream and push it to a visualizer.

Unfortunately, the spectrum element has a fixed window size of 1024 samples (per channel) and will only post once per 1024 samples.  At 44100Hz, this means 44100/1024 =~ 43.07 posts per second, not 60.  In order to achieve 60 posts per second I would need a samplerate of 1024*60 = 61440Hz.  The resample would degrade performance noticeably on mid-lower end boxes.

Can this be fixed so that the window size is not declared at compile-time and is instead a function of the samplerate and requested interval?  Or could this limitation at least be documented somewhere?

Thanks,
Chris

Steps to reproduce:


Actual results:


Expected results:


Does this happen every time?


Other information:
Comment 1 Sebastian Dröge (slomo) 2009-01-13 20:25:42 UTC
The "window size" is defined by the bands property. You have to get at least bands * 2 - 2 samples before you can apply the FFT. This limitation should be noted in the docs though ;)

Also we could instead cache the last bands*2-2 samples and always generate a spectrum for them in the requested interval. I'll implement this tomorrow.
Comment 2 Chris Howie 2009-01-13 20:31:13 UTC
(In reply to comment #1)
> The "window size" is defined by the bands property. You have to get at least
> bands * 2 - 2 samples before you can apply the FFT. This limitation should be
> noted in the docs though ;)
> 
> Also we could instead cache the last bands*2-2 samples and always generate a
> spectrum for them in the requested interval. I'll implement this tomorrow.

Has this changed since 0.10.8 then?  There was a macro for the window length in the version I'm looking at:

#define SPECTRUM_WINDOW_BASE 9
#define SPECTRUM_WINDOW_LEN (1 << (SPECTRUM_WINDOW_BASE+1))

Everything is taken in slices of SPECTRUM_WINDOW_LEN, which is 1024.  Or I'm missing something obvious.
Comment 3 Chris Howie 2009-01-13 20:36:10 UTC
(In reply to comment #2)
> Has this changed since 0.10.8 then?  There was a macro for the window length in
> the version I'm looking at:
> 
> #define SPECTRUM_WINDOW_BASE 9
> #define SPECTRUM_WINDOW_LEN (1 << (SPECTRUM_WINDOW_BASE+1))
> 
> Everything is taken in slices of SPECTRUM_WINDOW_LEN, which is 1024.  Or I'm
> missing something obvious.

Hmm, interestingly in 0.10.9 those macros are still defined but not used anywhere.  Pay no attention to the man behind the curtain...
Comment 4 Sebastian Dröge (slomo) 2009-01-14 10:46:38 UTC
2009-01-14  Sebastian Dröge  <sebastian.droege@collabora.co.uk>

	* gst/spectrum/Makefile.am:
	* gst/spectrum/README:
	* gst/spectrum/gstspectrum.c: (gst_spectrum_base_init),
	(gst_spectrum_class_init), (gst_spectrum_init),
	(gst_spectrum_reset_state), (gst_spectrum_finalize),
	(gst_spectrum_set_property), (gst_spectrum_start),
	(gst_spectrum_stop), (gst_spectrum_setup),
	(gst_spectrum_transform_ip):
	* gst/spectrum/gstspectrum.h:
	Post a spectrum message on the bus for every interval, even
	if the interval is small than the length of the FFT.
	Fixes bug #567642.

	Major cleanup of the spectrum element.
Comment 5 Sebastian Dröge (slomo) 2009-01-14 10:47:06 UTC
Created attachment 126413 [details] [review]
spectrum.diff
Comment 6 Chris Howie 2009-01-14 19:17:44 UTC
Should the target milestone be 0.10.22?  I think 0.10.12 has been out for some time.

Anyway, thanks for the patch, I'll grab cvs head and build ASAP.
Comment 7 Chris Howie 2009-01-14 19:24:34 UTC
Ah, I just saw that 0.10.11 is the latest stable gst-plugins-good.  Still getting used to the whole version numbers not being equal thing.
Comment 8 Chris Howie 2009-01-15 00:25:26 UTC
(In reply to comment #5)
> Created an attachment (id=126413) [edit]
> spectrum.diff

When I apply this patch to 0.10.8, the test suite fails on the spectrum element:

Running suite(s): spectrum
0%: Checks: 4, Failures: 0, Errors: 4
gstcheck.c:247:E:general:test_int16:0: (after this point) Received signal 11 (Segmentation fault)
gstcheck.c:247:E:general:test_int32:0: (after this point) Received signal 11 (Segmentation fault)
gstcheck.c:247:E:general:test_float32:0: (after this point) Received signal 11 (Segmentation fault)
gstcheck.c:247:E:general:test_float64:0: (after this point) Received signal 11 (Segmentation fault)
FAIL: elements/spectrum

Is this expected?  Should I ignore it, or do the tests need updating, or...
Comment 9 Sebastian Dröge (slomo) 2009-01-15 08:43:06 UTC
No this should not happen and doesn't happen for me... it's valgrind clean here ;)

Could you get backtraces? in tests/check just run "make elements/spectrum.gdb" and then "run" from the gdb prompt.
Comment 10 Sebastian Dröge (slomo) 2009-01-15 11:38:59 UTC
Created attachment 126496 [details] [review]
spectrum-crash.diff

This should fix the crash... my local version of the gstfft library allows to call _free() with NULL.
Comment 11 Sebastian Dröge (slomo) 2009-01-15 11:40:10 UTC
2009-01-15  Sebastian Dröge  <sebastian.droege@collabora.co.uk>

        * gst/spectrum/gstspectrum.c: (gst_spectrum_reset_state):
        Don't call gst_fft_f32_free() with NULL to prevent a
        crash. Fixes bug #567642.
Comment 12 Chris Howie 2009-01-15 19:31:03 UTC
(In reply to comment #10)
> This should fix the crash... my local version of the gstfft library allows to
> call _free() with NULL.

Yup, the spectrum tests pass now.  Thanks!

I'll test tonight and see if the element seems to behave better.
Comment 13 Chris Howie 2009-01-16 00:09:42 UTC
The element seems to be working great except for one minor detail.  (Or a major detail depending on how you look at it.)

Now it's firing off just slightly *more* messages than the interval property indicates.  Most seconds it's fine and fires of somewhere between 59-61 when interval=GST_SECOND/60, and it all averages out.  But some seconds it fires off 65 and never compensates for that, which makes the spectrum analysis behind the playing audio.

I can try to come up with a testcase if you'd like.

Good work so far though, it's much smoother.
Comment 14 Sebastian Dröge (slomo) 2009-01-16 09:55:31 UTC
Chris, see bug #567955 for this ;)