fossfreedom / coverart-browser

Browse your cover-art albums in Rhythmbox v2.96 - 3.0+
http://xpressubuntu.wordpress.com/
GNU General Public License v3.0
74 stars 19 forks source link

Automatically queue next track from track-view #130

Closed londumas closed 11 years ago

londumas commented 11 years ago

Here is a bug I have discovered. When you are in the normal song-browser and you type an artist to only see his songs, you select a song and it plays. When it is the end of the song, Rhythmbox will either go to a random song of this artist or to the next one, depending on the play mode. But when you are doing the same thing with the coverart-browser, no song is played. It is not very important but it is another difference between the normal browser and the coverart-browser.

fossfreedom commented 11 years ago

Hi - yes, I noticed that with RB. Its a feature I don't like - and personally I regard as a bug.

Remember, RB deals mainly with track-based stuff - it doesnt handle albums very well - hence why we have this plugin.

I dont think we should "queue" the next album when the last album has played. I'm not sure how you would determine what "next" is - is it the very next album after currently selected album in the cover view? Doesnt really makes sense to me.

@asermax - any thoughts on this?

londumas commented 11 years ago

What I mean is that if I select an album on the coverart-browser, go to tracks and select one. Play this one. At the end of the track, the player is not going to play an other song, it is going to stop there. Do you understand ?

fossfreedom commented 11 years ago

You are referring to the "music library" tab/source. Yes - we dont interact with this. We add what tracks we need to play to the queue library source. When stuff on the queue library finishes, no other tracks are played.

You are (I'm assuming) talking about the "music library" source - you filter on an artist - select play - when no more songs from the artist is to be played, RB starts playing some other random song.

Thats the feature I personally dont like.

asermax commented 11 years ago

It may make some sense to play the next song within the same album, but I'm not very sure how to do it. I'll take a look at it later; if it doesn't complicates things a lot, I'll let you know and we'll see what we can do :3

londumas commented 11 years ago

Cool, Hope to see it

hdave commented 11 years ago

+1 for this. Would be great to option to play the next track in an album after the one playing is finished.

fossfreedom commented 11 years ago

a couple ways at the moment you could do this - just double click the album itself to play the whole album.

in the track-view - just highlight all the tracks you want to play - right click and choose play or queue.

Yes - agree - we need to consider a simpler way to achieve this.

fossfreedom commented 11 years ago

something to investigate later - maybe a RB.PlayOrder could be useful here

i.e. create an empty querymodel - fill it with entries from the position of play on the entryview to the end. I suppose the theory is that the ShellPlayer could be persuaded to use the PlayOrder rather than the coverart-browser using the Queue Source to play from.

[EDIT] - play_order stuff is a dead-end.

Will investigate whether a hidden playlistsource is a better alternative.

[EDIT] - dunno - found nothing so far.

fossfreedom commented 11 years ago

@asermax - re this commit. I'm out of ideas how to do this without doing stuff with threads etc to queue the next track.

Logically it should be possible to force the ShellPlayer to play from the entryview querymodel - dunno though how to do this. If you have some ideas then let me know and then we can backout this "kludge"

asermax commented 11 years ago

I'm not sure about your change, I think it's a monkey patch for something a lot bigger. You can set the playing of the player through the set_playing_source method, or tell him to play an entry from a given source using play_entry. The thing is, we have a couple of problems that we would need to solve before that two methods can work correctly. We're currently using the query model from the library source, wich leads to various issues:

So, to summarize, I think we should somehow keep and update a RhythmDBQueryModel on our AlbumsModel. This query model should get filtered or sorted whenever the albums model gets filtered or sorted (I'm not sure about how to implemen this tho). Then we should tie the source's query model to the albums model's query model. This is probably the best approach to avoid this problem resurfacing in the future (e.g. if we implement new views, they wouldn't have to care about this stuff since is managed on the model).

Said that, I made a little change on my last commit to show you how it would work. What my 'patch' currently does is:

  1. When an album or group of albums is set to play, instead of queuing the contents, I replace the query model to only contain the contents of those albums.
  2. When a track in the entryview is set to play, I do the same thing, replace the query model to the contents on the entryview.
  3. When there are selected tracks on the entryview, and they are set to play, only use those tracks on the query model.

This is pretty close to what I was looking for, but there's is still one big flaw: now, whenever you double click on the source name on the source's list, it will only play those entries on the source's query model, which would be those that where set there the last time you played an album/entry from the source.

Conclusion: as I said earlier, this should be implemented at the model level. The default behavior should be to keep playing through the whole source (filtered and sorted of course), doesn't matter if you started off by playing an album or an entry (if you want to play only one or a couple of albums, that's what the queue option is for). That way we would have a coherent behavior for the whole source.

fossfreedom commented 11 years ago

ok - sounds bigger than I thought.

The changes have made for a few regressions such as mixing queue songs with browser songs.

May have to back this out anyway and move this to a separate branch to fully develop and test etc.

Will have a look at this later this week (hopefully). absolute rubbish! rhythmbox itself mixes queue songs with songs playing from the library.

fossfreedom commented 11 years ago

@londumas & @hdave

some changes have been made on this issue. I would be grateful if you have the time to checkout the master branch and give this a test.

We need to ensure playing & queuing from both albums and track-view work satisfactorily

TIA

londumas commented 11 years ago

This seems perfect to me, I don't seem to have seen a bug. Thank for your work.

Note: I put it here because it is small, the string "first alternate genre" doesn't seem to be translateble in Launchpad. Am I right? (in Plugin/Tool Bar/)

fossfreedom commented 11 years ago

@londumas - correct - yes the string is untranslatable - it is an example that the user could either modify or delete. It is there to give the user a quick first impression that there is new functionality there and "this is how you do it".