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.

Sign up for GitHub

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

Handle case where user moves/adds/removes tracks in a playlist while the next track is already fully streamed #964

Merged
merged 11 commits into from
Jan 5, 2024
Merged

Handle case where user moves/adds/removes tracks in a playlist while the next track is already fully streamed #964

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
470be59
initial commit
philippe44 Dec 21, 2023
987da5f
handle STMf in stream controller to move to next track
philippe44 Dec 21, 2023
bcbf5a5
simplified version with player::flush setting readytostream state
philippe44 Dec 22, 2023
dd19ec9
shift on scalar is deprecated on recent perl
philippe44 Dec 23, 2023
20929d3
last track modification in case it has been fully streamed should be …
philippe44 Dec 23, 2023
6b1cae8
complete last track add/remove/move issue
philippe44 Dec 24, 2023
3dddf21
in insert_mode, use playinIndex, not streamingIndex!
philippe44 Dec 30, 2023
5be2022
handle multiple remove
philippe44 Dec 31, 2023
54b4910
handle shuffle case in insert
philippe44 Dec 31, 2023
976dcd7
need to plan for end of playlist restart in all cases, at the begining
philippe44 Jan 1, 2024
5974b61
can flush from IDLE (streaming) because streaming track might be comp…
philippe44 Jan 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 57 additions & 25 deletions Slim/Player/Playlist.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,10 @@ sub addTracks {
my $playlist = playList($client);

my $maxPlaylistLength = $prefs->get('maxPlaylistLength');

# do we need to plan for restart of streaming?
my $restart = (Slim::Player::Source::playingSongIndex($client) == Slim::Player::Source::streamingSongIndex($client)) &&
(Slim::Player::Source::playingSongIndex($client) == count($client) - 1);

# How many tracks might we need to remove to make space?
my $need = $maxPlaylistLength ? (scalar @{$playlist} + scalar @{$tracksRef}) - $maxPlaylistLength : 0;
Expand Down Expand Up @@ -245,17 +249,25 @@ sub addTracks {
});
}


if ($insert) {
_insert_done($client, $canAdd);
}

# if we are adding a track while we are playing the last one, might
# need to relaunch the process if it has been streamed fully.
if ($restart) {
$log->info("adding track while playing last, check if streaming needs relaunch");
$client->controller->nextIfStreamed($client);
}

return $canAdd;
}

sub _insert_done {
my ($client, $size, $callbackf, $callbackargs) = @_;

my $playlistIndex = Slim::Player::Source::streamingSongIndex($client)+1;
my $playlistIndex = Slim::Player::Source::playingSongIndex($client) + 1;
my $moveFrom = count($client) - $size;

if (shuffle($client)) {
Expand All @@ -266,6 +278,11 @@ sub _insert_done {
} else {
push @{$client->shufflelist}, @reshuffled;
}
# need to flush if we are changing streaming song
if (Slim::Player::Source::streamingSongIndex($client) == $playlistIndex % count($client)) {
main::INFOLOG && $log->info("add+replace streaming (not playing) track");
Slim::Player::Source::flushStreamingSong($client);
}
} else {
if (count($client) != $size) {
moveSong($client, $moveFrom, $playlistIndex, $size);
Expand Down Expand Up @@ -382,25 +399,25 @@ sub removeTrack {
}

if (!$stopped) {
if (Slim::Player::Source::streamingSongIndex($client) >= $tracknum && Slim::Player::Source::streamingSongIndex($client) < $tracknum + $nTracks) {

my $index = Slim::Player::Source::streamingSongIndex($client);
my $queue = $client->currentsongqueue();

# udpate index of tracks in queue but capture the streamingIndex before...
for my $song (@$queue) {
if ($tracknum < $song->index()) {
$song->index($song->index() - $nTracks);
}
}

if ($index >= $tracknum && $index < $tracknum + $nTracks) {
# If we're removing the streaming song (which is different from
# the playing song), get the client to flush out the current song
# from its audio pipeline.
main::INFOLOG && $log->info("Removing currently streaming track.");

Slim::Player::Source::flushStreamingSong($client);
}

} else {

my $queue = $client->currentsongqueue();

for my $song (@$queue) {

if ($tracknum < $song->index()) {
$song->index($song->index() - $nTracks);
}
}
}
}

if ($stopped) {
Expand Down Expand Up @@ -446,6 +463,7 @@ sub removeMultipleTracks {

my $stopped = 0;
my $oldMode = Slim::Player::Source::playmode($client);
my $flush = 0;

my $playingTrackPos = ${shuffleList($client)}[Slim::Player::Source::playingSongIndex($client)];
my $streamingTrackPos = ${shuffleList($client)}[Slim::Player::Source::streamingSongIndex($client)];
Expand Down Expand Up @@ -473,7 +491,7 @@ sub removeMultipleTracks {

} elsif ($streamingTrackPos == $oldCount) {

Slim::Player::Source::flushStreamingSong($client);
$flush = 1;
}

} else {
Expand Down Expand Up @@ -539,6 +557,8 @@ sub removeMultipleTracks {
for my $song (@{$queue}) {
$song->index($oldToNewShuffled{$song->index()} || 0);
}

Slim::Player::Source::flushStreamingSong($client) if $flush;
}

refreshPlaylist($client);
Expand Down Expand Up @@ -589,18 +609,10 @@ sub moveSong {

splice @{$listref},$dest, 0, @item;

my $queue = $client->currentsongqueue();

my $playingIndex = Slim::Player::Source::playingSongIndex($client);
my $streamingIndex = Slim::Player::Source::streamingSongIndex($client);
# If we're streaming a different song than we're playing and
# moving either to or from the streaming song position, flush
# the streaming song, because it's no longer relevant.
if (($playingIndex != $streamingIndex) &&
(($streamingIndex == $src) || ($streamingIndex == $dest) ||
($playingIndex == $src) || ($playingIndex == $dest))) {
Slim::Player::Source::flushStreamingSong($client);
}

my $queue = $client->currentsongqueue();

for my $song (@$queue) {
my $index = $song->index();
Expand All @@ -611,6 +623,26 @@ sub moveSong {
$song->index(($dest>$src)? $index - 1 : $index + 1);
}
}

# If we're streaming a different song than we're playing and
# moving either to or from the streaming song position, flush
# the streaming song, because it's no longer relevant.
if (($playingIndex != $streamingIndex) &&
(($streamingIndex == $src) || ($streamingIndex == $dest) ||
($playingIndex == $src) || ($playingIndex == $dest))) {
$log->info("move+replace streaming (not playing) track");
Slim::Player::Source::flushStreamingSong($client);
}

# if we are either moving the last track or moving one after it, we
# might need to relaunch the process if it has been streamed fully.
if (($playingIndex == $streamingIndex) &&
($streamingIndex == count($client) - 1) &&
($src == $streamingIndex || $dest == $streamingIndex)) {
$log->info("moving last track itsef or another track past it");
$client->controller->nextIfStreamed($client);
}


refreshPlaylist($client);
}
Expand Down
3 changes: 3 additions & 0 deletions Slim/Player/Squeezebox2.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,9 @@ sub flush {

$client->stream('f');
$client->SUPER::flush();

# once flush, don't wait for answer, just get ready
$client->readyToStream(1);
return 1;
}

Expand Down
26 changes: 21 additions & 5 deletions Slim/Player/StreamingController.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,8 @@ Flush =>
[ [ \&_Invalid, \&_BadState, \&_BadState, \&_Invalid], # STOPPED
[ \&_BadState, \&_Invalid, \&_Invalid, \&_BadState], # BUFFERING
[ \&_BadState, \&_Invalid, \&_Invalid, \&_BadState], # WAITING_TO_SYNC
[ \&_Invalid, \&_FlushGetNext,\&_FlushGetNext,\&_Invalid], # PLAYING
[ \&_Invalid, \&_FlushGetNext,\&_FlushGetNext,\&_Invalid], # PAUSED
[ \&_FlushGetNext,\&_FlushGetNext,\&_FlushGetNext,\&_FlushGetNext], # PLAYING
[ \&_FlushGetNext,\&_FlushGetNext,\&_FlushGetNext,\&_FlushGetNext], # PAUSED
],
Skip =>
[ [ \&_StopGetNext, \&_BadState, \&_BadState, \&_NoOp], # STOPPED
Expand All @@ -164,7 +164,6 @@ JumpToTime =>
[ \&_JumpToTime, \&_JumpToTime, \&_JumpToTime, \&_JumpToTime], # PLAYING
[ \&_JumpPaused, \&_JumpPaused, \&_JumpPaused, \&_JumpPaused], # PAUSED
],

NextTrackReady =>
[ [ \&_NoOp, \&_BadState, \&_BadState, \&_Stream], # STOPPED
[ \&_BadState, \&_Invalid, \&_Invalid, \&_BadState], # BUFFERING
Expand All @@ -186,7 +185,6 @@ LocalEndOfStream =>
[ \&_Invalid, \&_Streamout, \&_Invalid, \&_Invalid], # PLAYING
[ \&_Invalid, \&_Streamout, \&_Invalid, \&_Invalid], # PAUSED
],

BufferReady =>
[ [ \&_Invalid, \&_BadState, \&_BadState, \&_Invalid], # STOPPED
[ \&_BadState, \&_WaitToSync, \&_WaitToSync, \&_BadState], # BUFFERING
Expand Down Expand Up @@ -442,7 +440,7 @@ sub _CheckPaused { # only called when PAUSED
} elsif (!$song->duration()) {

# Bug 7620: stop remote radio streams if they have been paused long enough for the buffer to fill.
# Assume unknown duration means radio and so we shuould stop now
# Assume unknown duration means radio and so we should stop now
main::INFOLOG && $log->info("Stopping remote stream upon full buffer when paused (no resume)");

_Stop(@_);
Expand Down Expand Up @@ -979,6 +977,9 @@ sub _Skip {
sub _FlushGetNext { # flush -> Idle; IF [moreTracks] THEN getNextTrack -> TrackWait ENDIF
my ($self, $event, $params) = @_;

# flush means that we get rid of the streaming song
shift @{$self->{'songqueue'}};

foreach my $player (@{$self->{'players'}}) {
$player->flush();
}
Expand Down Expand Up @@ -2112,6 +2113,21 @@ sub closeStream {
$_[0]->{'songStreamController'}->close() if $_[0]->{'songStreamController'};
}

sub nextIfStreamed {
my ($self) = @_;

# this is called when we are adding/moving another track after last one
# or moving it upper in the list. If it has been fully streamed and we
# are playing/buffering/waiting, then we need to grab next track. If we
# are paused then we'll restart the process later upon resume
if ($self->{'streamingState'} == IDLE &&
$self->{'playingState'} != STOPPED &&
$self->{'playingState'} != PAUSED) {
main::INFOLOG && $log->info("getting next track to re-launch streaming process");
_getNextTrack($self);
}
}

####################################################################
# Incoming events - <<interface>> PlayControl

Expand Down