GNOME Bugzilla – Bug 638130
Banshee doesn't recognizes changes when rescanning Database
Last modified: 2011-12-21 19:33:12 UTC
Overview Banshee doesn't recognizes changes in the tags. Even when rescanning the Database it doesnt recognizes them. Just deleting banshee.db brings banshee to recognize the new version of the song. I tested this for Ratings, Comments etc. with flac and mp3 files. Steps to Reproduce *Add one song to your Banshee music library *For Example rate this song with 5 stars (Banshee writes the change into the file) *Now edit the rating of the song (NOT with banshee, but with an other application (or with Banshee on an other Computer)) *Let Banshee rescan the Database Actual Result Banshee will not recognize the change of the rating/comment/etc. Expected Results Banshee recognizes the change of the rating and displays it correctly (in other words updates the banshee.db for the new rating and shows the shows the song now e.g. with 4 stars.) Additional Information: This bug matters if you sync your Music library (e.g. with Dropbox). If you change the rating of the song on computer A, dropbox updates the file on computer B, but banshee on computer B hasn't got the new rating... Same goes for e.g. comments. It causes in a completly unstructured music library, because some files are wrong displayed on Computer A, some on computer B, depending on which computer a rating/comment/etc. was changed. Deleting banshee.db on every start of banshee is the only solution.
Rescan is supposed to update the metadata in the Banshee db iff the file has been modified more recently than the metadata for the track in Banshee has been updated. Sounds like that is broken.
*** Bug 659719 has been marked as a duplicate of this bug. ***
I have a working patch for this, I've tested it using MP3's only. two things i note, 1) i was using EasyTAG to modify my tags externally, and it had an option "preserve file modification time" which causes banshee's scan to skip the file, thinking it hasn't been touched since last time. 2) I was testing with WMA files(unknowingly) and banshee doesn't write play-counts or ratings to WMA's, so for this to work, one would have to have write [ratings/play-count] enabled for this to work. however, it did work for me, once had EasyTAG write the modified time. Also i did sync my laptop and my desktop over dropbox, played the song on my laptop, and scanned on my desktop, and the play-count did change.
Created attachment 197307 [details] [review] banshee now defaults to the metadata in the file, vs the database
*** Bug 625775 has been marked as a duplicate of this bug. ***
your patch add import of playcount and rating but do not put higher priority on file. to have priority on file just change Banshee.Streaming.StreamTagger.TrackInfoMerge (track, file, false, true); to Banshee.Streaming.StreamTagger.TrackInfoMerge (track, file, true, true); It is just a remark because I prefer your patch to the description of your patch. ^^
(In reply to comment #6) > your patch add import of playcount and rating but do not put higher priority on > file. > to have priority on file just change > Banshee.Streaming.StreamTagger.TrackInfoMerge (track, file, false, true); > to > Banshee.Streaming.StreamTagger.TrackInfoMerge (track, file, true, true); > > It is just a remark because I prefer your patch to the description of your > patch. > ^^ Olivier, are you sure about that? The third argument is called "preferTrackInfo", if you turn it to true then you're actually giving priority on the track and not on the file. Now, regarding the patch: seems fine, except for one use case. What if the user doesn't have the options "Write {ratings|playcounts} to files" enabled? That may mean that the user prefers to use Banshee DB as the holder of his preferences, so maybe it's not good that we overwrite his preferences, even if they come from his files (because he may have some program that modifies them without noticing).
Comment on attachment 197307 [details] [review] banshee now defaults to the metadata in the file, vs the database So, given my last comment I'll mark this as reviewed for now. Kevin, sorry it took so long to review: now can you tell me your thoughts about this? If you agree with me, I'll mark it as needs-work (because I assume you will work on using the flag only when these options are enabled).
The last parameter is only asking if we also want to check the ratings and playcount, the third parameter is the flag on which to choose. All my original patch did was set it so we call the Choose function on these two fields.
Thanks for the patch Kevin ! I've just committed a more general solution for this, looking over other uses of TrackInfoMerge : http://git.gnome.org/browse/banshee/commit/?id=0b7630c Be aware that, in order for changes by an external tool to the rating (or play count) to be detected by Banshee : 1/ That tool has to update the file modification time 2/ The "Write ratings to files" (or "Write play counts to file") has to be enabled. This is to avoid any data loss, as Andrés mentioned.
(In reply to comment #10) > 2/ The "Write ratings to files" (or "Write play counts to file") has to be > enabled. This is to avoid any data loss, as Andrés mentioned. Nice one! Thanks for taking that in account. Now, I guess that after this fix has been committed, the wording of the preference "Write {ratings|play counts} to file" is not really accurate, because it also means Reading too now. However, changing it to "Read and write" instead of "Write" would be misleading too because it's always reading (importing) playcounts/ratings even if the setting is disabled. Oh well, I guess I should open a new bug about this :)
Created attachment 203302 [details] [review] Correct the wording of settings after fix (In reply to comment #11) > Oh well, I guess I should open a new bug about this :) After some debate in the IRC channel about the inaccuracy of the "Write * metadata" setting names and the lengh/complexity of the improvements ("Write and update ratings to/from files"), I think I've found the proper wording (easy to understand, and not too wordy, and accurate enough). Posting here instead of opening a new bug, for simplicity. Please let me know what you think! Thanks
I think we should use another word then Metadata. for Rating and PlayCount maybe use the word "Information" and for normal Metadata, maybe "Track and Video Data"?
(In reply to comment #13) > I think we should use another word then Metadata. for Rating and PlayCount > maybe use the word "Information" and for normal Metadata, maybe "Track and > Video Data"? That is a different topic that is covered in bug 658059. Let's not mix things :)
Created attachment 203343 [details] [review] Correct the wording, v2 Improved, after DNielsen's feedback.
Created attachment 203760 [details] [review] Correct the wording, v3 I realised that the SchemaEntry (besides SchemaPreference) descriptions needed changing too, and their corresponding fields accordingly. One thing this patch could be missing is migrating the configuration key of the SchemaEntry instances too. However I looked into this and is kind of difficult because: 1) gconf-sharp doesn't [1] bind gconf_client_unset [2] (yet?). 2) The XML config backend should also implement an "unset" method accordingly. 3) To migrate the settings only once rather than on each get operation or each time Banshee launches, we would need a similar approach as the one we have with DB Schema changes: have a CURRENT_VERSION field and yada yada. Doing (1) is kind of pointless now because in theory we're going to migrate to gsettings soon (and I'm still not familiar with the migration path from gconf to that). About (2) I haven't investigated yet if it's hard to implement. And (3): at some point we will need it (especially if in GSettings all the tree is conserved in the same way because at some point we would want to migrate from the "banshee-1" name to "banshee" that we still have for the configuration), however it's such a big task just for the sake of this patch. So I've decided to mark the "write_*" key names with TODOs for now.
Forgot: [1] http://developer.gnome.org/gconf/stable/gconf-gconf-client.html#gconf-client-unset [2] https://github.com/mono/gnome-sharp/blob/master/gconf/GConf/Client.cs
Created attachment 203761 [details] [review] Correct the wording, v3 Oops, I attached the wrong v3 patch, this is the one.
Please open a new bug for the wording change, patches attached to closed bugs are not visible anywhere in Bugzilla, so it might get forgotten. (It took me 5 minutes to find this bug again...) Please also limit the change to the wording of user-visible strings. The main point of those preferences is still about writing, their influence during rescan is really a side effect. So I think the names in the code are still OK. In particular, I don't think were going to migrate user preferences for this. Sorry, I should have commented earlier about this.
Comment on attachment 203761 [details] [review] Correct the wording, v3 After further discussion, I've committed the wording change, but only for the user-visible strings: http://git.gnome.org/browse/banshee/commit/?id=8f75d1624