Created attachment 110728 [details] [review]
Add support for POPM in banshee

M      src/Core/Banshee.ThickClient/Banshee.Gui/TrackActions.cs
M      src/Core/Banshee.Core/Makefile.am
M      src/Core/Banshee.Core/Banshee.Core.mdp
M      src/Core/Banshee.Core/Banshee.Streaming/SaveTrackMetadataJob.cs
M      src/Core/Banshee.Core/Banshee.Streaming/StreamTagger.cs
A      src/Core/Banshee.Core/Banshee.Streaming/StreamRatingTagger.cs

Hey Nick, this looks quite good.  A couple things:

Please use // for comments, not /* */.

Put spaces between method names and the argument list, eg method (args).  (See HACKING for more detailed style guidelines).

Lastly, I'm concerned about the write-to-files you queue up in the rating method.  I would prefer that we instead nudged a background process that did this:

while (track = "select track from CoreTracks where LastSyncedAt < LastUpdatedAt" != null) {
  queue track save job
}

This has a few benefits, including writing data to disk on the next Banshee run if, say, the app crashed after writing some info to the db.  It also means we don't have to call write-to-disk from every place we Save (), or from within TrackInfo.Save ().  I don't expect you to write this (though you're welcome to), but if you don't, please remove the write-to-disk on rating for now.  You can add a TODO there if you want.  Note that we don't have a LastSyncedAt column atm.

Anyway, great job, this looks like a fantastic first patch.

I've attached an updated copy of the patch with fixed formatting.

I've omitted the new threading model that you mention, as it falls outside the scope of this patch and should be introduced separately. This feature does not add any writes beyond what already occurs within trunk. While you are able to queue several writes by setting ratings on multiple files, you are also already able to do this by renaming the artist on multiple files. And in either situation, the writes are disabled if the "Write metadata to files" option is turned off.

To be specific, the actual write is performed at "file.Save()", which is already a part of trunk.

I've created a new feature request for the new queueing model: http://bugzilla.gnome.org/show_bug.cgi?id=533502

Created attachment 111025 [details] [review]
Revised patch with formatting changes

Created attachment 111026 [details] [review]
Re-revised with some additional formatting changes that I missed in the last pass :)

Created attachment 111660 [details] [review]
Updated to compile with current trunk

Nick, that patch is looking really good.  The big thing I want to see before committing it is a Preference checkbox enabling reading/writing this information.  Should be very easy to add - Aaron has done a great job w/ the preferences stuff.  See http://svn.gnome.org/viewvc/banshee/trunk/banshee/src/Extensions/Banshee.AudioCd/Banshee.AudioCd/AudioCdService.cs?revision=3969&view=markup for a simple example of adding preferences.

Created attachment 118896 [details] [review]
Includes requested preference option which is currently labeled "Read/Write ratings metadata to files"

Diffed against current SVN

The old track editor was just removed from SVN, so your patch doesn't apply anymore. I guess you also have to make the change in the new track editor.

I think there might be an issue in the following change :

--- src/Core/Banshee.Core/Banshee.Streaming/SaveTrackMetadataJob.cs	(revision 4559)
+++ src/Core/Banshee.Core/Banshee.Streaming/SaveTrackMetadataJob.cs	(working copy)
@@ -49,7 +49,7 @@
     
         public void Run ()
         {
-            if (!LibrarySchema.WriteMetadata.Get ()) {
+            if (!LibrarySchema.WriteMetadata.Get () && !LibrarySchema.WriteRatings.Get ()) {
                 Console.WriteLine ("Skipping scheduled metadata write, preference disabled after scheduling");
                 return;
             }

If WriteMetadata is false and WriteRatings is true, the method will not return and write all the metadata to the file.

Created attachment 118977 [details] [review]
Updated to match new track editor

Whoops, you're right, I must've lost an if-statement when cleaning up svn conflicts.

I also updated some of the function style in StreamRatingTagger and reworded the preferences option to be more accurate: At the moment, rating reads are always performed alongside the rest of the metadata import. Only writes are optional.

Nick, this is really a great patch.  A couple more nitpicks:

1) if () not if() in a few places
2) I know it's not pretty, but can we change WriteRatings => WriteUserMetadata or WriteRatingAndPlayCount, and "Write _ratings metadata to files" => "Write _ratings and play counts.." etc

Created attachment 120073 [details] [review]
Minor cleanup (and updated to work with current HEAD)

Function names updated. I didn't see any instances of "if()", but I did see a couple "function()"s. Those have also been fixed.

Created attachment 120975 [details] [review]
Functional update: Overwrite all POPMs with new values if the option is enabled

This patch has a few functional changes from before. There's been a lot of changes since this feature was first submitted, so here's a quick recap of how things are now:

- A new option named "Store ratings within supported files" has been added. Ratings/playcounts will only be written to files if this option is enabled, and only if those files already have an ID3v2 tag (the patch supports using other tag formats in the future).
- Known-good ratings/playcounts are always imported, regardless of whether the new option is enabled. A rating/playcount is known-good if its creator is listed in the POPM_creator_list array. Currently listed are Banshee, WMP/Vista Explorer, and MediaMonkey.
- Exported ratings/playcounts are saved to each POPM frame in the file, regardless of who created them. A given file can contain multiple POPM frames, with a different owner for each frame. If no known-good POPM frames are found in the file, a new frame labeled "Banshee" will also be created.
- Ratings/playcounts are written whenever the rating is changed. In other words, the playcount is only updated when you manually change the rating on that file, NOT whenever the file is played.

Let me reiterate that imports will only pull info from known-good POPM frames, while exports are written to all POPM frames. This is okay because writing is disabled unless enabled by the user.

Thanks a lot Nick, this is quite solid.  At some point we might add the Rating code directly into TagLib#, but that can happen after this actually lands.

Could you add different checkboxes for saving ratings and playcounts?

*** Bug 500034 has been marked as a duplicate of this bug. ***

Created attachment 126395 [details] [review]
Ogg Comment support (used in Vorbis, FLAC, and Speex files)

This update adds support for Ogg metadata, which is commonly used by Vorbis, FLAC, and Speex files. It follows the standard that has been published by the Quod Libet team (see http://code.google.com/p/quodlibet/wiki/Specs_VorbisComments )

On the ID3v2 side, this update adds import support for Quod Libet-tagged files (assuming that Quod Libet's default creator name is used).

I want to get this in, but I'm not comfortable adding yet another set of things to queue up per-track when we hit save.  I have some work to change the write-to-file and rename-files/folders jobs to background jobs that go based on the UpdatedAt stamp of each track (thereby using CoreTracks as a queue for what needs to be done), and will resume if you restart Banshee, etc.  I'll update here when I've got that in.

(In reply to comment #17)
> Created an attachment (id=126395) [edit]
> 
Thank you Nick! Great patch, works as expected.

Created attachment 129814 [details] [review]
Updated to trunk

There was a conflict with the recently commited sorting patch, this one should apply to r5089.

Great! Eager for seeing that in trunk ;)

Created attachment 131065 [details] [review]
Updated to trunk

Fixed a conflict in the Preferences dialogue. Also added iTSfv [1] to the list of known POPM creators.

[1] http://itsfv.sourceforge.net/

Ok, two things I think need to be done before I'm comfortable committing this:

1)  Let's factor the taglib#-rating stuff into a patch against taglib# itself, it's gotten big enough that it's pretty ugly in the context of Banshee

2)  Split the remainder into two patches: the part that does the read/write and the part that does the scheduling, because

3)  The scheduling blocks on the new jobs work (bug #577772).  I'm just not comfortable turning what is currently a very fast operation (rating however many tracks at a time you want) into something that could trigger a ton of CPU and memory use (to queue all those jobs in the Banshee.Kernel).  The new jobs stuff is addressing this issue. 

Created attachment 133721 [details] [review]
Using new job scheduling

This patch uses the the new job scheduler to write ratings and play counts.

> 1)  Let's factor the taglib#-rating stuff into a patch against taglib# itself,
> it's gotten big enough that it's pretty ugly in the context of Banshee

I will do this later.

> 2)  Split the remainder into two patches: the part that does the read/write and
> the part that does the scheduling, because
> 
> 3)  The scheduling blocks on the new jobs work (bug #577772).  I'm just not
> comfortable turning what is currently a very fast operation (rating however
> many tracks at a time you want) into something that could trigger a ton of CPU
> and memory use (to queue all those jobs in the Banshee.Kernel).  The new jobs
> stuff is addressing this issue. 
> 

I guess now that the new scheduler is already in the master branch this split is no longer needed. 

On a side note, I see a lot of random crashes described in bug 580044. However, the nice thing about the new scheduler is that it writes ratings next time I start Banshee ;)

Thanks for fixing this up, Alexander.  It looks good.  One tiny nitpick:

+            }
+            if (write_rating_and_play_count) {

put a newline between those lines, and also check PlayCount in the unit test.  You can leave the rating code in this patch for now; this had been long enough coming, don't need to slow it down more because of that.

In StreamRatingTagger.cs make all the classes except StreamRatingTagger internal.

Should we have a checkbox in the Import dialog asking whether to import the ratings/playcounts?  I'm thinking we should.  Aaron?

Created attachment 134458 [details] [review]
Updated to comment #25

(In reply to comment #25)
> Should we have a checkbox in the Import dialog asking whether to import the
> ratings/playcounts?  I'm thinking we should.  Aaron?

Let me know if you want this and I will update the patch. However, I personally don't think it's such a useful feature. Why would someone *not* want to import their ratings?

(In reply to comment #26)
> Let me know if you want this and I will update the patch. However, I personally
> don't think it's such a useful feature. Why would someone *not* want to import
> their ratings?
> 

When running this patch, I've actually had instances where I've inherited other family members' ratings that they'd set in WMP, so it might be worth considering this even if it's somewhat of an edge case.

Another useful feature would be "flush my current in-banshee ratings to the selected files", in the event that someone did an upgrade to a version of banshee containing this patch and wants to export the metadata they have in the banshee DB. Although this should probably be done in a separate patch.

Created attachment 135707 [details] [review]
Added an option to the Import dialogue to import ratings and play counts

Removing dependency on bug 577772 because the last patch uses the new job scheduler.

Adding dependency on bug 559013. The "import ratings and play counts" option is duplicated in some other importers (e.g. iTunes importer from bug 441093).

Ideally the fix for bug 559013 should allow importers to inject their custom options into the import dialogue.

Created attachment 136533 [details] [review]
Resync to current Git -- otherwise same as June 1 patch

Noticed that the June 1 patch was failing against current HEAD with TaglibReadWriteTests.cs and StreamTagger.cs, this patch should fix it (no functional changes).

Thanks Nick, marking the June 1st patch as obsolete.

(In reply to comment #31)
> Created an attachment (id=136533) [edit]
> Resync to current Git -- otherwise same as June 1 patch
 
I have been trying this last patch to import ratings from Windows Media Player and it seems to work perfectly

(In reply to comment #33)
> I have been trying this last patch to import ratings from Windows Media Player
> and it seems to work perfectly
> 

Just to let everyone know, I'm keeping an eye on thins bug. As soon as blocking bug 559013 is finalised, I will update the patch to use the import dialogue as described in comment #25.

Besides thanking and thanking for that great patch, just a suggestion... what about importing the ratings on rescanning the library? Thanks again ;)

Hey guys,

I was just pointed this way and would like to make some comments.

First off, regarding a specific item in the patch: The POPM frame specifies that an email address be used as the identifier. While WMP9 already breaks spec here, that's not a great reason for you guys to ignore the spec as well  :-|  (granted, it's already breaking the spec not to use an email address to the actual user, but...)

Second, before you apply this and start getting peoples' songs tagged, I'd like to make a proposal.

We (the Amarok team) have had discussions with various other media players in the past about implementing this kind of functionality not just for MP3s but for other file types as well. The problem is that while there's already ambiguity in the MP3 spec (no user wants their email address embedded in their files, so what do you use? and, do you use the entire 1-255 scale, or just the values you use in your application?) there is even more ambiguity outside of MP3, for instance Ogg with VorbisComments where there's just a key:value pair.

The problem is getting everyone to agree on something common. But if we can get most FOSS players to agree on common email/key names and common values, it will be a win for all of us, because it will increase interoperability across all of our products.

Although Banshee doesn't (AFAIK) export a MPRIS interface ( http://wiki.xmms2.xmms.se/wiki/Media_Player_Interfaces#About_MPRIS ), in the same spirit in which MPRIS was born, I'd like to propose that you and we work together, and work with other players as well (XMMS2, VLC, BMPx, Audacious, maybe Songbird if they'll play ball) and come up with a standard, cross-file-format naming and rating system. While this wouldn't prevent importing from WMP9/iTunes/whatever, it would allow the majority of the FOSS players to write and read in a common format, without having to hack around each others' systems.

I don't think this would be terribly difficult or take a terribly long time to do -- it just requires actually getting everyones' heads together.

Thoughts?

P.S. I'm willing to put in the legwork of contacting other projects.

For Ogg tags, the patch follows the schema provided here: http://code.google.com/p/quodlibet/wiki/Specs_VorbisComments

I've seen that schema (which your patch incorrectly refers to as a standard), and it rather proves my point.

First off, several of the vorbiscomments in that schema use an email address. It's true that it allows several users to have their own playcounts/ratings, but for the most part this isn't going to be used (and for now, let's just assume a default of the user not setting their email address). In this case Quod Libet is using its email address, and your patch uses BANSHEE. So if a user plays a file in Banshee, and then plays it in Quod Libet, is Banshee going to scan the file the next time the user plays it in Banshee to see if some other players' playcount tags have been updated (which requires knowing what it was before) and then add that playcount onto the Banshee tag? By implementing it the way the patch does, you're essentially guaranteeing that users that use multiple players (or change players from one to the next) have no meaningful data for the other player, other than maybe during an initial import when you're specifically looking for that data.

If you were to work with us and other players to develop a standard naming scheme, we could easily all read each others' playcount and rating information, because it would be as simple as reading the same information you use.

There are several other problems with the Quod Libet schema having to do with ratings that would be nice to have players agree upon:

* The value is a floating point value between 0.0 and 1.0, inclusive. Does 0 mean it's a terrible track or that there is no rating?

* The schema says that no rating value present should assume it to be 0.5, but that incorrectly causes unrated tracks to be rated middle-of-the-road. We've specifically had users request the ability for a song to have no rating at all, but as per my point above, that's not defined.

* 0.0-1.0 gives it a 0-100 range, which is easy to map to things like 0-10 ratings in an application, or zero to five stars. What is unclear is how it maps to the 0-255 defined in the ID3v2 spec. Should apps implementing this use 0-100 in ID3, rounding to the nearest whole number? Should they map it out to the range of 0-255?

These questions should be answered if we want to have different players interacting. Quod Libet developed their schema in a vacuum and threw it out there, but it's insufficient for interoperability. I'd rather do the legwork and work with you and others to put together a multiple-player standard that leaves no ambiguity and that uses a common identification scheme for when a user-defined email/identifier isn't set, so that we can all benefit.

Hey Jeff,

I'm keen on standardizing among FOSS players at least how we handle ratings.  I don't have a lot of time or energy to devote to it, but as long as the resulting spec lets us (Banshee) consistently save/read our 1 - 5 stars (we treat 0 as unrated in our database, and I assume we'd just not write out a rating to file for such tracks), I'm happy.

Hullo, I'm a Songbird developer and since Jeff contacted me, I figured I would chime in.

I'd love it if there were a simple, sane, unambiguous standard. As far as I'm concerned, there are three issues here:

1) how many values are permitted per file (one per user? per player? per file?)
2) where do we find the value?
3) what does it store?

Given that I would prefer to err on the side of simplicity and consistency in the hopes that more developers will adopt any selected standard, I'd recommend these answers:

1) One rating / playcount per file. Users will want ratings to persist changes in media player.
2) "rating". "playcount". Just like that, as adapted to the file format's standard. For ape, this would read "Rating", for Vorbis comment, "RATING". For ID3v2, use a TXXX. I think adopting POPM is a mistake.

3) I favor an integer range of 1-100 for the rating, though floating point values strike me as reasonable too. To indicate an unrated file, simply do not store a rating value.

In order to make the values unambiguous, we could consider storing rating values as a ratio explicitly. In other words values would look like: "0.334/1.0", "4/5", "99/100".

If we can't reach a consensus, Songbird will continue to do what it has done for several releases now, which is storing our rating of 0-5 stars (if the users enable writing ratings to file) into the "rating" field. I'd certainly be open to scaling this to 0-100 to improve interoperability.

Well, the stuff you put here is rather contrary to many of the things we already discussed on IRC.  :-|  I also want simplicity and unambiguity but for reasons already discussed (but not here) I think it requires a bit more than that.

I think that reaching a consensus may mean some compromising, so no one can really go into it with the idea of "if it's not exactly what I want, I'll just go on and do it my own way"...that defeats the point. 

All that being said, I'd like to wait a day or two for replies from some of the other player developers I contacted, and if either of you know how to get in touch with the BMPx or Audacious folks that would be good too (couldn't find addresses for them, although they worked with us on the MPRIS spec). Once we figure out who's interested in getting this settled, I'll set up a mailing list on Google Groups or some such thing and we can take the discussion there.

(In reply to comment #42)
> Well, the stuff you put here is rather contrary to many of the things we
> already discussed on IRC.  :-|  I also want simplicity and unambiguity but for
> reasons already discussed (but not here) I think it requires a bit more than
> that.
> 

As clarified in IRC, it's not. :) I know every project is going to have its own ideas, so hopefully we can discuss this openly and reach a shared solution.

As reported by nickbp on IRC, the last patch doesn't work with git master anymore: the writes don't happen when the user changes rating.

Commit f86324f [1] is causing this. The patch needs to be updated to treat ratings/playcounts as non-transient fields when the write ratings/playcounts option is set.

[1] http://git.gnome.org/cgit/banshee/commit/?id=f86324f4a31230968c26ec0b501944af83940e71

Created attachment 146178 [details] [review]
Support transient_fields (has a flaw, see comments) and rebase to current git (fix new incompatibility in MassStorageSource.cs)

This patch has the following updates from the previous version:

1) MassStorageSource.cs recently added a call to StreamTagger.SaveToFile. Update that call to support the two new parameters introduced by this patch.

2) Exclude Rating and Playcount database fields from "transient_fields" if enabled in preferences. My quick fix has a flaw: I don't believe the value of transient_fields is automatically updated after the user has changed their preferences. This means that after toggling the "write ratings/playcounts" option in the preferences, you'll need to quit and reopen banshee before it'll really take effect. This fix is just meant to be a stopgap measure so that the patch still works in Git until BGO #585896 is complete.

FYI: You can check that banshee is writing ratings by running "banshee-1 --debug" and checking the output. If you see lines in stdout that look like this, then your ratings are successfully being written to disk:
[Debug 12:02:18.946] Exporting ID3v2 Rating=5(255) and Playcount=2(2) to File "/home/nick/Music/exceptionally new music/65daysofstatic - one time for all time - 09 - radio protector.mp3" as Creator "Windows Media Player 9 Series"

Bulk changing the assignee to banshee-maint@gnome.bugs to make it easier for people to get updated on all banshee bugs by following that address.  It's usually quite apparent who is working on a given bug by the comments and/or patches attached.

Just an update -- there are about 7 players involved in the spec discussion, which is ongoing (including a banshee dev). I'll post here as progress warrants.

@Jeff: I just came across this ticket and the mentioned standardization discussion. As a developer of MusicBrainz Picard Tagger I would like to know more about this discussion. We added support for ratings in Picard 0.12, using a configurable e-mail as identifier and the quodlibet schema for vorbis comments. How can we, the Picard developers, get involved into the discussion? Are there any public references for the state of the discussion?

Thanks,
Philipp

@Jeff, I'm developer of the jaudiotagger tagging library used in a number of opensource taggers/players such as Jukes, aTunes, Jajuk and I would be interested in this. Jaudiotagger supports alot of fields ( see http://www.jthink.net/jaudiotagger/tagmapping.html ) , but I have shied away from rating because of the issues in this thread.

Hey Philipp and Paul,

So far it's just been six or so emails sent around to a group. I'm going to start a Google Group (cause it's quick and easy) and forward the existing messages to it so that it's public and accessible, and will make sure you guys get a link, so that you can get full backlog of the discussion up to this point.

I came up with a more complex list for application compatibility, since Media Monkey decided (in version 3.1) to make their ratings more compatible with WMP...

POPM=0,2-8 → Rating=0
POPM=1,9-28,30-53 → Rating=1
POPM=29,54-59,60-113 → Rating=2
POPM=114-167 → Rating=3
POPM=168-218 → Rating=4
POPM=219-255 → Rating=5 

Source:
http://www.mediamonkey.com/forum/viewtopic.php?f=7&t=40532&p=226587

Moreover, MusicBrainz Picard writes those ratings as per it's code (as of version 0.12):
0  ->0star
51 ->1star
102->2star
153->3star
204->4star
255->5star
And allows for multiple POPM tags. I am working on a compatibility option that allows only 1 POPM tag to be present.
This is a messy ticket, but I simply post my findings as I go along.
http://bugs.musicbrainz.org/ticket/5471#comment:18

IMO we would change our code to write the ratings Banshee is set to use. Is your list final, or is it open to change?

Who are you talking to and which list are you referencing?

I was talking to everyone involved in this ticket.

The list can be found at the link I posted.
[MM3.1.0.1256's POPM and Rating]

POPM=0 → Rating=0
POPM=1 → Rating=1
POPM=2-8 → Rating=0
POPM=9-18 → Rating=0.5
POPM=19-28 → Rating=1 (MM2.5:Rating=0.5 → POPM=28) (MM3.0:Rating=0.5 → POPM=26)
POPM=29 → Rating=1.5
POPM=30-39 → Rating=0.5
POPM=40-49 → Rating=1
POPM=50-59 → Rating=1.5 (MM2.5:Rating=1 → POPM=53) (MM3.0:Rating=1 → POPM=51)
POPM=60-69 → Rating=2
POPM=70-90 → Rating=1.5
POPM=91-113 → Rating=2
POPM=114-123 → Rating=2.5
POPM=124-133 → Rating=3 (MM2.5:Rating=2.5 → POPM=129) (MM3.0:Rating=2.5 → POPM=128)
POPM=134-141 → Rating=2.5
POPM=142-167 → Rating=3
POPM=168-191 → Rating=3.5
POPM=192-218 → Rating=4
POPM=219-247 → Rating=4.5
POPM=248-255 → Rating=5


[WMP11's Rating and POPM]

Raing=0 → POPM=0
Raing=1 → POPM=1
Raing=2 → POPM=64
Raing=3 → POPM=128
Raing=4 → POPM=196
Raing=5 → POPM=255

This is my proposed list for reading ID3v2 POPM tags. as you know ID3v2 POPM tags use a scale from 0-255. You can cross check this, but I think it should make sense. So if:
POPM=0,2-8 → Rating=0
POPM=1,9-28,30-53 → Rating=1
POPM=29,54-59,60-113 → Rating=2
POPM=114-167 → Rating=3
POPM=168-218 → Rating=4
POPM=219-255 → Rating=5

Review of attachment 146178 [details] [review]:

Nick, I committed the patch with a few changes:

 * transient_fields are updated when the user changes the preference.
 * Removed the option from the import dialogue as suggested by Gabriel.
 * Minor clean up.

Thanks to everyone involved, that was a long road!

Jeff, Peter, Philipp, Paul,

After discussing this among Banshee developers we decided to commit the patch as is, without waiting for the final spec. I created a new bug 602158 to make Banshee compliant to the spec, could you comment in it when/if there's a consensus or when you have more information about it such as the link to the Google Group, etc.

Closing the bug as FIXED.

(In reply to comment #15)
> Could you add different checkboxes for saving ratings and playcounts?

See bug 602159

(In reply to comment #56)
> Jeff, Peter, Philipp, Paul,
> 
> After discussing this among Banshee developers we decided to commit the patch
> as is, without waiting for the final spec. I created a new bug 602158 to make
> Banshee compliant to the spec, could you comment in it when/if there's a
> consensus or when you have more information about it such as the link to the
> Google Group, etc.

A heads-up would have been nice, so I could have given you this information:

There have already been three drafts of the spec, the latter two of which were posted to the xdg@freedesktop.org list, the latest of which is here:

http://lists.freedesktop.org/archives/xdg/2009-November/011120.html

It seems quite premature of you guys to commit this patch right now. Gabriel was on the original mailing thread discussing the spec and received (or at least, was sent) several notifications of the discussion moving to the XDG list, so he should be aware of the current progress. We seem to be fairly close to a consensus -- at least, for those that are actually actively discussing it. I wouldn't be surprised if the spec is finalized fairly soon after I get back from vacation, unless someone brings up issues in the interim.

This patch adds support for the current in-the-wild metadata. Once the new standard is finalized, support can be added. In either case, users will want the ability to parse their existing/"legacy" metadata, and I doubt WMP et al are going to be fast about supporting the new standard.

I'm looking for this feature a long time already and i'm happy with the decision that this patch is added already now. Switching to the other standard is just a new patch, and as Nick said, without support for the quasi std. of WMP or Itunes their will be no real use in a std. by the Linux Community.

(In reply to comment #60)
> as Nick said, without support for the quasi std. of
> WMP or Itunes their will be no real use in a std. by the Linux Community."

That's not what he said, but if anyone thinks that iTunes or Winamp or WMP will *ever* adopt these conventions, I've got some nice land in Siberia they might be interested in. Adoption by them was never the point -- the point is for the various free music players to interoperate with each other, which will be a far cry better than what we have now. If any of the bug boys adopt it, score.

By committing that patch now, one of two things is going to happen: either the Banshee guys will never bother adopting the new standard, because they already have something in there that works well enough for their uses; or they'll adopt the new standard but have to then write even more code to convert ratings from their short-lived scheme that they just implemented to the new standard. The first does a disservice to their users; the second just makes more work for them.

The better approach probably would have been to work on changing the patch to support the spec -- this will have to be done anyways, and by working on this now, they could have significantly shortened the time between the spec being finalized and them adopting it. Banshee's been around for many, many years and I don't see why a couple of more weeks of waiting (and perhaps commenting on the spec to indicate if they are happy with it or if they feel there are issues) is such a killer.