Bug 132782 - New moodbar patch, official submit
Summary: New moodbar patch, official submit
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: Compiled Sources Linux
: NOR wishlist
Target Milestone: ---
Assignee: Amarok Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-08-22 09:20 UTC by Joe Rabinoff
Modified: 2006-08-24 21:15 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments
Moodbar patch to SVN 575816 (100.28 KB, patch)
2006-08-22 09:23 UTC, Joe Rabinoff
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Joe Rabinoff 2006-08-22 09:20:16 UTC
Version:           SVN 575816 (using KDE KDE 3.5.3)
Installed from:    Compiled From Sources
OS:                Linux

The patch to SVN 575816 is attached below.  dangle and gruen0 will attest to its stability.  Check the Moodbar wiki page for news and screenshots.  Feel free to flame me below :)
Comment 1 Joe Rabinoff 2006-08-22 09:23:15 UTC
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.
Comment 2 Gábor Lehel 2006-08-22 22:08:32 UTC
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.
Comment 3 Joe Rabinoff 2006-08-22 22:51:54 UTC
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!
Comment 4 Tyson Key 2006-08-24 18:08:05 UTC
*** This bug has been confirmed by popular vote. ***
Comment 5 Mark Kretschmann 2006-08-24 21:15:26 UTC
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