Created attachment 120358 [details] [review]
Patch

Created attachment 120364 [details] [review]
New patch

Updated patch.  Now almost completely disables the visualization pipeline when the callback is NULL, which happens when nothing is listening to the event on the managed side.

The only thing left running is a 5-second queue so when visualization resumes there is data available immediately.  This doesn't seem to have any significant impact on performance.

With this patch and the latest Banshee.OpenVP trunk, I see no noticeable difference in CPU load when the visualization source is not selected.

Great job !

I've discovered a nasty bug in this patch related to how gstreamer detects when a pipeline is done running.  Let me try to resolve this issue before any patch is applied to trunk.

Marking patch "needs_work" as per previous comment.

Created attachment 122473 [details] [review]
New new patch!

Fixes issue with previous patch.  (When the visualizer was not the active source, a playing song would continue "playing" until the visualizer was activated.)

After applying the patch and with the Banshee.OpenVP extension, I have the following problem :
1. Start banshee
2. play a track
3. Go to the Visualization source
4. Pause the track
5. Quit banshee
I get the following stack trace :
[Debug 21:28:30.792] Service disposed (AudioscrobblerService)
[Debug 21:28:30.797] Service disposed (Network)
[Debug 21:28:30.847] Service disposed (LastfmRecommendationService)
[Debug 21:28:30.860] Service disposed (AudioCdService)
[Debug 21:28:30.867] Service disposed (NotificationAreaService)
[Debug 21:28:30.912] Service disposed (DapService)
[Debug 21:28:30.915] Service disposed (BookmarksService)
[Debug 21:28:30.921] Service disposed (MultimediaKeysService)
[Debug 21:28:30.929] Service disposed (NereidPlayerInterface)
[Debug 21:28:30.930] Service disposed (CollectionIndexerService)
[Debug 21:28:30.933] Service disposed (HardwareManager)
[Debug 21:28:30.936] (libbanshee:player) bp_stop: setting state to GST_STATE_NULL
[Debug 21:28:31.012] Player state change: Paused -> Idle
[Debug 21:28:31.020] (libbanshee:player) bp_destroy: disposed player
[Debug 21:28:31.026] Service disposed (PlayerEngine)
[Debug 21:28:31.096] Service disposed (SourceManager)
[Debug 21:28:31.102] Service disposed (DbConnection)
Stacktrace:


Native stacktrace:

	/usr/bin/mono [0x817f8e4]
	/usr/bin/mono [0x80802b0]
	[0x4001c40c]

Debug info from gdb:

(no debugging symbols found)
Using host libthread_db library "/lib/libthread_db.so.1".
(no debugging symbols found)
(no debugging symbols found)
(no debugging symbols found)
(no debugging symbols found)
(no debugging symbols found)
[Thread debugging using libthread_db enabled]
[New Thread 0x402966e0 (LWP 24402)]
[New Thread 0x47600b90 (LWP 24435)]
[New Thread 0x45445b90 (LWP 24431)]
[New Thread 0x44e15b90 (LWP 24413)]
[New Thread 0x438e3b90 (LWP 24412)]
[New Thread 0x43ae4b90 (LWP 24411)]
[New Thread 0x40f91b90 (LWP 24404)]
[New Thread 0x40912b90 (LWP 24403)]
(no debugging symbols found)
...

Might be helpful : The following actions
1. Start banshee, do not play anything
2. Go to the Visualization source
3. Quit banshee
Gets me the following messages :

[Debug 21:31:54.339] Service disposed (CoverArtService)
[Debug 21:31:54.340] Service disposed (GStreamerCoreService)
[Debug 21:31:54.440] Service disposed (PodcastService)
[Debug 21:31:54.441] Service disposed (GnomeService)
[Debug 21:31:54.446] Service disposed (AudioscrobblerService)
[Debug 21:31:54.455] Service disposed (Network)
[Debug 21:31:54.465] Service disposed (LastfmRecommendationService)
[Debug 21:31:54.525] Service disposed (AudioCdService)
[Debug 21:31:54.533] Service disposed (NotificationAreaService)
[Debug 21:31:54.540] Service disposed (DapService)
[Debug 21:31:54.541] Service disposed (BookmarksService)
[Debug 21:31:54.587] Service disposed (MultimediaKeysService)
[Debug 21:31:54.591] Service disposed (NereidPlayerInterface)
[Debug 21:31:54.592] Service disposed (CollectionIndexerService)
[Debug 21:31:54.594] Service disposed (HardwareManager)
[Debug 21:31:54.603] (libbanshee:player) bp_stop: setting state to GST_STATE_NULL
[Debug 21:31:54.605] (libbanshee:player) bp_destroy: disposed player
[Debug 21:31:54.605] Service disposed (PlayerEngine)
[Debug 21:31:54.677] Service disposed (SourceManager)
[Debug 21:31:54.722] Service disposed (DbConnection)

(Nereid:24527): GStreamer-CRITICAL **: gst_element_get_static_pad: assertion `GST_IS_ELEMENT (element)' failed

(Nereid:24527): GStreamer-CRITICAL **: gst_pad_set_blocked_async: assertion `GST_IS_PAD (pad)' failed

(Nereid:24527): GStreamer-CRITICAL **: gst_object_unref: assertion `object != NULL' failed

Created attachment 122626 [details] [review]
Even newer new patch!!

Thanks Bertrand.  I think this new patch fixes those issues.

Better, but not perfect ;)
The GStreamer-CRITICAL are gone, but still crashing on exit with those steps :
1. Start banshee
2. play a track
3. Go to the Visualization source
4. Pause the track
5. Quit banshee

[Debug 21:15:21.201] Player state change: Playing -> Paused
[Debug 21:15:24.864] Service disposed (CoverArtService)
[Debug 21:15:24.864] Service disposed (GStreamerCoreService)
[Debug 21:15:24.964] Service disposed (PodcastService)
[Debug 21:15:24.965] Service disposed (GnomeService)
[Debug 21:15:24.969] Track Alanis Morissette - Moratorium (on Flavors Of Entanglement) <00:05:35.2730000> [file:///home/lorentz/Music/Library/Alanis%20Morissette/Flavors%20Of%20Entanglement/08%20-%20Moratorium.mp3] had playtime of 14947 msec (14sec), duration 335273 msec, queued: False
[Debug 21:15:24.981] Service disposed (AudioscrobblerService)
[Debug 21:15:24.986] Service disposed (Network)
[Debug 21:15:25.038] Service disposed (LastfmRecommendationService)
[Debug 21:15:25.053] Service disposed (AudioCdService)
[Debug 21:15:25.059] Service disposed (NotificationAreaService)
[Debug 21:15:25.108] Service disposed (DapService)
[Debug 21:15:25.109] Service disposed (BookmarksService)
[Debug 21:15:25.115] Service disposed (MultimediaKeysService)
[Debug 21:15:25.125] Service disposed (NereidPlayerInterface)
[Debug 21:15:25.126] Service disposed (CollectionIndexerService)
[Debug 21:15:25.129] Service disposed (HardwareManager)
[Debug 21:15:25.228] (libbanshee:player) bp_stop: setting state to GST_STATE_NULL
[Debug 21:15:25.320] Player state change: Paused -> Idle
[Debug 21:15:25.333] (libbanshee:player) bp_destroy: disposed player
[Debug 21:15:25.334] Service disposed (PlayerEngine)
[Debug 21:15:25.416] Service disposed (SourceManager)
[Debug 21:15:25.421] Service disposed (DbConnection)
Stacktrace:

  at (wrapper managed-to-native) Banshee.GStreamer.PlayerEngine.bp_set_vis_data_callback (System.Runtime.InteropServices.HandleRef,Banshee.GStreamer.BansheePlayerVisDataCallback) <0x00004>
  at (wrapper managed-to-native) Banshee.GStreamer.PlayerEngine.bp_set_vis_data_callback (System.Runtime.InteropServices.HandleRef,Banshee.GStreamer.BansheePlayerVisDataCallback) <0xffffffff>
  at Banshee.GStreamer.PlayerEngine.remove_DataAvailable (Banshee.MediaEngine.VisualizationDataHandler) [0x00029] in /home/lorentz/Projets/banshee/src/Backends/Banshee.GStreamer/Banshee.GStreamer/PlayerEngine.cs:104
  at Banshee.OpenVP.BansheePlayerData.set_Active (bool) [0x00034] in /home/lorentz/Projets/openvisualizationplatform-trunk/Banshee.OpenVP/Banshee.OpenVP/BansheePlayerData.cs:55
  at Banshee.OpenVP.BansheePlayerData.Dispose () [0x00000] in /home/lorentz/Projets/openvisualizationplatform-trunk/Banshee.OpenVP/Banshee.OpenVP/BansheePlayerData.cs:97
  at Banshee.OpenVP.VisualizationDisplayWidget.RenderLoop () [0x000a0] in /home/lorentz/Projets/openvisualizationplatform-trunk/Banshee.OpenVP/Banshee.OpenVP/VisualizationDisplayWidget.cs:133
  at (wrapper runtime-invoke) System.Object.runtime_invoke_void (object,intptr,intptr,intptr) <0xffffffff>

Native stacktrace:

	/usr/bin/mono [0x817f8e4]
	/usr/bin/mono [0x80802b0]
	[0x4001c40c]
	./libbanshee.so [0x43cef558]
	./libbanshee.so(bp_set_vis_data_callback+0x28) [0x43cef5c8]
	[0x457482cc]
	[0x47e59dd3]
	[0x45748140]
	[0x47e59d15]
	[0x45747df6]
	[0x408ff911]
	/usr/bin/mono(mono_runtime_delegate_invoke+0x36) [0x80ea4d6]
	/usr/bin/mono [0x80c85fd]
	/usr/bin/mono [0x811f6b2]
	/usr/bin/mono [0x8139e45]
	/lib/libpthread.so.0 [0x4012d18b]
	/lib/libc.so.6(clone+0x5e) [0x4022709e]

Created attachment 122736 [details] [review]
New NEW patch!!!1!one!

Bertrand:

I was unable to reproduce the crash you mentioned but based on the console output it looks like either a race or some random order of disposal that's different.  This patch should fix the case where the player gets disposed before the OpenVP extension.

With this patch, I still have the same crash on exit.
Tell me if I can do more to help track this one down.

Created attachment 122753 [details] [review]
Work around the crash on exit

Here's a ridiculously bad patch to prevent my crash on exit.
It has to be applied on top of the latest one from Chris.

Doesn't make much sense to me, unless strange things are happening to player->vis_resampler...

(In reply to comment #12)
> With this patch, I still have the same crash on exit.
> Tell me if I can do more to help track this one down.

Please try with the latest Banshee.OpenVP.  I believe the bug was located there.

I updated Banshee.OpenVP from svn trunk, but I still have the same crash. :(

(In reply to comment #15)
> I updated Banshee.OpenVP from svn trunk, but I still have the same crash. :(

Then this crash is a problem with Banshee itself, where it's disposing of two objects simultaneously: the GStreamer.PlayerEngine and the stack of sources.  Can we modify Banshee so it's not disposing of stuff from more than one thread during shutdown?

Chris' latest patch is fine for me, my crash is caused by a problem in Banshee.GStreamer.PlayerEngine. See bug #561281.

Btw, CPU usage is normal when the visualization source is not selected.

Created attachment 126886 [details] [review]
Fantabulous patch

New patch.  Fixes issues with the spectrum analysis and also includes a patch for #561281.

Created attachment 127031 [details] [review]
Stupendicular patch

Ok, this is the last patch for a while.  Sorry about all the noise.

This patch fixes a lot of timing and general "odd behavior" problems, improves display of the visualizations by giving them data slightly ahead of time (reduces laggy appearance) and fixes one rare segfault caused by a race.

Created attachment 127230 [details] [review]
New patch against r4961

Updated patch that applies cleanly to r4961.

With a couple minute scan, this looks fine, but I'll leave it Aaron to fully review since the libbanshee stuff is his bag.

Note that this bug is said to depend on #561281.  This is true, but the last few patches on this bug include the patch I developed to fix that other bug.  I'll leave this up to you guys to figure out how to best handle this.

Give me a bit to come up with another patch.  There are some lingering issues I found yesterday while applying it to a fresh Banshee checkout.  I'm not exactly sure why since it applied cleanly but apparently something has changed that caused the patch to screw up without failing.

Created attachment 128403 [details] [review]
New patch against r5021

Updated patch.  Fix calling of a nonexistent function and remove that function's prototype from libbanshee/banshee-player-vis.h.

Created attachment 133239 [details] [review]
Patch updated to work on git master

Updated to build, and started testing and reviewing. I'll dig deeper tomorrow. Right now I am seeing unacceptably high CPU usage (20-30%) when visualization is turned on. However, this patch is good for no CPU extra usage when visualization is turned off.

I'd like to commit this ASAP and focus on reducing CPU usage for the enabled case. I'll follow up tomorrow, hopefully committing.

I have created a new branch from stable called stable-vis where this patch is now applied [1].

Further CPU reduction work will happen on this branch and is of high priority to me, so I am going to close this bug. This is a good patch that does a good job at cleanly accomplishing the desired pass of CPU reduction.

When the pending further CPU reduction work is finished, the stable-vis branch will merge back into the stable branch, and from there will port up to master.

Thanks Chris!

[1] http://git.gnome.org/cgit/banshee/commit/?h=stable-vis&id=d712cedfd86997339850f00edcab4eddeeb02416