Bug 11447 - Clearing the current playlist does not clear the current index of the playlist
: Clearing the current playlist does not clear the current index of the playlist
Status: CLOSED FIXED
Product: Logitech Media Server
Classification: Unclassified
Component: CLI
: 7.3.2
: All All
: P3 normal (vote)
: 7.4.0
Assigned To: Alan Young
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-03-23 22:08 UTC by Barry Gordon
Modified: 2009-10-05 14:27 UTC (History)
1 user (show)

See Also:
Category: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Barry Gordon 2009-03-23 22:08:51 UTC
Clearing the current playlist does not clear the current index into the playlist. When a new playlist is loaded it then begins at the index number of the last song played in the prior playlist.

Using the CLI interface:

SEND: 00:04:20:16:01:e0 playlist clear
RCVD: <ID>  playlist clear
Correct response to clearing the playlist
USOL: <ID>  menustatus ARRAY(0x8880fc8) add 00%3A04%3A20%3A16%3A01%3Ae0
USOL: <ID>  prefset server currentSong
The above shows correctly the current song is null since the playlist is cleared

SEND: 00:04:20:16:01:e0 playlist index ?
RCVD: <ID>  playlist index 8
the above is incorrect as the current playlist is cleared

SEND: 00:04:20:16:01:e0 playlist index 0
RCVD: <ID>  playlist index 0
The above is the correct response to a forced setting of the playlist index

SEND: 00:04:20:16:01:e0 playlist index ?
RCVD: <ID>  playlist index 8
The above is incorrect for 2 reaons, (1) the currentplaylist is clear and (2) I just sent a command to set the current playlist index to 0

I tried the same thing in the GUI using the standard web interface to the Squeezecenter.  I loaded an album into a clear playlist and then started the 4th song in the current playlist to play.  I then cleared the current playlist and added a new album to the current playlist.  It showed that the current song was the fourth one and started playing the fourth song in the album I just added

I feel this is a major error and needs to be quickly fixed.
Comment 1 Barry Gordon 2009-03-24 09:06:19 UTC
The same behavior is observed with the latest nightly from 7.3.3.  It is easy to see. Add an album to a player whose current play list is cleared.  Play something other than the first song and let it start so the current song index is other than 1.  Clear the playlist.  Add a new album to the empty current playlist.  observe what the current song index is.  it is not 1. it is what ever was played in the prior step.  Start it playing. It plays the song at the incorrect current index
Comment 2 James Richardson 2009-03-27 11:06:31 UTC
Confirmed this happens if you click on the Add to Playlist button.  if you click the Play button then the index is cleared properly.
Comment 3 Barry Gordon 2009-03-27 11:15:59 UTC
Surprised no one reported this before.  I believe it may have been like this since 7.2.

QA needs a good regression test suite!
Comment 4 Michael Herger 2009-03-28 00:43:11 UTC
Alan - might this be an issue in the new (as of 7.3) state machine?
Comment 5 Alan Young 2009-03-28 08:59:45 UTC
Not exactly, but due to the insidious relationship between a Playlist, and the songqueue maintained in StreamingController. 

I agree that it is annoying but, just at the moment, I cannot see a reliable fix.
Comment 6 Barry Gordon 2009-03-28 10:14:22 UTC
It is more than annoying. It is wrong!!! I have never heard a bug fix answer like that one before.

Is there some other variable that can be set to force the incorrect playlist index back to 0 when the playlist is cleared.  Note that as documented in my test using "<ID> playlist index 0", while documented as a proper CLI command does not work either!!! However on playlist index you may be "safe" in that it is always discussed in the context of the current song playing and when the playlist is cleared there is no current song playing.

My primary issue is that the CLI command "<ID> playlist clear", should ALWAYS set the system such that the playlist index is 0. Not to do that is apriori wrong.

May I suggest that Alan look harder?
Comment 7 Blackketter Dean 2009-03-31 07:49:59 UTC
Alan: Would it be possible to set the index to zero before clearing the playlist?
Comment 8 Alan Young 2009-03-31 11:29:58 UTC
We don't actually hold the 'index' anywhere. What we hold is a songqueue of 0..2 elements. On the songqueue are Song objects, including possibly the song streaming, and possibly the song playing, or the song that has finished or failed. Song objects hold an integer reference to their position in the Playlist. You cannot just clear the songqueue when you clear the playlist because a song may be playing, and the controller needs to be able to find it in its songqueue when it stops or gets stopped, so that onStop actions can be executed. 

When the controller is asked to play the next song, it uses the last Song in the songqueue to find the index of the current song, from which to advance.

It's a horrible mess.
Comment 9 Barry Gordon 2009-03-31 11:49:36 UTC
From your description I concur.  However, why can't the song queue be cleared when the current Playlist is cleared.  If I clear the current playlist any music playing stops.  Ergo what is the song queue needed for?

Okay there is onStop action, but clearing the song queue should force an onStop action if a song was playing. If no song was playing wasn't there a proper onStop action somewhere in the past?

Logically the "long way around" for a user would be to stop a song then clear the play list.  That would resolve the onstop end action issue?  

So why can't a clear of the playlist do all those things instead of the user doing the multiple actions?

If it is too much of a mess, can I, over the CLI interface, if a song is playing  (1) stop the current song (2) set the playlist index to 0; (remember that did not seem to work when I tried it) then clear the playlist.

When I was managing multimillion $$ software projects and someone told me it is a "Mess" my response was not "Who made the Mess?", but rather "Clean up the Mess!"

Messes that are not cleaned up will come around later to bite whomever is still there.
Comment 10 Alan Young 2009-05-18 23:15:39 UTC
Fixed in change 26678.

Note that the 'playlist index' command will only have effect if the current playlist is not empty.

Note that loading a saved playlist will also restore the current playing track from the time that playlist was last played.
Comment 11 Barry Gordon 2009-05-19 07:18:51 UTC
Sounds OK to me.  I will test it out as soon as I find the time.  Thanks
Comment 12 James Richardson 2009-10-05 14:27:56 UTC
This bug has been marked as fixed in the 7.4.0 release version of SqueezeBox Server!
    * SqueezeCenter: 28672
    * Squeezebox 2 and 3: 130
    * Transporter: 80
    * Receiver: 65
    * Boom: 50
    * Controller: 7790
    * Radio: 7790  

Please see the Release Notes for all the details: http://wiki.slimdevices.com/index.php/Release_Notes

If you haven't already, please download and install the new version from http://www.logitechsqueezebox.com/support/download-squeezebox-server.html

If you are still experiencing this problem, feel free to reopen the bug with your new comments and we'll have another look.