Skip to content

Easy way to add whole CD's to the playlist #475

Closed
MPconta opened this Issue Feb 21, 2014 · 44 comments

5 participants

@MPconta
MPconta commented Feb 21, 2014

Well it's possibly a kind of egoistic wish... :-)

When i found the right CD in the library, i have a very small button to get a list of 4 actions. And one of them is "add, replace and play-back".
I don't know if i am the only one, but i take always this button. Would it be possible to make this button better reachable, because it is the most important of them, at least to my opinion?

@hurzl
hurzl commented Feb 21, 2014

I would like a mode where you don't have to bother with the playlist. So you browse the library and when you click a song, you want to play it immediately and the rest of the album. So the playlist is deleted, the album is added and the clicked song played.

Like a cd player where you throw in a cd ...

@abarisain
Owner

That's like how every classic media player works (I like it better that way, but that's not the case for most mpd users) . I'll maybe add a mode like this later.

Btw : in the latest betas the button has a bigger hitbox. It should be easy to hit.

@hurzl
hurzl commented Feb 21, 2014

I can try to do it if more people are interested

@keithsimister

That would be perfect for me as I pretty much only play full albums.

@hurzl
hurzl commented Feb 25, 2014

I called it "cd player mode", please suggest a better name
https://github.com/hurzl/dmix/tree/cdplayer

not ready for PR yet

@abarisain
Owner

I'm not sure why you made this so complicated (basically why you are tweaking BrowseFragment and Albums fragment. I believe that long click does not need to be overriden since "Add replace and play" is the CD Player mode, and the other options are still there if you want to queue albums in cd player mode)

Overriding the SongsFragment (and playlist song list) list item click is basically what's wanted no ? What other tweaks do you have in mind ?

I'd call it "Classic mode" "Standard Android mode" or even "Simple mode". CD player mode doesn't sound that bad to me either !

@kingosticks

"1-Click" mode (a la Amazon).

How are you supposed to browse down into albums with this mode enabled? Wouldn't it be frustrating if you couldn't? You would also need some way of properly explaining the difference between the two modes as it makes navigation a completely different experience.

@abarisain
Owner

You can browse down into albums. Only the song fragment simple click is overriden in my idea of the function.

In @hurzl 's implementation, the long press on albums is also overriden but I really don't know why since the current menu shows the option we want (it's that option that we want to bring to the Song list click)

@hurzl : when we all agree on what it needs to change, I'll take your changes.

@hurzl
hurzl commented Feb 25, 2014

because it's faster than the menu

@kingosticks

Right OK, only the song fragment makes sense, the description "Always keep one Album in the Playlist" threw me.

@hurzl
hurzl commented Feb 25, 2014

Better "Only have one album in the playlist" or sth. like that

I want it also for people who never heard of the playlist concept

@abarisain
Owner

Oh, your code makes sense now.

I saw it more as a "simple mode" :
Simple tap on a song in SongFragment : Add & replace for the whole album.
Long tap on a song in SongFragment : Queue/Play Next

Long tap on an album, menu with 3 items : "Play" (which destroys the queue and replaces it with the album", "Play next" and "Add to queue". Same for artists.

I guess that I'm annoying but I feel like limiting to only one album to the playlist is a little too limited in my opinion. Sure, bringing a menu is slower, but it's one one more tap.

@hurzl
hurzl commented Feb 25, 2014

Ok that would be a compromise between simpleness and playlist usage

@abarisain
Owner

We could force users to make a playlist in simple mode before using the queue, but I think that it's a really frustrating experience.

Anyway, howerver we implement it, I'm in. I think that the current Add/replace complexity is annoying. When users will start using aMPD (or the official MPD server for android) on their phones/tablets, it will be even more annoying. So yeah, a simple (or 1-click) mode is an idea I really like.

@hurzl
hurzl commented Feb 25, 2014

Would make the code less buggy if I restrict the album menu to 3 items instead of using the extra itemlongclicklistener

@abarisain
Owner

I think that the alternative code would be at the menu generation itself so it would be really less buggy. If we agree on the idea, I can implement it myself if you want (and take your SongsFragment changes)

@hurzl
hurzl commented Feb 25, 2014

If you want to do it, why not

Maybe you can also think of a better way than the extra MPD.java method, but the problem is you have to put everything in a single thread

@abarisain
Owner

I implemented it, in a much simpler way. I also got rid of the extra MPD.java method, and it all happens in one thread.

A video is worth a thousand screencaps (since I'm too lazy to do them anyway) : http://c.nlss.fr/31032l0c0C3x

Note: Selecting "Play" in the context menu of SongsFragment will Add replace and play the selected song only. It's a little caveat of the way BrowseFragment and child are implemented, I'll have to make it better somehow ...

Note 2 : we're totally out of topic for this bug, it was CD centric and we are Album centric. Oh well, "cdplayer mode" was requested a lot

@abarisain
Owner

@hurzl : If you don't like how this new feature works (I'm sorry you wasted your time working on it :/), tell me. It's not set in stone :)

@hurzl
hurzl commented Feb 25, 2014

For me it's important to skip to the clicked song

@abarisain
Owner

Well, it does do that (at least since the latest commit I pushed). The commit you commented on was a work in progress and I forgot to push the fix.
The other songs are still in the queue though, the whole album is added bug it skips to the song you clicked.

@hurzl
hurzl commented Feb 25, 2014

Ah ok, you don't get threading problems like this?
With "add" you start a thread in MPD, and immediately seek, I thought this was a problem

(With a big album, like 150 tracks...?)

@abarisain
Owner

If you can try with a big album of yours, this would be nice. I don't have albums like that.

The threading is done by line 369 4a31815#diff-2eb8f3bbddd2058e72b665d5398750ccL369

MPD.java's add by itself does not thread anything, it's all the asyncHelper's job. Sure, add makes a runnable and sends it, but it's only for emulating a lambda : it's executed in the caller thread.

EDIT : I should probably only Add & replace and trigger the play myself rather than Add replace & play and then seek. If the network has issues between the commands, mpd will start the first song and not the wanted one.

@hurzl
hurzl commented Feb 25, 2014

You're right, the add method doesn't return before the runnable is done

And it works with 178 tracks, so looks ok

... why not show the NowPlayingSmallFragment in the library browser, too, maybe in simple mode?

@abarisain
Owner
@hurzl
hurzl commented Feb 25, 2014

Not really needed, but faster ;)

@abarisain
Owner
@hurzl
hurzl commented Feb 25, 2014

An optimization I was thinking about, probably too much hassle, when clicking another song, same album, just skip to the song ....

... another one: when clicking on the SmallFragment, return to NowPlaying

@abarisain
Owner

another one: when clicking on the SmallFragment, return to NowPlaying

Yeah, I was searching what kind of feedback I could provide and that's probably great. Problem is that if the user presses back ... they get kicked out of the app while expecting to land in the library. Maybe we can ignore this issue for now.

An optimization I was thinking about, probably too much hassle, when clicking another song, same album, just skip to the song ....

As you said, probably too much hassle. At least with this method we're sure that the queue is clean.

@abarisain abarisain closed this Feb 28, 2014
@abarisain
Owner

@hurzl can you check out https://github.com/abarisain/dmix/tree/sliding ? I reworked the UI and I wonder if people will like it or not.

Note that the new sliding view does not work in "Go to artist/album" yet

@hurzl
hurzl commented Feb 28, 2014

It's very nice, but ...

  • it should start at NowPlaying, but people won't understand that you can slide it
  • the icon at the top should probably link to the library screens instead of the playlist?
@abarisain
Owner
@kingosticks

I hope this is easy to use 'cause it looks really smart, great work. I'll try it out this weekend.

@abarisain
Owner

The slider hitbox is a little funky, but in the end it will work much like Play Music.

BTW: Thanks to positive feedback, this has been merged into master and published as 1.07 Beta 3

@hurzl
hurzl commented Mar 1, 2014

The settings should be accessible by the menu - or the sideways menu should be available in NowPlaying

I don't find refresh MPD's database!?

@kingosticks

After fighting with Android Studio (as usual) and checking out a fresh workspace it's finally up and running (I appreciate I could have just got Beta 3). I just wanted to say the UI is really nice. You do have to be quite accurate when sliding the "now playing" upwards, on my phone you can easily inadvertently activate that stupid ok google circle thing. But it's cool to use.

In "simple mode" the different behaviour when browsing artists/albums compared to browsing playlists is a bit annoying but I appreciate why it's like that. (It's also a bit messy with mopidy-spotify as adding all songs from the album will add everything from all copies of the album. So you usually end up with many copies of each track. But that's not your issue.)

@hurzl, There's a refresh button on the action bar when you're in the "Files" browser/library. I assume it refreshes... I can't test it myself.

@abarisain
Owner

After fighting with Android Studio (as usual)

I'm really curious of what issues your are having, it works pretty well for me (maybe the new library inclusion was a little confusing for your IDE.). It works much better than Eclipse !

The settings should be accessible by the menu - or the sideways menu should be available in NowPlaying

I'll probably do that, and I'll probably need an icon to trigger the menu just like on the library view.

I do not think that the settings need to be accessed that often though. (Play music blocks the navigation drawer if the sliding menu is expanded) Especially once we'll have the multiple server manager.

I don't find refresh MPD's database!?

I was afraid that this would be an issue. I really want to unclutter the menus, and refresh does not belong in the settings. The refresh button is now in the FS pane, and pressing it from a subfolder triggers an incremental refresh.

The discoverability sucks though, I'm not really satisfied.

You do have to be quite accurate when sliding the "now playing" upwards

You can just tap the song name/cover art. The panel will slide up. I never slide it up manually :)

I think that I should probably make this panel's taller though.

compared to browsing playlists is a bit annoying but I appreciate why it's like that.

That's because simple mode is not finished yet. I'll probably make it just like SongsFragment in the end.

@kingosticks
@hurzl
hurzl commented Mar 1, 2014

That refresh button could be at least on artists and albums, too?

@abarisain
Owner
@hurzl
hurzl commented Mar 1, 2014

That's why it belongs in the settings ... under servers

Seeing the icon you don't really know what it means

@abarisain
Owner
@hurzl
hurzl commented Mar 1, 2014

Menu on the library screens?
On files screen add "refresh this folder" item

@abarisain
Owner

The menus are already crowded, I think that I'll put them back in the settings (again, play music does it that way too) and keep the fs menu.

This is related to #483 by the way

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.