Player restart - fix unexpected playback start on reconnects #797
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
michaelherger
requested changes
Jul 25, 2022
|
@philippe44 - any opinion other than "this is a nightmare"? 😂 |
I'll be back at a real computer in a few days and will be able to be more constructive 😃 |
mw9
force-pushed
the
feature/play_on_reconnect_issues
branch
from
af74f15
to
bfbe4bc
Compare
Now done. |
mw9
force-pushed
the
feature/play_on_reconnect_issues
branch
from
9369282
to
6b00288
Compare
This is a fix to the changes introduced by Player restart proposal LMS-Community#699, LMS-Community#699. A client that disappears/disconnects for a few seconds/minutes before reconnecting may start playing immediately it reconnects, for no good reason. This change ensures that a client's playing state is recorded as soon as the client disappears/disconnects. The playing state is used to determine if the client should restart playback when it connects to LMS, so it is essential that it is recorded as soon as the client is disconnected. At present, the playing state (recorded in preference 'playingAtPowerOff') is not recorded until LMS starts to 'forget' the disconnected client. This will take place 5 minutes after 'slimproto_close', via a timer that calls 'forget_disconnected_client'. But if the client reconnects in the interim, '_hello_handler' will kill the relevant timer, and the true playing state is not recorded. It may, in fact, reflect a condition that existed some days/weeks in the past.
On power on, or reconnection, clients will automatically start playback if the persistent preference setting 'playingAtPowerOff' is enabled. Once acted upon, this setting is no longer valid. LMS will set it again if required, i.e. on Power off, or a player disconnection. This "belt & braces" change explicitly resets 'playingAtPowerOff' once it has been acted upon. It is intended to protect against unexpected resumption of playback that may occur in otherwise unforeseen circumstances.
mw9
force-pushed
the
feature/play_on_reconnect_issues
branch
from
6b00288
to
8b81f02
Compare
Merged
|
I think that I have addressed the point that you raised. Is there anything else that I can do to clarify ? |
|
I had hoped that @philippe44 could give some feedback, too. |
|
Let's give this a try. Thanks @mw9! |
|
I'm finally really back home now and restarting everything. I will have a look |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I have been recently suffering some very unexpected behaviour from one of my Squeezebox Radios. It would randomly start playing back at all hours of the day and night.
I eventually realized that the behaviour derived from:
The nub of the problem is that, when
Slim::Networking::Slimproto::slimproto_closecloses the connection on loss of heartbeat, the Radio's current playing state is not immediately saved. Saving is tied toforget_disconnected_client, which doesn't happen for 5 minutes. And it doesn't happen at all in my circumstances, because the Radio reconnects well within 5 minutes.So when the Radio reconnected, within some 30/40 seconds, its playing state was always assessed to be 'On'. That derives from my exercise of the 'Power off' button, a week or so ago. Unwanted playback ensued, frequently.
Proposed changes:
This removes the 5 minute wait to save the client's current playing state, so the state is saved as soon as the connection is closed. It fixes the immediate issue. I've used a subroutine,
persistPlaybackStateForPowerOff, for readability.This attempts to add some robustness/hardening to the process.
The client's playing state is saved in a, persistent, preference
playingAtPowerOff. If 'On', playback will resume on Power on/reconnect. Once that is done, the goal has been achieved and the 'On' state is no longer valid. So it should be immediately reset.This would have prevented the issue I experienced.
Just that, an attempt to make the above more readable.