New moodbar implementation

Joe Rabinoff rabinoff at post.harvard.edu
Wed Aug 9 08:32:59 UTC 2006


Hi all,

I have completely rewritten the Moodbar implementation.  There is a
test patch at 
   http://shadowfax.homelinux.net/~guru/moodbar/
I discussed this on #amarok.dev with many of you.  As there was 
a lot of debate and no decision about what I should do with this sweet
patch, I thought I'd email you a summary of what I did and start a 
flamewar :)

There is now a separate binary, totally independent of Amarok, which
does the actual analysis.  This binary uses the GStreamer libraries
(and a couple of plugins that I wrote) to read the file, but is in no
way related to the Gst engine.  The code in Amarok proper launches
this process when it needs to create a .mood file.

All of the code in Amarok proper is new, except for some purely
algorithmic snippets from Gav Wood's code.  This code has no library
dependencies whatsoever; it just needs to be able to find the analyzer
binary.

I think my code gets rid of a lot of the problems in the old moodbar:

 * moodbar had memory leaks.
   As I understand it, most of these leaks happened in the Exscalibar
   library.  My patch does not use Exscalibar, and the analyzer is a
   separate process anyway.  There is only one place in the entire 
   patch where I allocate something on the heap, so I dare you to
   valgrind it.

 * moodbar was unstable.
   It's not surprising, since moodbar support was hacked into the 
   Amarok source in a none-too-pretty fashion.  The code I wrote is 
   good, and is meant to be hard to break -- please take a look at it.
   Again, since the analyzer is in a separate binary, that can no longer
   crash Amarok.

 * the moodbar code was all over the place.
   Almost all of the moodbar-related code is now in the well-commented
   moodbar.{cpp,h}.  There is a small amount of code in sliderwidget.cpp
   to add moodbar support to the PrettySlider, and in playlist.cpp and
   playlistitem.cpp, to add the Mood column.  Of course Options1.ui and
   amarok.kcfg have some new (old) code, and there are trivial amounts 
   (<10 lines) of support code in a few other files.  Search for 
   HAVE_MOODBAR to see exactly how much has been added.

 * moodbar was memory-consuming eye-candy
   First of all, I tried to make my code add as little overhead as 
   possible, except where you're actually displaying a Moodbar.  In any 
   case, in this patch, moodbar can be disabled at compile time or in the 
   config dialog if you think it's too big.

 * exscalibar was unmaintained
   I don't think anybody's going to be dropping GStreamer anytime soon,
   and I'll do any required maintenence on my moodbar plugin, which btw
   is very simple.

 * nobody was fixing the bugs in moodbar
   ... and I don't blame them, the old code was unmaintainable.  That
   said, I hereby volunteer to maintain the moodbar code, at least until
   such a time as it stabilizes (nb it's very stable already).  Assign me
   any relevant bugreports.  I don't think I need SVN write access or an 
   email address @kde.org or anything.

Some other notes about this patch:

 * If you pass --without-moodbar to configure, all but about 10 lines
   of added code is #ifdef'd away.  Annoyingly, moc doesn't use the
   preparser, so signals and slots can't be entirely gotten rid of,
   although the code inside the { } can be preprocessed away :)

 * The new analyzer is marginally faster than the old one, and is also
   more accurate (Gav wasn't calculating his Fourier transform correctly).

 * There's an overview of the moodbar implementation at the beginning of
   moodbar.cpp.

 * From the forums and the irc channel, I know I'm not the only one who
   misses the moodbar :)

The discussion on #amarok.dev makes it clear that there are packaging
issues with making gst a dependency for a minor feature like the Moodbar.
Several solutions were suggested: (1) it's ok like it is (my favorite), (2) 
package the analyzer binary as a separate tarball, with it's own website
etc. (eean's suggestion), (3) try to hack together a xine-based analyzer
binary (probably can be done, but it would be a hack since xine really
isn't set up for this kind of thing, whereas gst is), and (4) drop it entirely
pending a full-fledged generic plugin api.

I think the gstreamer solution is elegant, plus it already works, and works
well.  That's my case for (1).  But I know nothing about packaging, as I don't
use a distro, so I understand if it's not feasible.  In that case I can do (2)
easily enough; the analyzer-binary package would be tiny and would require
almost no maintenance.  I can look into (3) if you really want.  As for (4),
since the code is all in one place now, it's easily enough converted into a
plugin when the time comes, so since I've already done all the work, I see no
reason not to re-implement the moodbar for the 1.4 series, as it is a popular
feature.

Anyway, please do check out the code, and thanks for your time.  I'll /join
#amarok.dev again tomorrow.

Joe Rabinoff (QBob) (bobqwatson at yahoo.com also)

PS -- I wrote the iPod-style view too, so this isn't my first patch :)




More information about the Amarok mailing list