Index: ChangeLog
===================================================================
--- ChangeLog	(revision 4928)
+++ ChangeLog	(working copy)
@@ -1,3 +1,25 @@
+2009-01-20  Chris Howie  <cdhowie@gmail.com>
+
+	* src/Backends/Banshee.GStreamer/Banshee.GStreamer/PlayerEngine.cs: Add
+	the bands argument to BansheePlayerVisDataCallback, track this change
+	in OnVisualizationData, and set the PlayerData* handle to IntPtr.Zero
+	after it's destroyed.
+
+	* libbanshee/banshee-player-vis.c: Overhaul the pipeline again.  Allow
+	disabling of the pipeline to conserve CPU when the data is not being
+	used.  Discard the spectrum element and use libgstfft directly to
+	eliminate complex timing and synchronization issues.
+
+	* libbanshee/banshee-player-pipeline.c: Enable the visualization
+	pipeline.
+
+	* libbanshee/banshee-player-private.h: Change signature of
+	BansheePlayerVisDataCallback to match changes to
+	Banshee.GStreamer.PlayerEngine, and tweak members of BansheePlayer for
+	the pipeline changes.
+
+	* build/m4/banshee/gstreamer.m4: Link against libgstfft.
+
 2009-01-20  Aaron Bockover  <abock@gnome.org>
 
 	Banshee 1.4.2 Released
Index: src/Backends/Banshee.GStreamer/Banshee.GStreamer/PlayerEngine.cs
===================================================================
--- src/Backends/Banshee.GStreamer/Banshee.GStreamer/PlayerEngine.cs	(revision 4928)
+++ src/Backends/Banshee.GStreamer/Banshee.GStreamer/PlayerEngine.cs	(working copy)
@@ -56,7 +56,7 @@
     internal delegate void BansheePlayerStateChangedCallback (IntPtr player, GstState old_state, GstState new_state, GstState pending_state);
     internal delegate void BansheePlayerIterateCallback (IntPtr player);
     internal delegate void BansheePlayerBufferingCallback (IntPtr player, int buffering_progress);
-    internal delegate void BansheePlayerVisDataCallback (IntPtr player, int channels, int samples, IntPtr data, IntPtr spectrum);
+    internal delegate void BansheePlayerVisDataCallback (IntPtr player, int channels, int samples, IntPtr data, int bands, IntPtr spectrum);
 
     internal delegate void GstTaggerTagFoundCallback (IntPtr player, string tagName, ref GLib.Value value);
     
@@ -166,6 +166,7 @@
             UninstallPreferences ();
             base.Dispose ();
             bp_destroy (handle);
+            handle = new HandleRef (this, IntPtr.Zero);
         }
         
         public override void Close (bool fullShutdown)
@@ -322,7 +323,7 @@
             OnTagFound (ProcessNativeTagResult (tagName, ref value));
         }
         
-        private void OnVisualizationData (IntPtr player, int channels, int samples, IntPtr data, IntPtr spectrum)
+        private void OnVisualizationData (IntPtr player, int channels, int samples, IntPtr data, int bands, IntPtr spectrum)
         {
             VisualizationDataHandler handler = data_available;
             
@@ -340,8 +341,8 @@
                 cbd[i] = channel;
             }
             
-            float [] spec = new float[512];
-            Marshal.Copy (spectrum, spec, 0, 512);
+            float [] spec = new float[bands];
+            Marshal.Copy (spectrum, spec, 0, bands);
             
             try {
                 handler (cbd, new float[][] { spec });
Index: libbanshee/banshee-player-vis.c
===================================================================
--- libbanshee/banshee-player-vis.c	(revision 4928)
+++ libbanshee/banshee-player-vis.c	(working copy)
@@ -26,13 +26,15 @@
 // WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 //
 
+#include <math.h>
+
 #include "banshee-player-vis.h"
 
-#define SPECTRUM_SIZE 512
+#define SLICE_SIZE 735
 
 static GstStaticCaps vis_data_sink_caps = GST_STATIC_CAPS (
     "audio/x-raw-float, "
-    "rate = (int) 30720, "
+    "rate = (int) 44100, "
     "channels = (int) 2, "
     "endianness = (int) BYTE_ORDER, "
     "width = (int) 32"
@@ -49,33 +51,86 @@
     GstStructure *structure;
     gint channels, wanted_size;
     gfloat *data;
+    GstClock *clock;
+    GstClockTime clock_time;
     
     g_return_if_fail (IS_BANSHEE_PLAYER (player));
     
     if (player->vis_data_cb == NULL) {
         return;
     }
+
+    if (player->vis_thawing) {
+        clock = GST_ELEMENT_CLOCK (sink);
+        clock_time = gst_clock_get_time (clock);
+
+        if (GST_BUFFER_TIMESTAMP (buffer) < clock_time)
+            return;
+
+        gst_adapter_clear (player->vis_buffer);
+
+        memset (player->vis_fft_sample_buffer, 0, sizeof(gfloat) * SLICE_SIZE);
+
+        player->vis_thawing = FALSE;
+    }
     
     structure = gst_caps_get_structure (gst_buffer_get_caps (buffer), 0);
     gst_structure_get_int (structure, "channels", &channels);
     
-    wanted_size = channels * SPECTRUM_SIZE * sizeof (gfloat);
-    
+    wanted_size = channels * SLICE_SIZE * sizeof (gfloat);
+
     gst_adapter_push (player->vis_buffer, gst_buffer_copy (buffer));
     
     while ((data = (gfloat *)gst_adapter_peek (player->vis_buffer, wanted_size)) != NULL) {
         gfloat *deinterlaced = g_malloc (wanted_size);
+        gfloat *specbuf = g_new (gfloat, SLICE_SIZE * 2);
+
         gint i, j;
+
+        memcpy (specbuf, player->vis_fft_sample_buffer, SLICE_SIZE * sizeof(gfloat));
         
-        for (i = 0; i < SPECTRUM_SIZE; i++) {
+        for (i = 0; i < SLICE_SIZE; i++) {
+            gfloat avg = 0.0f;
+
             for (j = 0; j < channels; j++) {
-                deinterlaced[j * SPECTRUM_SIZE + i] = data[i * channels + j];
+                gfloat sample = data[i * channels + j];
+
+                deinterlaced[j * SLICE_SIZE + i] = sample;
+                avg += sample;
             }
+
+            avg /= channels;
+            specbuf[i + SLICE_SIZE] = avg;
         }
+
+        memcpy (player->vis_fft_sample_buffer, &specbuf[SLICE_SIZE], SLICE_SIZE * sizeof(gfloat));
+
+        gst_fft_f32_window (player->vis_fft, specbuf, GST_FFT_WINDOW_HAMMING);
+        gst_fft_f32_fft (player->vis_fft, specbuf, player->vis_fft_buffer);
+
+        for (i = 0; i < SLICE_SIZE; i++) {
+            gfloat val;
+
+            GstFFTF32Complex cplx = player->vis_fft_buffer[i];
+
+            val = cplx.r * cplx.r + cplx.i * cplx.i;
+            val /= SLICE_SIZE * SLICE_SIZE;
+            val = 10.0f * log10f(val);
+
+            val = (val + 60.0f) / 60.0f;
+            if (val < 0.0f)
+                val = 0.0f;
+
+            specbuf[i] = val;
+        }
+
+        player->vis_data_cb (player,
+                             channels, SLICE_SIZE, deinterlaced,
+                             SLICE_SIZE, specbuf);
         
-        player->vis_data_cb (player, channels, SPECTRUM_SIZE, deinterlaced, player->spectrum_buffer);
-        
         g_free (deinterlaced);
+        g_free (specbuf);
+
         gst_adapter_flush (player->vis_buffer, wanted_size);
     }
 }
@@ -84,63 +139,104 @@
 // Internal Functions
 // ---------------------------------------------------------------------------
 
-void
-_bp_vis_process_message (BansheePlayer *player, GstMessage *message)
+static void
+_bp_vis_pipeline_block_callback (GstPad *pad, gboolean blocked, gpointer data)
 {
-    const GstStructure *st;
-    const GValue *spec;
-    gint i;
-    
-    g_return_if_fail (IS_BANSHEE_PLAYER (player));
-    
-    st = gst_message_get_structure (message);
-    if (strcmp (gst_structure_get_name (st), "spectrum") != 0) {
+    BansheePlayer *player = (BansheePlayer *) data;
+
+    if (!blocked) {
+        // Set thawing mode (discards buffers that are too old from the queue).
+        player->vis_thawing = TRUE;
+    }
+}
+
+static void
+_bp_vis_pipeline_set_blocked (BansheePlayer *player, gboolean blocked)
+{
+    GstPad *queue_sink;
+
+    if (player->vis_resampler == NULL)
         return;
+
+    queue_sink = gst_element_get_static_pad (player->vis_resampler, "src");
+
+    gst_pad_set_blocked_async (queue_sink, blocked, _bp_vis_pipeline_block_callback, (gpointer) player);
+
+    gst_object_unref (GST_OBJECT (queue_sink));
+}
+
+static gboolean
+_bp_vis_pipeline_event_probe (GstPad *pad, GstEvent *event, gpointer data)
+{
+    BansheePlayer *player = (BansheePlayer *) data;
+
+    if (player->vis_enabled)
+        return TRUE;
+
+    switch (GST_EVENT_TYPE (event)) {
+    case GST_EVENT_EOS:
+        _bp_vis_pipeline_set_blocked (player, FALSE);
+        break;
+
+    case GST_EVENT_NEWSEGMENT:
+        _bp_vis_pipeline_set_blocked (player, TRUE);
+        break;
+
+    default: break;
     }
-    
-    spec = gst_structure_get_value (st, "magnitude");
-    
-    for (i = 0; i < SPECTRUM_SIZE; i++) {
-        // v is in the range -60 to 0.  Move this up to 0 to 1.
-        gfloat v = g_value_get_float (gst_value_list_get_value (spec, i));
-        player->spectrum_buffer[i] = (v + 60.0f) / 60.0f;
-    }
+
+    return TRUE;
 }
 
 void
 _bp_vis_pipeline_setup (BansheePlayer *player)
 {
-    GstElement *fakesink, *converter, *resampler, *audiosinkqueue, *spectrum;
+    // The basic pipeline we're constructing is:
+    // .audiotee ! queue ! audioresample ! audioconvert ! fakesink
+
+    GstElement *fakesink, *converter, *resampler, *audiosinkqueue;
     GstCaps *caps;
     GstPad *pad;
+    gint wanted_size;
     
     player->vis_buffer = NULL;
-    player->spectrum_buffer = NULL;
+    player->vis_fft = gst_fft_f32_new (SLICE_SIZE * 2, FALSE);
+    player->vis_fft_buffer = g_new (GstFFTF32Complex, SLICE_SIZE + 1);
+    player->vis_fft_sample_buffer = g_new0 (gfloat, SLICE_SIZE);
     
-    // Privided by gst-plugins-good
-    spectrum = gst_element_factory_make ("spectrum", "vis-spectrum");
-    if (spectrum == NULL) {
-        bp_debug ("Could not create the spectrum element. Visualization will be disabled.");
-        return;
-    }
-    
-    g_object_set (G_OBJECT (spectrum), "bands", SPECTRUM_SIZE, "interval", GST_SECOND / 60, NULL);
-    
     // Core elements, if something fails here, it's the end of the world
     audiosinkqueue = gst_element_factory_make ("queue", "vis-queue");
+
+    pad = gst_element_get_static_pad (audiosinkqueue, "sink");
+    gst_pad_add_event_probe (pad, G_CALLBACK (_bp_vis_pipeline_event_probe), player);
+    gst_object_unref (GST_OBJECT (pad));
+
     resampler = gst_element_factory_make ("audioresample", "vis-resample");
     converter = gst_element_factory_make ("audioconvert", "vis-convert");
     fakesink = gst_element_factory_make ("fakesink", "vis-sink");
-    
+
+    // channels * slice size * float size = size of chunks we want
+    wanted_size = 2 * SLICE_SIZE * sizeof(gfloat);
+
     if (audiosinkqueue == NULL || resampler == NULL || converter == NULL || fakesink == NULL) {
         bp_debug ("Could not construct visualization pipeline, a fundamental element could not be created");
         return;
     }
+
+    // Keep around the 5 most recent seconds of audio so that when resuming
+    // visualization we have something to show right away.
+    g_object_set (G_OBJECT (audiosinkqueue),
+            "leaky", 2,
+            "max-size-buffers", 0,
+            "max-size-bytes", 0,
+            "max-size-time", GST_SECOND * 5,
+            NULL);
     
     g_signal_connect (G_OBJECT (fakesink), "handoff", G_CALLBACK (bp_vis_pcm_handoff), player);
-    g_object_set (G_OBJECT (fakesink), "signal-handoffs", TRUE, "sync", TRUE, NULL);
+    g_object_set (G_OBJECT (fakesink), "signal-handoffs", TRUE, "sync", TRUE, "async", FALSE, NULL);
     
-    gst_bin_add_many (GST_BIN (player->audiobin), audiosinkqueue, resampler, converter, spectrum, fakesink, NULL);
+    gst_bin_add_many (GST_BIN (player->audiobin), audiosinkqueue, resampler,
+                      converter, fakesink, NULL);
     
     pad = gst_element_get_static_pad (audiosinkqueue, "sink");
     gst_pad_link (gst_element_get_request_pad (player->audiotee, "src%d"), pad);
@@ -149,13 +245,16 @@
     gst_element_link_many (audiosinkqueue, resampler, converter, NULL);
     
     caps = gst_static_caps_get (&vis_data_sink_caps);
-    gst_element_link_filtered (converter, spectrum, caps);
+    gst_element_link_filtered (converter, fakesink, caps);
     gst_caps_unref (caps);
     
-    gst_element_link (spectrum, fakesink);
-    
     player->vis_buffer = gst_adapter_new ();
-    player->spectrum_buffer = g_new0 (gfloat, SPECTRUM_SIZE);
+    player->vis_resampler = resampler;
+    player->vis_thawing = FALSE;
+    player->vis_enabled = FALSE;
+
+    // Disable the pipeline till we hear otherwise from managed land.
+    _bp_vis_pipeline_set_blocked (player, TRUE);
 }
 
 void
@@ -164,9 +263,19 @@
     if (player->vis_buffer != NULL) {
         gst_object_unref (player->vis_buffer);
         player->vis_buffer = NULL;
-        
-        g_free (player->spectrum_buffer);
-        player->spectrum_buffer = NULL;
+
+        gst_fft_f32_free (player->vis_fft);
+        player->vis_fft = NULL;
+
+        g_free (player->vis_fft_buffer);
+        player->vis_fft_buffer = NULL;
+
+        g_free (player->vis_fft_sample_buffer);
+        player->vis_fft_sample_buffer = NULL;
+
+        player->vis_resampler = NULL;
+        player->vis_enabled = FALSE;
+        player->vis_thawing = FALSE;
     }
 }
 
@@ -177,5 +286,11 @@
 P_INVOKE void
 bp_set_vis_data_callback (BansheePlayer *player, BansheePlayerVisDataCallback cb)
 {
-    SET_CALLBACK (vis_data_cb);
+    if (player == NULL)
+        return;
+
+    player->vis_data_cb = cb;
+
+    _bp_vis_pipeline_set_blocked (player, cb == NULL);
+    player->vis_enabled = cb != NULL;
 }
Index: libbanshee/banshee-player-pipeline.c
===================================================================
--- libbanshee/banshee-player-pipeline.c	(revision 4928)
+++ libbanshee/banshee-player-pipeline.c	(working copy)
@@ -261,7 +261,7 @@
         gst_element_link (audiosinkqueue, audiosink);
     }
     
-    // _bp_vis_pipeline_setup (player);
+    _bp_vis_pipeline_setup (player);
     
     // Now that our internal audio sink is constructed, tell playbin to use it
     g_object_set (G_OBJECT (player->playbin), "audio-sink", player->audiobin, NULL);
Index: libbanshee/banshee-player-private.h
===================================================================
--- libbanshee/banshee-player-private.h	(revision 4928)
+++ libbanshee/banshee-player-private.h	(working copy)
@@ -37,6 +37,7 @@
 #include <gst/gst.h>
 #include <gst/base/gstadapter.h>
 #include <gdk/gdk.h>
+#include <gst/fft/gstfftf32.h>
 
 #ifdef HAVE_GST_PBUTILS
 #  include <gst/pbutils/pbutils.h>
@@ -65,7 +66,7 @@
 typedef void (* BansheePlayerIterateCallback)      (BansheePlayer *player);
 typedef void (* BansheePlayerBufferingCallback)    (BansheePlayer *player, gint buffering_progress);
 typedef void (* BansheePlayerTagFoundCallback)     (BansheePlayer *player, const gchar *tag, const GValue *value);
-typedef void (* BansheePlayerVisDataCallback)      (BansheePlayer *player, gint channels, gint samples, gfloat *data, gfloat *spectrum);
+typedef void (* BansheePlayerVisDataCallback)      (BansheePlayer *player, gint channels, gint samples, gfloat *data, gint bands, gfloat *spectrum);
 
 struct BansheePlayer {
     // Player Callbacks
@@ -100,8 +101,13 @@
     #endif
     
     // Visualization State
+    GstElement *vis_resampler;
     GstAdapter *vis_buffer;
-    gfloat *spectrum_buffer;
+    gboolean vis_enabled;
+    gboolean vis_thawing;
+    GstFFTF32 *vis_fft;
+    GstFFTF32Complex *vis_fft_buffer;
+    gfloat *vis_fft_sample_buffer;
     
     // Plugin Installer State
     GdkWindow *window;
Index: build/m4/banshee/gstreamer.m4
===================================================================
--- build/m4/banshee/gstreamer.m4	(revision 4928)
+++ build/m4/banshee/gstreamer.m4	(working copy)
@@ -8,7 +8,8 @@
 		gstreamer-base-0.10 >= $GSTREAMER_REQUIRED_VERSION
 		gstreamer-plugins-base-0.10 >= $GSTREAMER_REQUIRED_VERSION
 		gstreamer-controller-0.10 >= $GSTREAMER_REQUIRED_VERSION
-		gstreamer-dataprotocol-0.10 >= $GSTREAMER_REQUIRED_VERSION)
+		gstreamer-dataprotocol-0.10 >= $GSTREAMER_REQUIRED_VERSION
+		gstreamer-fft-0.10 >= $GSTREAMER_REQUIRED_VERSION)
 
 	GST_LIBS="$GST_LIBS -lgstvideo-0.10 -lgstinterfaces-0.10 -lgstcdda-0.10"
 
