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.

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

Autoplay time limit #5871

Draft
wants to merge 2 commits into
base: development
Choose a base branch
from

Conversation

kommunarr
Copy link
Collaborator

Autoplay time limit

Pull Request Type

  • Feature Implementation

Related issue

closes #4270

Description

Implements time-limited autoplay feature which prevents the next video from playing automatically after X hours of 0 click or keydown events. This is set to a default of 3 hours, but it can be configured to be up to 12. This timer resets back to the start for every click or keydown event made at any time in the window. The closely related Next Video Interval EN-US label is also renamed to Autoplay Countdown Timer to minimize user confusion regarding its meaning.

This feature will be nice for folks like me who occasionally fall asleep to videos and have to reset watch history for a good chunk of them. It should also reduce net power draw and bandwidth expenditure for asleep or otherwise AFK users who neglected to hit the pause button.

To preemptively answer the question of "why not allow infinity," I don't think allowing indefinite automatic streaming regardless of user interaction is ever a desirable state for 99+% of our users. The default of 3 hours without any detected click or keyboard activity is a fine sweet spot for minimizing streaming to sleeping/AFK users, much more generous than YouTube's or Netflix's, to name a couple of similar examples. 12 hours of total inactivity before blocking autoplay should be more than enough for users with very particular edge cases.

Testing

Note: I have this a draft because there are still a couple of autoplay PRs out, so I'd prefer not to make anyone have to re-validate post-merge conflicts.

  • on line 1630 of Watch.js:
  • this.autoplayInterruptionTimeout = setTimeout(() => { this.blockVideoAutoplay = true }, this.defaultAutoplayInterruptionInterval * 3_600_000)
    • Replace it with this line to make the timeout time much lower:
this.autoplayInterruptionTimeout = setTimeout(() => { this.blockVideoAutoplay = true }, this.defaultAutoplayInterruptionInterval * 10_000)
  • Then enable Recommended Videos Autoplay and/or Playlist Videos Autoplay in settings.
  • Then you can run such validations like:
    • Ensure the timer is reset by any click or key event (e.g., by seeking close to the end of a video by a time slightly greater than the countdown time + autoplay interruption timer combined, then clicking 15 seconds in)
    • Ensure that the next video is blocked from starting automatically if no user interaction is made for an amount of time that is >= the autoplay interruption timer

Desktop

  • OS: OpenSUSE
  • OS Version: TW
@absidue
Copy link
Member

absidue commented Oct 15, 2024

I know this pull request is still in draft state, but I think I have a different understanding of the feature request, so I wanted to mention it now, so we can already start a discussion around it.

In FreeTube we have 3 different types of auto play:

  • start playing the video automatically
  • go to next playlist video after the current one ends
  • go to next recommended video after the current one ends

From reading the feature request and pull request description I would have expected this pull request to affect the later two settings, so that when the timer ends it finishes playing the current video and then just stays there. However the code in this pull request seems to target the first one, so when the timer ends it will finish playing the current video, go to the next one and then stop there. That means when the timer expires, it still does a bunch of work to get the new data and process it, updates the UI, downloads new images. Loading the next video also means that it is likely to get marked as watched.

@kommunarr
Copy link
Collaborator Author

That is indeed a fair interpretation and set of implications. My main consideration with modifying the autoplay setting (qua the second / third definitions) was that I didn't want to disable the persistent setting on this condition, and I was thinking that such a feature would necessitate us implementing the ability for non-permanent settings modifications, although I realize now that we could instead achieve this logic by a Watch.js handleVideoEnded check (similar to the Playlist Pause button). I'm leaning toward that new solution now unless there are any other alternatives I have not considered.

@absidue
Copy link
Member

absidue commented Oct 15, 2024

Handling it in Watch.js sounds like the best solution, you don't even need to change the logic in handleVideoEnded (only thing needed there would be to cancel the timeout), as you can change the logic in the autoplayPlaylists and playNextVideo computed props to return value of setting and not auto play timer expired (currently they just return the value of the setting).

i.e., instead of 'start video automatically' autoplay
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@absidue absidue added the DO NOT MERGE UNTIL AFTER RELEASE Do not merge before the next release as this is not a bug fix label Oct 16, 2024
@ChunkyProgrammer ChunkyProgrammer removed the DO NOT MERGE UNTIL AFTER RELEASE Do not merge before the next release as this is not a bug fix label Oct 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment