fix: clearing watch positions doesn't works sometimes #5557
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
|
This PR assumes that the timer task is still running in the background even though the player fragment already got killed? I think it'd be easier to just release the timer in the |
manish99verma
changed the title
manish99verma
commented
Jan 26, 2024
Bnyro
reviewed
Jan 26, 2024
| @@ -252,6 +251,9 @@ class PlayerFragment : Fragment(), OnlinePlayerOptions { | |||
| } | |||
| } | |||
|
|
|||
| // schedule task to save the watch position each second | |||
| private var watchPositionTimer = Timer() | |||
There was a problem hiding this comment.
Suggested change
| private var watchPositionTimer = Timer() | |
| private var watchPositionTimer: Timer? = null |
We don't need to initialize the timer here - it's going to be overwritten before the first usage anyways.
Bnyro
reviewed
Jan 26, 2024
| @@ -276,6 +278,18 @@ class PlayerFragment : Fragment(), OnlinePlayerOptions { | |||
| 100 | |||
| ) | |||
| } | |||
|
|
|||
| //Start or pause watch position timer | |||
There was a problem hiding this comment.
Suggested change
| //Start or pause watch position timer | |
| // Start or pause watch position timer |
Bnyro
reviewed
Jan 26, 2024
| @@ -907,7 +912,7 @@ class PlayerFragment : Fragment(), OnlinePlayerOptions { | |||
|
|
|||
| val videoStream = streams.videoStreams.firstOrNull() | |||
| val isShort = PlayingQueue.getCurrent()?.isShort == true || | |||
| (videoStream?.height ?: 0) > (videoStream?.width ?: 0) | |||
| (videoStream?.height ?: 0) > (videoStream?.width ?: 0) | |||
There was a problem hiding this comment.
Please undo that formatting change.
Bnyro
reviewed
Jan 26, 2024
| @@ -945,7 +950,7 @@ class PlayerFragment : Fragment(), OnlinePlayerOptions { | |||
| if (binding.playerMotionLayout.progress != 1.0f) { | |||
| // show controllers when not in picture in picture mode | |||
| val inPipMode = PlayerHelper.pipEnabled && | |||
| PictureInPictureCompat.isInPictureInPictureMode(requireActivity()) | |||
| PictureInPictureCompat.isInPictureInPictureMode(requireActivity()) | |||
There was a problem hiding this comment.
Please undo that formatting change.
|
Just some nitpick comments, the idea generally looks good to me 👍 |
Bnyro
changed the title
Bnyro
changed the title
manish99verma
commented
Jan 26, 2024
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.
No description provided.