Bug 16890 - RandomPlay can fail to add new songs when multiple songs skipped
: RandomPlay can fail to add new songs when multiple songs skipped
Status: CLOSED FIXED
Product: Logitech Media Server
Classification: Unclassified
Component: Plugin
: 7.6.0
: All All
: -- major with 3 votes (vote)
: 7.6.0
Assigned To: Adrian Smith
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-02-04 00:08 UTC by Brett
Modified: 2011-05-09 09:43 UTC (History)
2 users (show)

See Also:
Category: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Brett 2011-02-04 00:08:19 UTC
According to user Erland since svn revision 29324 SBS 7.6 waits 15 second before generating random tracks in Random Mix plugin after every track change. The timer is reset after every track change.

This has the undesired side effect that users get stuck in a small pool of songs that just repeat over and over without understanding why or how to fix the problem.

Cause:
-A 15 second delay timer was added to reduce on track change CPU load
-By default only 10 songs are generated ahead
-Random songs are only generated if a song is played for atleast 15 seconds
-Users may not like the first 10 songs and end up skipping through them before 15 seconds is played through any file.
-SBS 7.6 will end up just looping through those 10 songs until the user stops skipping and plays 15 seconds of a song.

Solution:
-Reduce delay to 10 seconds and do not reset timer on every track change and increase default songs ahead count to 20. Recommended solution as this means that extra songs are always generated after a maximum of 10 seconds after any track changes. This should prevent fast skipping users from seeing the problem as they should not be skipping through 20 songs in under 10 seconds. Low end machines also get a 10 second grace period after a track change to heap with CPU load issues.
-Increase the default songs ahead count to make it more likely the user will play 15 seconds of any song in the current list generated. Not recommended as track generation count gets too high causing CPU load issues and there is no fixed number of tracks to ensure the user will play atleast 1.
-Decrease the 15 second timer. Not recommended as low end machines experience CPU load issues on track changes.


Issue is at line 1170 in server\Slim\Plugin\RandomPlay\Plugin.pm
I would suggest the below but apparently pendingTimers has been removed or something?

# Delay adding random tracks to playlist for 10 seconds unless already in a pending delay
if(Slim::Utils::Timers::pendingTimers($client, \&_addTracksLater) == 0)
{
Slim::Utils::Timers::setTimer($client, time() + 10, \&_addTracksLater);
}
Comment 1 Jim McAtee 2011-02-04 00:48:46 UTC
Unlike normal playlist behavior, random mix can never be allowed to loop around to the beginning of the current playlist.  If the user skips the track and you're playing the last track of the playlist, then you must add new tracks before going to the next track.
Comment 2 Andy Grundman 2011-04-25 09:27:10 UTC
Triode: Want to look at this?
Comment 3 Adrian Smith 2011-04-26 05:23:33 UTC
Andy - yes will look at it, but I am unsure what the reason for the delay was.  This came from embedded branch.  If its due to SP performance, I think we would be better off doing direct streaming of local files to SP to avoid the server being involved in streaming ( I've always thought this :) )
Comment 4 SVN Bot 2011-04-26 07:11:51 UTC
 == Auto-comment from SVN commit #32351 to the slim repo by adrian ==
 == http://svn.slimdevices.com/slim?view=revision&revision=32351 ==

Bug: 16890
Description: don't defer adding new tracks if we are reaching the end of a random playlist as this results in the playlist repeating
Comment 5 Adrian Smith 2011-04-26 07:13:02 UTC
Please try 7.6 trunk svn 32351 and report if this fixes the issue.
Comment 6 Brett 2011-04-27 08:03:18 UTC
I still believe this causes a UI breakage after testing on the latest nightly.
It doesn't solve the problem that additional tracks are not generated in appropriate amount of time.

If I go to the playlist, the "future" list should be x(default 10) songs deep. With this bug fix it could be as little as 2.

This is a terrible UI breakage for some crazy idea of "we sold some under powered device that cant do what we said it could".

Broken devices should be broken not at the sacrifice of all other devices.

The idea is to have x(default 10) songs in advance in the playlist at all times. This function is vital and should not be totally broken by unworkable devices.

I see the only workable solution is the one I suggested about checking if a delay already existed, all other solutions are a poor hack.

The current playlist can not be as little as 2 songs deep, that has been broken in the 7.6 branch.
Comment 7 Adrian Smith 2011-04-27 10:16:27 UTC
Guess I don't see why having 10 songs at all times is vital to the ui - they will appear within 15 seconds with the latest version.

Can you cause the playlist to loop with the current fix?  This is the problem we are trying to fix?  The number of tracks held in the playlist ahead of time is less important than ensuring we don't loop.
Comment 8 Jim McAtee 2011-04-27 10:34:50 UTC
Yes, it still loops if you hit FFWD enough times and "go off the end" of the playlist.

But I seriously question whether this should be considered a 'major' problem. This has to affect very few users.
Comment 9 SVN Bot 2011-04-27 14:07:10 UTC
 == Auto-comment from SVN commit #32367 to the slim repo by adrian ==
 == http://svn.slimdevices.com/slim?view=revision&revision=32367 ==

Bug: 16890
Description: make bug fix more aggressive - try to fill the playlist earlier
Comment 10 Adrian Smith 2011-04-27 14:09:49 UTC
I've made the fix slightly more aggressive and hope this covers most cases.

Brett - note that the current timer code doesn't have a pendingTimers call so its not possible to test for a timer and only set one if there is not a pending one.

I note that there are other cases that you can cause the random playlist to wrap, such as fwd on the last track of an album in album mode.  This is because album mode only adds tracks when the last track is reached (so this is a feature which existed prior to this timer being added).