Albums with same title are combined into one

[jrbastien]

I have discovered a small issue with the coverart browser. I have 2 different artists with an album titled "Greatest Hits". The coverart browser only shows one of the two cover.

If I expand the track and coverart pane, it displays the tracks for the 2 albums under this single coverart.

[asermax]

We discussed this issue with foss when we first started developing the plugin. We had problems conciliating a solution since we weren't fully aware of how the whole albums structure worked on rhythmbox; our logic was that, since some albums doesn't have an album artist, it would be really difficult to separate albums with the same name with different artist from those individual albums with multiple artists working on it. I think we came to understand this structure better now, and we have, in some way, overcomed that problem with the albums with no album artist by infering it from it's tracks; so we may be able to refactor the plugin to include the posibility to differentiate between these two cases.

An idea (that I might not be able to implement until I get some free time x_x): We could transform each value of the albums dict into a list of albums (if necesary, I mean, if there's really more than one album with that name, otherwise there's no need for that). Some problems that will arise:

• This change would implicate some more changes on the code to manage this new structure.
• Also, it would cause problems when a entry gets updated or added. We should take extra care to correctly identify to which album it belongs: since we might have infered both album artists, finding out which album the entry belongs to might be a little difficult.
• If two albums have the same name and multiple artists, it would be pretty much imposible to differentiate between them.

Those are my thougths d:

[jrbastien]

@asermax, the problem here is not that the albums do not have an artist. Each album have a single distinct artist (e.g. don't question my taste here :) Rick Springfield - Greatest Hits and Eurythmics - Greatest Htis).

[asermax]

Well, yours is the easy use case. The problem is that it is not always that case, and we have to take into account all the other posibilities that may be out there. That said, it might seem like a trivial problem, but it implicates various changes in the internal strcuture of the plugin, so we have to approach it carefully.

[asermax]

@jrbastien @fossfreedom I modified the model structure to support the album artists as an identity parameter for the albums. You can check the results of this on the album-artists branch. Please test this thoroughly, since changes like this are bounded to introduce bugs somewhere (since the api of the album and the model changed a little). I already caught some of them, but there probably are some more lurking around d:

@jrbastien let me know if this is what you were aiming too with your request. Luckily, with the new structure it was way easier to implement than it would've been before the refactoring, but I'm not sure that it covers your expectations.

[fossfreedom]

For albums where each track is a different artist (a variety album) - our view now has an album icon for each track.

On the flip-side - where two albums with the same name but with different artists (all tracks on an album is from one artist) - then this is now correctly separated.

[asermax]

Well yes, that was the principal concern when fixing this bug, albums with various artists and without a defined album artist would get separated; but also, if the plugin asumes that all tracks with the same album name but no album artist belong to the same album, it might be grouping together tracks that have nothing to do with each other.

So in the end, I think it should be responsability of the user to correctly tag their music collection (changing the album artist to "Various Artist" for those tracks that belong to a compillation album).

[fossfreedom]

hmmm - I see where you are coming from.

in my case, the albums with various artists do not have anything in the "Album Artist" field what-so-ever.

It seems all my variety albums (I've got quite a few) never have an album-artist field filled. I presume ripping software makes this non-filling of this field the default for variety albums.

[asermax]

Should grouping those tracks with no album artist together be the default behavior then? Both strategies seems to have their flaws, and in the end the only "true" solution is correctly tagging the tracks, so I don't really know if it makes a difference xD

[fossfreedom]

just to say - with our plugin its very easy to retag these :) - just search by album name - select all - right click and select properties.

I think you are correct - re-tagging is the way to go here. We are diverging from default rhythmbox behaviour though because on the music library view it is this "combined" incorrect behaviour.

This might be worth posting something on the rhythmbox mailing list - I wonder if Jonathan Matthew (rhythmbox maintainer) has any views on this.

I'm going to retag and going to test what happens when searching for covers.

[fossfreedom]

ok - something strange with searching for covers on these now separated icons - I've only switched on the default rhythmbox cover art search (not our own provider).

right click and search for covers on each individual album - it appears that no art is retrieved. Restarting rhythmbox though, the correct album art for each album is shown.

[asermax]

It probably haves to do with this piece of code on the artsearch plugin:

...
self.current_key = RB.ExtDBKey.create_storage("album", album)
    if artist is not None:
        self.current_key.add_field("artist", artist)
...

As you can see, it's adding the field artist instead of album_artist. That may be causing the problem. I'm going to see if I can do something about it.

[asermax]

Fixed, I think. Something related: the cover search status bar still doesn't show, even when our providers are deactivated, do you have the same problem?

[fossfreedom]

yes - fixed.

Strange - the cover search status bar + spinny thing appears correctly

When both providers are switched off - the status bar appears for the 30 secs we implemented before hiding cancelling the search.

The only time it doesnt appear is if you attempt to search for the covers twice on the same album. You have to restart rhythmbox if you want to research for covers on the said album

[asermax]

The statusbar seems to work correctly if I deactivate the coverart browser provider, by commenting the code where it connects to the ext-db request signal:

#self.req_id = self.art_store.connect("request", self.album_art_requested)

My guess is that since the plugin keeps being connected, it receives the signal, and since it doesn't have providers (they're both deactivated) it returns inmediatly indicating that it didn't found anything, misleading the request proccess. Maybe it's better that we bypass the request signal completelly and we create an extensible architecture to be able to include providers directly into our plugin, which would be called before asking the artsearch plugin. That way both can coexist. I'll look into how this could be implemented later.

Anyway, I went really offtopic. Coming back to this issue, I'll merge the changes to master, since it seems to be working as expected and we agreed that the current strategy is better. Gonna close this issue as well, but if any problem arises, feel free to reopen it.

[jrbastien]

Thank you for fixing this.

[fossfreedom]

@asermax - sort by album artist is broken

Traceback (most recent call last):
  File "/home/dad/.local/share/rhythmbox/plugins/coverart_browser/coverart_widgets.py", line 384, in _sort_changed
    self.callback(sort)
  File "/home/dad/.local/share/rhythmbox/plugins/coverart_browser/coverart_browser_source.py", line 1097, in sorting_criteria_changed
    self.album_manager.model.sort(self.sort_prop, self.sort_order)
  File "/home/dad/.local/share/rhythmbox/plugins/coverart_browser/coverart_album.py", line 755, in sort
    self._albums.key = lambda album: getattr(album, key)
  File "/home/dad/.local/share/rhythmbox/plugins/coverart_browser/coverart_utils.py", line 106, in _setkey
    self.__init__(self._items, key=key)
  File "/home/dad/.local/share/rhythmbox/plugins/coverart_browser/coverart_utils.py", line 96, in __init__
    decorated = sorted((key(item), item) for item in iterable)
  File "/home/dad/.local/share/rhythmbox/plugins/coverart_browser/coverart_utils.py", line 96, in 
    decorated = sorted((key(item), item) for item in iterable)
  File "/home/dad/.local/share/rhythmbox/plugins/coverart_browser/coverart_album.py", line 755, in 
    self._albums.key = lambda album: getattr(album, key)
AttributeError: 'Album' object has no attribute 'album_artist'

we used to have an album_artist property on the album class - indeed a see a _album_artist in the do_modified method.

Put back on the album class a @property album_artist that returns self.artist ?

[asermax]

I prefer to change the places were it still references to album_artist than mirroring the attribute. I changed it since it was redundant in the first place, so it would be a waste if I just added it again.