Autoplay time limit

[kommunarr]

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

• on line 1630 of Watch.js:

• FreeTube/src/renderer/views/Watch/Watch.js

  Line 1630 in 27f59e0

  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]

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]

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]

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).

[github-actions]

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

[github-actions]

Conflicts have been resolved. A maintainer will review the pull request shortly.

[efb4f5ff-1298-471a-8973-3d47447115dc]

One issue i see with this is the following case:

Slider is set to 3hrs

• User is actively watching a stream that exceeds the 3hr mark without interaction
• User is listening to a podcast that is longer than 3hrs without interaction

Yeah i know you can move the slider but that isnt the issue here. Some people might not want this feature be on by default with no way for it to turn off.

[kommunarr]

If the problem is the "people will complain about the default before checking the settings" issue like we had with the play button, I can change the default to 4 or even 5 hours. There's no one-size-fits-all default necessarily, because you run into a balancing issue of the % of people who are bothered by their videos going on too long while they're asleep / AFK versus the % of people who want to watch back-to-back 3+ hour videos without any pausing, adjusting fullscreen, changing the audio level, etc. I still think 3 is the closest magic number from my guesstimate, but open to hearing your opinion.

To your latter concern, if we get a singular user for whom the max of 12 hours without user interaction is too small, we can talk once they make an issue for it; but our concerns should be proportional to # of affected users and degree of affectedness, and both of those metrics are quite small in this case.

[PikachuEXE]

This should stay visible for longer (1 hour?)
New users (new to this setting) will take time to realize the playlist is stopped and switch back and wondering why it's stopped
Also the text should mention the setting name if possible, but at least should include the value (e.g. 3h)

[kommunarr]

How about this?

[PikachuEXE]

Looks better

[PikachuEXE]

I am surprised that linter is ok with this o_0

[kommunarr]

It not only tolerates it: it enforces it to be like that. ¯\(ツ)/¯

[absidue]

The reason that is happening is because showToast( and this.$t( are both on the same line, so the linter sees this.$t as having one level of indentation and showToast as having none. That is why it forcing 3_600_000 and ) to have no indentation.

Moving this.$t to its own line fixes the issue, as then the linter allows them both to have indentation:

It would also improve the general readability of that code snippet if the placeholder name in the translation string weren't so long e.g. {hours} would already suffice, as there is only one placeholder in that string and there is enough context in the string and the code where it is used, to understand what the purpose of the placeholder is.

However as both things are stylistic issues and don't influence the functionality of the code, we can leave the cleanup for some future pull request.

[efb4f5ff-1298-471a-8973-3d47447115dc]

To your latter concern, if we get a singular user for whom the max of 12 hours without user interaction is too small, we can talk once they make an issue for it; but our concerns should be proportional to # of affected users and degree of affectedness, and both of those metrics are quite small in this case.

Agreed.

Also

1. So i set the value to 1hr
2. Went to watch a long video
3. No activity for 1hr and 6mins
4. Didnt see a toast message and video didnt pause

Did i do something wrong?

[kommunarr]

Interesting, did you increase volume, touch any unrelated keys (e.g., F10), something like that at any point? I'd recommend replacing line 1630 with a shorter time period as per the PR description to make your testing experience a bit easier

[efb4f5ff-1298-471a-8973-3d47447115dc]

No i really didnt touch my machine for more than an hour

[kommunarr]

@efb4f5ff-1298-471a-8973-3d47447115dc Cannot replicate for recommended videos autoplay, user playlist autoplay, or remote playlist autoplay. I added logging statements on lines 1670 and 1671 of Watch.js as well, and did not see any unexpected behavior or condition triggering. If you could, replace this function in Watch.js as such to include these logging statements and see if you can replicate. This is using a 1s = 1hr time adjustment as well, but I tested at a smaller conversion rate and still could not replicate:

    resetAutoplayInterruptionTimeout() {
      console.log('clearing autoplay timeout')
      clearTimeout(this.autoplayInterruptionTimeout)
      this.autoplayInterruptionTimeout = setTimeout(() => { this.blockVideoAutoplay = true; console.log('blocking video autoplay') }, this.defaultAutoplayInterruptionIntervalHours * 1_000)
      this.blockVideoAutoplay = false
    },

[efb4f5ff-1298-471a-8973-3d47447115dc]

so this is interesting...it says its blocking but i dont see the toast message

[kommunarr]

Silly question, but are you testing with autoplay enabled? The toast message is not intended to appear until the video ends.

[efb4f5ff-1298-471a-8973-3d47447115dc]

Silly question, but are you testing with autoplay enabled?

Yup autoplay is enabled.

The toast message is not intended to appear until the video ends.

ooooh my bad i interpreted this wrong. I thought the video would pause or something and show the toast message. I didnt know the video has to end first

[efb4f5ff-1298-471a-8973-3d47447115dc]

Sorry about that, LGTM

[PikachuEXE]

@efb4f5ff-1298-471a-8973-3d47447115dc

[absidue]

The reason that is happening is because showToast( and this.$t( are both on the same line, so the linter sees this.$t as having one level of indentation and showToast as having none. That is why it forcing 3_600_000 and ) to have no indentation.

Moving this.$t to its own line fixes the issue, as then the linter allows them both to have indentation:

It would also improve the general readability of that code snippet if the placeholder name in the translation string weren't so long e.g. {hours} would already suffice, as there is only one placeholder in that string and there is enough context in the string and the code where it is used, to understand what the purpose of the placeholder is.

However as both things are stylistic issues and don't influence the functionality of the code, we can leave the cleanup for some future pull request.

[absidue]

Thanks GitHub for duplicating the comment lol.