Version: (using KDE KDE 3.4.3) Installed from: Ubuntu Packages In iTunes there is a field to add tempo for the songs. I really wish this was available in amaroK as well. By the way, thanks for adding the composer-field.
Created attachment 15714 [details] Patch against amarok-svn 532128. Applying this patch against 532128 compiles and works for me. I've chosen to use a float number for the bpm since programs that calculate bpm automatically often return a floating point number.
The above patch implements what I was asking for. I've used the term bpm instead of tempo since that seems to be the norm.
Unfortunately your patch does not implement that completely: adding a new field to the tags table breaks a lot of things, as far as I can remember this is at least: - dragging smart playlists to the current playlist - transferring smart playlists to media devices (This is because many queries don't specify the fields from the tags table they are interested in, but they just assume a certain number and order for them.) In general, watch out for the magic number '12' in the source code! In addition, smart & dynamic playlists should support matches for BPM, too.
Created attachment 15895 [details] New patch that works with smart playlists > - dragging smart playlists to the current playlist Fixed. > - transferring smart playlists to media devices Not tested, but should work. > (This is because many queries don't specify the fields from the tags table they are interested in, but they just assume a certain number and order for them.) > In general, watch out for the magic number '12' in the source code! First part of the paranthesis doesn't seem to be true anymore, but the latter was correct. The magic number was '11' though, but it's 12 now. :) > In addition, smart & dynamic playlists should support matches for BPM, too. Smart playlists support matches for BPM now. Don't understand what has to be done with dynamic playlists to support matches for BPM. Anyways, let me know if there is anything else that needs to be added. Finally, please look at the changes in the dcop and decide wether the calls to bundle.save() are redundant or not.
Created attachment 15914 [details] Implements bpm-field support for amarok Added support for bpm on ipods. It all seems to work very well for me. Hope you'll be able to use this patch.
Created attachment 15979 [details] Updated patch that fixes a bug in the patch. .toFloat() was run without stripping the QString first. This resulted in not being able to read some bpm tags in vorbiscomment that had \n at the end.
Alf, could you please update your patch for current svn so that we can consider it for application?
Created attachment 16116 [details] Patch against svn revision 541427 Works for me. Let me know if I have overlooked something as I don't use all features of amaroK.
Created attachment 16381 [details] updated patch against 546976 Keeping it up to date.
Created attachment 16391 [details] Fixed a few bugs with dynamic and smart playlists Fixed a problem with dynamic playlists, forgot to increase a couple of values. Also fixed a problem with the smartplaylisteditor. Forgot to add the BPM field to the getValueType method.
Created attachment 16587 [details] Updated patch to include collectionbrowser Seems like I forgot a few changes in collectionbrowser.{h,cpp}. Fixed. Patch now applies to 551217.
Created attachment 16921 [details] updated patch against 559626 Updated the patch to apply cleanly against r559626. Haven't tested thoroughly yet. Note that currently does not work with ---with-mp4v2 enabled, since Taglib::MP4 does not support a bpm tag (at least in the version I am using).
Comment on attachment 16587 [details] Updated patch to include collectionbrowser Seems that attachment 16921 [details] makes 16587 obsolete.
Created attachment 16952 [details] adds bpm support for mp4 files against 559637 Alf, the TagLib::MP4::Tag in amarok/src/metadata/mp4/mp4tag.* does not have members bpm() and setBpm() which are called in metabundle.cpp in your patch... Were you using a custom version of taglib that included those, or did you not try buidling with `configure ---with-mp4v2` ? In case of the latter, here's a simple patch that should add these missing functions. Note however, this MP4V2_HAS_WRITE_BUG business (mp4file.cpp) is making me nervous: Someone who has mp4 files with BPM metadata should back up their files and test this. (I'll get around to it eventually...) Applying this patch along with attachment 16921 [details] will allow --with-mp4v2 to succeed again.
Sitting down to test this a bit thoroughly, there's a bug that in the smart playlist editor, if you add a modified date criterion, amarok expects a string value, and if you add a file path criterion, it expects a date! To late to fix it tonight... :)
One more bug before I forget: in the collection browser, if you are showing a flat view and you display the BPM column and sort by that column, it is sorted in string order instead of numeric order (200 comes before 90). Also, the column is formatted as %.0f, whereas in the playlist table, bpm is formatted as %f (I prefer the latter). (All the bpms I have tested with are integers-- not sure of the behavior if the value has a factional value-- does the playlist table round the values?)
Regarding #15, just edit smartplaylistbrowser.cpp and in the switch-statement in CriteriaEditor::getValueType at the end replace case 14: with case 15: and it should be good.
comment 16 is a general problem with CollectionItem::compare(). All numerical columns incorrectly sort by string order (Length and Score, for example). I'm not going to worry about it here. (See bug 130667)
Created attachment 16961 [details] updated patch against 559637 updated patch, combines the mp4 bpm support patch (attachment 16952 [details]), fixes the smartplaylisteditor (comment 15), and right-aligns the BPM column (should be right-aligned because it is a meaningful numerica value).
SVN commit 565040 by aumuell: merge bpm patch this means that bpm detection could be implemented as a script now BUG: 123142 CCBUG: 129125 M +2 -0 ChangeLog M +16 -0 src/amarokcore/amarokdcophandler.cpp M +2 -0 src/amarokcore/amarokdcophandler.h M +2 -0 src/amarokcore/amarokdcopiface.h M +7 -0 src/collectionbrowser.cpp M +1 -1 src/collectionbrowser.h M +26 -9 src/collectiondb.cpp M +38 -37 src/collectiondb.h M +1 -0 src/collectionscanner/collectionscanner.cpp M +2 -0 src/mediadevice/ipod/ipodmediadevice.cpp M +48 -18 src/metabundle.cpp M +7 -2 src/metabundle.h M +10 -0 src/playlist.cpp M +3 -0 src/playlistitem.cpp M +1 -0 src/playlistloader.cpp M +1 -0 src/scancontroller.cpp M +9 -6 src/smartplaylisteditor.cpp M +2 -0 src/xmlloader.cpp