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

Jump to bottom

Added support for specifying minimum skip length #169

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
+29 −3
Open

Added support for specifying minimum skip length #169

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
138c97c
Added support for specifying minimum skip length
bourkemcrobbo Jun 10, 2024
1c4ffe6
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jun 10, 2024
4d4016c
Fixed bad static method argument
bourkemcrobbo Jun 18, 2024
2636a4b
Fixed bad static method argument
bourkemcrobbo Jun 18, 2024
6c679f2
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jun 18, 2024
2b1b0f6
Fixed bad static method argument
bourkemcrobbo Jun 18, 2024
16b8fa1
Merge branch 'minimum-skip-length' of https://github.com/bourkemcrobb…
bourkemcrobbo Jun 18, 2024
cab33e5
Merge branch 'main' into minimum-skip-length
bourkemcrobbo Jun 27, 2024
0e87da5
Updated setup to match new format. Set default value of skip length t…
bourkemcrobbo Jun 27, 2024
d11bcba
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jun 27, 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
1 change: 1 addition & 0 deletions config.json.template
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
"skip_count_tracking": true,
"mute_ads": true,
"skip_ads": true,
"minimum_skip_length": 1,
"apikey": "",
"channel_whitelist": [
{"id": "",
Expand Down
9 changes: 6 additions & 3 deletions src/iSponsorBlockTV/api_helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ def __init__(self, config, web_session: ClientSession) -> None:
self.skip_count_tracking = config.skip_count_tracking
self.web_session = web_session
self.num_devices = len(config.devices)
self.minimum_skip_length = config.minimum_skip_length

# Not used anymore, maybe it can stay here a little longer
@AsyncLRU(maxsize=10)
Expand Down Expand Up @@ -146,10 +147,10 @@ async def get_segments(self, vid_id):
if str(i["videoID"]) == str(vid_id):
response_json = i
break
return self.process_segments(response_json)
return self.process_segments(response_json, self.minimum_skip_length)

@staticmethod
def process_segments(response):
def process_segments(response, minimum_skip_length):
segments = []
ignore_ttl = True
try:
Expand Down Expand Up @@ -191,7 +192,9 @@ def process_segments(response):
segment_dict["start"] = segment_before_start
segment_dict["UUID"].extend(segment_before_UUID)
segments.pop()
segments.append(segment_dict)
# Only add segments greater than minimum skip length
if segment_dict["end"] - segment_dict["start"] > minimum_skip_length:
segments.append(segment_dict)
except Exception:
pass
return segments, ignore_ttl
Expand Down
12 changes: 12 additions & 0 deletions src/iSponsorBlockTV/config_setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,18 @@ def main(config, debug: bool) -> None:

config.channel_whitelist = channel_whitelist

# Ask for minimum skip length. Confirm input is an integer
while True:
try:
minimum_skip_length = int(
input("Enter minimum length of segment to skip in seconds: ")
)
break
except ValueError:
print("You entered a non integer value, try again.")
continue
config.minimum_skip_length = minimum_skip_length

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #150 was merged probably between when you raised this one and now which refactored this script a bit. It should be pretty simple to adapt this to use the get_yn_input function - though truthfully this is likely fine as is. But you will need to pull the changes for the rest of the script to merge without conflict.

It also might be helpful for the user to include something like "set this to 0 to not skip any segments" or something to that effect.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, #150 was merged after raising this PR, but refactoring this section of the cli configurator shouldn't be too much work, and ideally a new "panel" would be added to the tui config editor to allow changing this value from there too. I'll be able to do it at some point but if someone wants to go ahead and help you're free to do it. PRs are always appreciated. Thanks for your comment @ryankupk

config.skip_count_tracking = (
not input(
"Do you want to report skipped segments to sponsorblock. Only the segment"
Expand Down
1 change: 1 addition & 0 deletions src/iSponsorBlockTV/helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ def __init__(self, data_dir):
self.skip_count_tracking = True
self.mute_ads = False
self.skip_ads = False
self.minimum_skip_length = 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that the minimum ought to be 0 rather than 1, since sponsorblock segments are defined by frames so there could be a segment that's less than 1 second that would be skipped when the user might think that no skipping is happening.

Although one of the configurators will likely always be ran, so the user will define a minimum segment length, I still think that ideally the default would represent no skipping.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find 1 to be an appropriate minimum time length, considering that YouTube only allows setting the play head to integer values (so no 10.8 seconds allowed, it'll just round it down to 10). If a user were to want to disable skipping, they should be able to just set the list of skippable categories to an empty one (that should work but it isn't something I've tested)

self.__load()

def validate(self):
Expand Down