Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MythMusic: "Play Now" any tracks from the music browsers #774

Merged
merged 13 commits into from
Sep 26, 2023

Conversation

twitham1
Copy link
Contributor

@twitham1 twitham1 commented Jul 18, 2023

ref: https://forum.mythtv.org/viewtopic.php?p=25164#p25164

Spontaneous listeners want to just play an album or song now and care less about any playlist. This adds a simple "Play Now" to the menus which is identical to "Add Tracks" except that it immediately starts playing the additions. When "Play Now" is preferred (a new toggle), selecting a single track adds it and plays it now, rather than just adding it.

This required moving previously added tracks to the end so that adding an album gets all tracks even if you played some of them earlier.

Possible documentation is now at the top of https://www.mythtv.org/wiki/Talk:MythMusic to be cut-n-pasted to the manual when this gets to production.

Checklist

twitham1 added 6 commits July 17, 2023 23:45
ref: https://forum.mythtv.org/viewtopic.php?p=25164#p25164

Spontaneous listeners want to just play an album or song now and care
less about any playlist.  This adds a simple "Play Now" to the menus
which is identical to "Add Tracks" except that it immediately starts
playing the additions.

This required changing the working list to allow duplicates as you
need the whole album now even if you played just part of it yesterday.
Of course removing a track removes all occurances of it.
@Jpilk
Copy link

Jpilk commented Jul 31, 2023

With these patches track selection is more intuitive and I'm less likely to want to use an external music player. That's good, thanks. But within a track I still get the old scrambled-buffer problems with FFWD or Rewind (PgDwn/PgUp).

Frontend log reports things like

'I avfdecoder.o: seek time 3420' or
' E AOBase: Buffer is full, AddData returning false',

and the visualiser pauses while playback continues until the corrupt region is past. Do you see this too?

@twitham1
Copy link
Contributor Author

twitham1 commented Aug 1, 2023

Thanks for confirming this works for you! Easier to use is exactly the intent.

The seek issue is unrelated. If you'd like it fixed we should report it separately under Issues (#785) to not sidetrack this feature. I don't get the error. For me each key press seeks immediately and causes about 1 second of playback at that position. So if I press REW or FFWD several times rapidly then it takes a while to catch up then works normally.

@paul-h
Copy link
Contributor

paul-h commented Aug 1, 2023

With these patches track selection is more intuitive and I'm less likely to want to use an external music player. That's good, thanks.

Can you explain how it's better than just using 'Replace Tracks' ?

@Jpilk
Copy link

Jpilk commented Aug 3, 2023

Paul, I don't think I can answer that. I believe we probably want different things. Most of my audio files started as DVB recordings, often lasting several hours and unknown to external databases. I want 'point and go' with clean skipping, and mostly used smplayer. But some of the visualisers are intriguing and I have tried using MythMusic again. With these 'PlayNow' patches, file selection seems to me to do what I want better than before. I hope they will be adopted and won't affect seasoned users.

@paul-h
Copy link
Contributor

paul-h commented Aug 3, 2023

It wasn't a loaded question. Since I can't test the patch at the moment I was having a hard time to understand how it differs from using 'Replace Track'.

Tim has since sent an explanation how his family uses it which helps to explain the differences.

The potential problems as I understand it at the moment are

  • The active play list will just keep growing forever if you only use 'Play Now'. Whether this is a big problem in practice I'm not sure. You can always periodically clear the active playlist but it's definitely not ideal.
  • The patch as is allows the active playlist to have duplicate tracks. This is the deal breaker for me since everything assumes the active playlist does not have duplicate tracks in it. For example shuffle mode wont work as expected and resume playback wont always work as expected. If the user saves the active playlist then the saved playlist will have duplicate tracks etc
  • The patch forces Tims view on how some things should work like making Albums the default item in the tree rather than All Tracks which is the default I prefer since most of the time that's what I want.
  • There are some inconsistencies for example if you have random shuffle mode selected then I think most users would expect that is what you get but if you use 'Play Now' it forces shuffle mode to 'None'. Is that really how it should work?

So that adding an album today doesn't miss tracks added yesterday.
@twitham1
Copy link
Contributor Author

  • The active play list will just keep growing forever if you only use 'Play Now'. Whether this is a big problem in practice I'm not sure. You can always periodically clear the active playlist but it's definitely not ideal.

This is a feature! It's an automated play history, similar to what we get on radio stream. My family can easily scroll back in time and re-hear some sound again in the same order. But users should learn to clear it when they no longer care about their listening history.

  • The patch as is allows the active playlist to have duplicate tracks. This is the deal breaker for me since everything assumes the active playlist does not have duplicate tracks in it. For example shuffle mode wont work as expected and resume playback wont always work as expected. If the user saves the active playlist then the saved playlist will have duplicate tracks etc

I fixed it so duplicates are again not allowed. The only difference is that re-add of existing track succeeds by removing it from previous location. This way you can add a track to the end even if it was already in the list elsewhere. Adding a full album now always succeeds even if some tracks were in the list previously.

  • The patch forces Tims view on how some things should work like making Albums the default item in the tree rather than All Tracks which is the default I prefer since most of the time that's what I want.

I normally browse by Artist. So my preference is the same distance as yours: 1 keypress. You will click left (gallery) or up (tree) while I click right or down before hitting SELECT. I chose Album in between to be default as in Gallery view one SELECT will immediately show all album covers. For those that like pictures, this might be their preferred view.

  • There are some inconsistencies for example if you have random shuffle mode selected then I think most users would expect that is what you get but if you use 'Play Now' it forces shuffle mode to 'None'. Is that really how it should work?

If we want to add to random shuffle, we should Add Tracks as before which won't alter shuffle. Play Now implies we want to play this selection now, so I think canceling the shuffle makes sense. Getting random songs from yesterday would be unexpected for Play Now of one album.

@twitham1 twitham1 marked this pull request as ready for review August 14, 2023 07:16
@paul-h
Copy link
Contributor

paul-h commented Aug 19, 2023

  • The patch forces Tims view on how some things should work like making Albums the default item in the tree rather than All Tracks which is the default I prefer since most of the time that's what I want.

I normally browse by Artist. So my preference is the same distance as yours: 1 keypress. You will click left (gallery) or up (tree) while I click right or down before hitting SELECT. I chose Album in between to be default as in Gallery view one SELECT will immediately show all album covers. For those that like pictures, this might be their preferred view.

Not sure what you mean. Maybe a compromise would be to have an hidden setting and if present use that as the default otherwise keep the default as is?

  • There are some inconsistencies for example if you have random shuffle mode selected then I think most users would expect that is what you get but if you use 'Play Now' it forces shuffle mode to 'None'. Is that really how it should work?

If we want to add to random shuffle, we should Add Tracks as before which won't alter shuffle. Play Now implies we want to play this selection now, so I think canceling the shuffle makes sense. Getting random songs from yesterday would be unexpected for Play Now of one album.

I disagree I hate inconsistencies like that. There is nothing worse than software that knows better than me and overrides what I told it to do :( Lets be honest if uses are too thick to understand that add tracks does what they want most of the time then they probably wont be able to work out how to switch the shuffle mode anyway :)

@twitham1
Copy link
Contributor Author

twitham1 commented Aug 20, 2023

Not sure what you mean. Maybe a compromise would be to have an hidden setting and if present use that as the default otherwise keep the default as is?

Entering Tree View in v33 looks like this (Mythbuntu):
Screenshot from 2023-08-19 22-23-01
My proposed entry to Tree View in v34 looks like this:
Screenshot from 2023-08-19 22-24-05

So you hit Up Right while I hit Down Right. But people that talk like this: "The default of "all tracks" is an extremely weird choice" should be happier to start at Albums and move Right or Down. In Gallery View, an Enter at this pont immediately shows all CD covers, which should be convenient for many.

I don't understand how you prefer browse by all tracks rather than by album or artist. Maybe you list search from there?

I have tracks of the same name listed 2 or 3 times and there is no context in "All Tracks" to know which artist performed the identical title. And I don't enjoy paging through my 5000 tracks as one long list. For searching the whole list I move over to Search for Music. So I suspect most people tend to browse by Artist, Album or both, or some even by directory for files with little to no metadata.

If we instead add a configuration option to change the default entry point from first position of the list to second, I think almost no one would ever find this. I think the change is so minor it should just happen. It costs only one Up keypress to still get quickly to All Tracks.

I disagree I hate inconsistencies like that. There is nothing worse than software that knows better than me and overrides what I told it to do :( Lets be honest if uses are too thick to understand that add tracks does what they want most of the time then they probably wont be able to work out how to switch the shuffle mode anyway :)

In v33 Add Tracks won't touch shuffle. In v34 Add Tracks won't touch shuffle. So no change to existing feature. What I am trying to do is add a new feature.

Play Now never existed, so why should anyone have any expectations whatsoever? The manual clarifies how these features differ so those differences should be their expectation. But, of course, no one RTFM so if they are surprised, I think that is their problem. :) But since they never use shuffle, they never notice this detail anyways. For those that do shuffle, they should be able to figure out how to Add Tracks rather than Play Now or Play Now and then reenable their shuffle.

@paul-h
Copy link
Contributor

paul-h commented Aug 20, 2023

Clearly while I'm willing to work with you to find solutions that keep both parties happy you are a you do it my way or not at all kind of guy :(

@twitham1
Copy link
Contributor Author

Paul, I'm sorry I sounded that way. I was only hoping the pictures would help show the minor change since it sounds like you cannot see MythMusic anymore.

I am only trying to do the right thing. Not for me (I already loved MythMusic as-is) but for all users of MythMusic. I already reverted to no duplicates because you were right: it works better that way.

On the browser entry point, I noticed user feedback against All Tracks and for Albums or Artists. Personally, I never minded pushing down arrow a couple times to get to my favorite node. But this seemed like a simple tweak to make more users happy. I have no experience with adding configuration options but I could look into that. Then I could go ahead and configure to Artist while you configure to All Tracks. But how do we explain to users to go configure this?

On the Play Now, I only have 1 user (outside my family of 6) that used it, and he said above:

With these patches track selection is more intuitive and I'm less likely to want to use an external music player. That's good, thanks

On the forum dlasher described wanting this, and warpme defined the "spontaneous users" that need a "browse and play". That was my motivation to try the new feature, and I was surprised it was not so difficult. And Playlist Editor and Add Tracks are there unchanged so the new feature can be ignored for those that don't need it.

I think more folks should just try it and let us know what they think.

@paul-h
Copy link
Contributor

paul-h commented Aug 22, 2023

On the browser entry point, I noticed user feedback against All Tracks and for Albums or Artists. Personally, I never minded pushing down arrow a couple times to get to my favorite node. But this seemed like a simple tweak to make more users happy. I have no experience with adding configuration options but I could look into that. Then I could go ahead and configure to Artist while you configure to All Tracks. But how do we explain to users to go configure this?

A quick fix would be to look for a setting in the settings table and if present use that as the default route.

Something like

QString routeSetting = gCoreContext->GetSetting("MusicEditorDefaultRoute", "");
if (!routeSetting.isEmpty)
{
    QStringList route = routeSsetting.split("."));
    restoreTreePosition(route);
}    

It would be a hidden setting so if you want anything other than the default you would have to poke the correct value into the settings table.

If you think other would benefit from being able to change the default and want to expose the setting in the UI then that would require a little more work. I would look for an existing setting and copy that. You want one that shows a list of options. You can have a different display values and data values so the display values would be something like ", Album, Artist, Genre ..." and the data values would be the routes to the correct nodes in the tree. The data value is what gets stored in the setting.

@twitham1
Copy link
Contributor Author

twitham1 commented Sep 4, 2023

A quick fix would be to look for a setting in the settings table and if present use that as the default route.

Ok, I understand how that would work. But in investigating this, I discovered the tree position must already be saved when switching from tree to gallery view. So I started thinking: why start all over at the root every time? Why not just enter where we left last time? This does that:

--- a/mythplugins/mythmusic/mythmusic/playlisteditorview.cpp
+++ b/mythplugins/mythmusic/mythmusic/playlisteditorview.cpp
@@ -154,7 +154,7 @@ bool PlaylistEditorView::Create(void)
 
     m_playlistTree->AssignTree(m_rootNode);
 
-    if (m_restorePosition)
+    if (true) // v34 - continue where we left off (was m_restorePosition)
     {
         QStringList route = gCoreContext->GetSetting("MusicTreeLastActive", "").split("\n");
         restoreTreePosition(route);

After trying this I like it better than entering at a root node.

Gallery users often exit by escaping up to the root so their root entry point is automatically saved for next time as we wanted. Tree users can ESCAPE from anywhere in the tree so they enter where they were last located. This turns out to be rather convenient: if I added an album of an artist, upon return I am right there ready to add another album. If I want to start all over with other music it is trivial to move LEFT a few steps to my favorite root entry point.

With this, it seems the entry option is not even needed. Thoughts?

as the 2 features are nearly identical.  A new menu option toggles
"Prefer Play Now" or "Prefer Add Tracks" which orders them on the menu
and defines the action for selecting a single track.
@twitham1
Copy link
Contributor Author

twitham1 commented Sep 8, 2023

I originally wrote Play Now as a separate entry point since the boolean checks on a single track can have only 1 function: add track or play now. But I think users would find this confusing since these options are otherwise identical. I was able to eliminate Play Now from the main menu by adding a preference toggle to the Playlist Editor menu.

upon escape back to the playlist.  I don't know what these disconnects
were for, but removing them restores updates to the parent window.
@warpme
Copy link
Contributor

warpme commented Sep 12, 2023

Tim,

Just want to say this is very nice enhancement and I love idea how you approached this.
Work very well for my MiniMyth2 users :-)

@twitham1
Copy link
Contributor Author

Tim,

Just want to say this is very nice enhancement and I love idea how you approached this. Work very well for my MiniMyth2 users :-)

Thank You! I'm happy to hear this works well for others.

To summarize:

  • Play now simplified to only 2 new menu options: Play Now, Prefer (Play Now|Add Tracks)
  • Music browser entry point is where you exited last time
  • No configuration is needed since the track add preference and browser position are stored in the database

The final idea I have on this is to rename "Playlist Editor" to "Browse Music Library" on the menu. Reasoning:

  • the playlist itself can already remove and move tracks, a big part of editing
  • the playlist editor is for only adding (or replacing)
  • but it seems primarily about browsing the music collection in many useful ways
  • brand new users might not realize their library is in an editor
  • after browsing their library, the menu makes it clear how to add tracks from there

Internally the code can still call this view the playlist editor. We can update the manual in a few places and note that they are synonyms or something like "Browse Music Library" was the "Playlist Editor" prior to MythTV 34.

"Playlist Editor" is renamed to "Browse Music Library" on the
menu. Reasoning:

* the playlist itself can already remove and move tracks, part of editing
* the playlist editor is for only adding (or replacing)
* but it is more about browsing the music collection in useful ways
* new users might not realize their library is in an "editor"
* after browsing, the menu makes it clear how to add tracks from there:

The empty list replaces "You haven't selected any tracks to play" with
"Press M (Menu) to add and play tracks" giving a clue how to start.

Internally the code still calls this view the playlist editor - only
the menu item is changed.  We can update the manual in a few places
and note that these are synonyms or something like "Browse Music
Library" was the "Playlist Editor" prior to MythTV 34.
@linuxdude42 linuxdude42 merged commit 9ee5817 into MythTV:master Sep 26, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants