Bug 132782

Created attachment 17452 [details]
Moodbar patch to SVN 575816

Feel free to remove the #ifdef HAVE_MOODBAR statements if you want.  If you do,
you shouldn't apply the patch to configure.in.in and configure.in.bot.	I'm of
course willing to tweak it to your liking (esp. after it's applied).  Assign
any relevant bugreports to me.

Just glancing through the code, it certainly looks a lot cleaner than the previous implementation. The one issue I can spot is that Moodbar inherits QObject and has a not insignificant amount of member data itself; thus, in order to conserve memory, I would prefer if MetaBundle stored its Moodbar by pointer rather than value, which would be null unless a moodbar is actually present. Apart from that, if there's no chance of this doing wacky things like killing you or your collection, I'd be fine with it going in. (If there is, then I think it should go in a branch for a couple of weeks before being applied).

Also, I agree with Ian -- if no extra dependencies are required at compile time, then the HAVE_MOODBAR stuff can be removed, and it should just check for the moodbar executable at runtime to determine whether to expose it in the UI.

Gábor: an uninitialized Moodbar should actually be quite small -- its member data is minimal when uninitialized, and most of it is implicitly shared anyway.  That said, it would be very easy to store the moodbar as a pointer, although it would be slightly more dangerous from a memory-management point of view.  In any case, this is the kind of tinkering I would be happy to do after the patch is applied.

I can't imagine any possible way this could mess up your collection.  The only thing I can think of would be if you have have *music* files with a .mood extension, and you choose to store your .mood files with your music; but even then, the moodbar would think the music file was a .mood file and wouldn't overwrite it.  The other issue with storing mood files with music is when a new .mood file is created, it triggers the incremental scanner -- but I think that is a topic for another bug report.

Anyway thanks!

*** This bug has been confirmed by popular vote. ***

SVN commit 576765 by markey:

Interface for new Moodbar implementation by Joe Rabinoff <bobqwatson@yahoo.com>.

BUG: 132782


 M  +8 -0      configure.in.bot  
 M  +23 -0     configure.in.in  
 M  +1 -0      src/Makefile.am  
 M  +206 -2    src/Options1.ui  
 M  +17 -0     src/Options1.ui.h  
 M  +25 -0     src/amarok.h  
 M  +20 -0     src/amarokcore/amarok.kcfg  
 M  +17 -0     src/app.cpp  
 M  +8 -0      src/app.h  
 M  +5 -0      src/columnlist.cpp  
 M  +11 -1     src/configdialog.cpp  
 M  +34 -0     src/metabundle.cpp  
 M  +28 -0     src/metabundle.h  
 M  +11 -2     src/organizecollectiondialog.ui.h  
 M  +7 -2      src/playerwindow.cpp  
 M  +3 -3      src/playerwindow.h  
 M  +61 -0     src/playlist.cpp  
 M  +4 -0      src/playlist.h  
 M  +66 -1     src/playlistitem.cpp  
 M  +5 -0      src/playlistitem.h  
 M  +158 -19   src/sliderwidget.cpp  
 M  +25 -2     src/sliderwidget.h  
 M  +1 -19     src/socketserver.cpp  
 M  +4 -1      src/statusbar/statusbar.cpp  
 M  +3 -2      src/statusbar/statusbar.h