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

"random songs" isn't very random at all #510

Closed
chetmurthy opened this issue Jan 14, 2021 · 18 comments
Closed

"random songs" isn't very random at all #510

chetmurthy opened this issue Jan 14, 2021 · 18 comments

Comments

@chetmurthy
Copy link

I don't really know how to debug this issue. I'm an experienced Perl programmer, and generally speaking a systems jock, so with some pointers I'm sure I can get going on debugging, so anybody who can suggest how to start, I'd appreciate.

I have a lot of songs in my music database. And I noticed that somehow "random songs" seems to find the same ones over and over. I mean, in the last hour it's picked the same "Dead Can Dance" song several times, and this isn't the only instance. It happens a lot. And of course, this means that many other parts of my library (which I can search for and find) so they're there for sure) just never get played.

Not sure how to diagnose.

@michaelherger
Copy link
Member

This has been the topic of discussions before. Even I believe something is wrong there. Alas, as much as I tried, I didn't find the culprit.

Repetition within a mix should not be possible, as at the beginning of a mix LMS would get the full list of IDs, shuffle and store it. Then it's popping IDs off of that list as new tracks are requested. Once the list is empty it'll start over. The list creation can be found in the RandomPlay plugin's Mixer.pm:

https://github.com/Logitech/slimserver/blob/c31478a4b816b371f5d3fef1a02267fed38ec548/Slim/Plugin/RandomPlay/Mixer.pm#L76

It uses the Fischer-Yates shuffling, as implemented in https://github.com/Logitech/slimserver/blob/c31478a4b816b371f5d3fef1a02267fed38ec548/Slim/Player/Playlist.pm#L689. Maybe it's worth checking whether that's implemented correctly?

FWIW: I'm no longer using that feature enough to be bothered, tbh. I'm mostly using the Don't Stop The Music feature with LastMix or Spotty.

@rgobbel
Copy link

rgobbel commented Jan 25, 2021

This is a one-line fix. At the beginning of fischer_yates_shuffle, add this line:

srand(time());

I did this in our server a few days ago, and the difference is dramatic and wonderful. I'll put in a pull request shortly.

@michaelherger
Copy link
Member

Hmm... if you read the documentation for stand, they clearly state that

Do not call srand() (i.e., without an argument) more than once per process.

We already do in more than one place... In general people seem to suggest NOT to call it explicitly at all, except for when you actually want to have reproducible randomness (but in that case you'd call it with the same seed). So... not sure.

As I mentioned: this has been topic of discussions before. I believe it's hard to say whether there's a dramatic improvement, as you noticed it. Because in my experience, the randomness felt perfectly fine at times, but sometimes not at all. But the there are some people who claim that this was part of randomness, that you could get result you wouldn't consider random enough...

Could you try to change stand() in slimserver.pl to use the timestamp as the seed and see whether this does help, too?

@rgobbel
Copy link

rgobbel commented Jan 26, 2021

The documentation says not to call srand() without an argument. I didn't. I call srand(time()). There is no doubt in my mind about the dramatic improvement, it's pretty obvious. In any case, I didn't do what they say not to do.

@rgobbel
Copy link

rgobbel commented Jan 27, 2021

After having looked at this issue more closely, I agree that simply adding srand(time()) to the shuffle routine is not a great way to deal with this--in fact, I'm not sure why it works, though as I've mentioned, it does dramatically improve the breadth of the sampling from our library. I suspect that the original problem may be due to the fact that srand was being called in slimserver.pl before daemonize, so that the child process has actually never had a call to srand at all. I moved that call to just after the fork() inside of daemonize, and I'm running with that now. So far it looks like it may have worked--at least we're not seeing the same old stuff, as we had been. I'll withdraw my initial PR, and if this looks good after running for a while, submit a new one (or just go ahead and make the change--I think what's needed is a call to srand() after the fork() in daemonize, as well as the call near the beginning of init.

@michaelherger
Copy link
Member

I consider implementing a shuffling algorithm based on this Spotify article:

https://engineering.atspotify.com/2014/02/28/how-to-shuffle-songs/

Is this mostly a problem with the tracks mix?

@chetmurthy
Copy link
Author

I continue to see this problem with the "Random Mix -> Random songs" mix.

@michaelherger
Copy link
Member

I continue to see this problem with the "Random Mix -> Random songs" mix.

Well, there's been no change so far... I was asking about where the issue was most obvious.

@chetmurthy
Copy link
Author

Sorry, didn't mean to imply that a fix had been attempted. I was just .... being succinct grin. Describing how I elicit the problem, and that it continues to be present.

@terual
Copy link
Member

terual commented May 1, 2021

I thought I would test if the random mix is really random.

  • I created a small library with ~800 songs
  • I changed Slim::Plugin::RandomPlay::Mixer.pm to export the shuffled ids to a file directly after the Slim::Player::Playlist::fischer_yates_shuffle call
  • From the webbrowser I called RandomPlay.mix('track', '0') from the JavaScript console about 17.000 times
  • I counted all the trackID occurences in the first 10 lines in those 17.000 files (because the first 10 songs would be very noticable if they were the same in subsequent calls) and plotted them:

afbeelding

The ID with the least occurrences in those first 10 lines occurred 173 times.
The ID with the most occurrences in those first 10 lines occurred 251 times.

The expected number of occurrences based on the sample would be 208.7 times.

Based on these data I would say that the randomisation works correctly and maybe indeed the issue is what is described in the Spotify article.

@michaelherger
Copy link
Member

I spent time on this rainy day doing some experiments with "balanced" shuffle following the above Spotify article. Please see my findings here: https://forums.slimdevices.com/showthread.php?114355-How-random-is-random&p=1021166&viewfull=1#post1021166

mherger added a commit that referenced this issue May 1, 2021
Purely random shuffling often leads to results which the user considers not random. But what she actually wants is a real non-random shuffle, where an artist's tracks would be evenly distributed in a mix. This new shuffling method is based on https://engineering.atspotify.com/2014/02/28/how-to-shuffle-songs/.
@michaelherger
Copy link
Member

Please give the latest 8.2 nightly a try. Make sure you select the "balanced" shuffle in the Random Play menu. It should be on by default in most cases.

@terual
Copy link
Member

terual commented May 2, 2021

I have redone the randomness test with the new algorithm and with an expected track occurrence of 200 in my sample the standard deviation increased from 14,1 to 16,8 with the balanced algorithm (as expected). So it is indeed a little less random, but it seems that tracks are indeed better distributed in the playlist.

@michaelherger
Copy link
Member

Sorry to hijack this issue

If you're serious about being sorry, then don't do it... Just create a new issue, or head over to the forums to discuss the idea.

I'm glad you're talking about "the feeling of randomness", which actually is anything but random 😁.

@mherger
Copy link
Contributor

mherger commented Jun 29, 2021

I'm closing this issue, as I believe we now have enough options to make randomness not so random.

@mherger mherger closed this as completed Jun 29, 2021
@nguillaumin
Copy link

I'm having a similar issue but with the random album mix. Over the past 4-5 days I had multiple albums repeat, over a collection of ~1200.

Where is "balanced shuffle" setting? I was unable to find it. Will it apply to random album mixes as well?

@michaelherger
Copy link
Member

Settings/Advanced/Performance

@nguillaumin
Copy link

Thanks, not where I was expecting this 😉 In any case it's already in balanced mode. I'll try to collect some actual data and open a new ticket.

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

No branches or pull requests

6 participants