-
-
Notifications
You must be signed in to change notification settings - Fork 76
add option to configure queue size #490
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
Conversation
I am planning on working on an alternate approach to this soon-ish - continuous queuing mode. The idea would be that when playing random songs, it would load 100, but then when it gets to the last song in the queue, load the next 100, and so-on. This would get rid of the slowness of loading 1000s of songs from the server at once, and it would also be a mode you could enable when listening to an album, for example, where once it finishes the album it plays similar songs automatically |
I know popular applications that also do this, and while I think it will be a nice addition, it's not something I personally like. I think the slowness is a different issue. Yesterday I made a flamegraph and it showed that most of the time is wasted doing Unmarshal of XML. For android I use symfonium music player, and it is capable of creating a playlist with 6000 songs in less than a second and recovering it (when restarting the application) in less than five seconds. I will be truly happy if supersonic could get close to that in the future. |
ui/dialogs/settingsdialog.go
Outdated
@@ -296,6 +309,10 @@ func (s *SettingsDialog) createGeneralTab(canSaveQueueToServer bool) *container. | |||
saveQueueHBox, | |||
trackNotif, | |||
albumGridYears, | |||
container.NewHBox( | |||
widget.NewLabel(lang.L("Queue size: ")), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this needs a more descriptive settings name. "Queue size" is kind-of ambiguous, maybe something like "Random queue size"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although now that I remember, that setting also applies for "play similar artists". Should be better to remove it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could remove it from the settings dialog but leave it in the config file. There are already a few settings that are config-file-only (which most people probably won't need to change)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think some users could find it useful to easily change it, but maybe it's just my personal preference. I remove it from the UI.
btw, there is plans to improve the performance regarding the lists?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well aside from switching away from Go's standard XML parser, or reworking the whole subsonic library to use JSON (including the work to handle date idiosyncracies across different servers), there's not much to be done. Switching to JSON would be something I would review if someone else were to do the work but it will be low on my personal priority list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To add onto this, it would not be a very large amount of work to switch to JSON, but the custom marshalXML/unmarshalXML implementations for the xsdDateTime type in here would need to be implemented and tested with the JSON parser as well. All the workarounds for things like timestamps with and without timezone suffixes would have to continue to work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think it would be feasible and easier to use concurrent jobs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Queuing random/similar tracks from the server is a single HTTP call so I don't think it's really parallelizable. On the other hand, loading the saved play queue currently issues one HTTP call per track, to get updated play count and other metadata, so concurrent calls could likely speed that up significantly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Thank you.
btw I changed the config name as requested
e6f2bab
to
a4a7f31
Compare
When playing random songs supersonic had a hardcoded value of 100. This was very small for my taste. This PR add the option to configure it.
I found out, however, that the current algorithm is worryingly slow. With a value of 1000 it takes more than 20 seconds on my computer. Recovering the queue when restarting supersonic is also very slow.