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

add sponsorblock support for videos with chapters #103

Merged
merged 1 commit into from
Mar 25, 2024

Conversation

alyyousuf7
Copy link

@alyyousuf7 alyyousuf7 commented Nov 1, 2023

Here is my attempt to add feature to support SponsorBlock on videos with Chapters.

I was able to iron out issues that #9 faced w.r.t infinite loop - i resolved by mounting segment overlay on parent node

I tested it locally on my computer (by mocking User-Agent + TamperMoney to inject the userscript), as well as on my LG webOS TV

User-Agent: LG Browser/8.00.00 (webOS.TV-2022), _TV_O18/03.33.80 (LG, 55UQ7590PUB, wireless)

I installed webpack-dev-server locally to avoid rebuilding manually on every change

image

Known Issue

The SponsorBlock segments do not change height when user is seeking the progress bar - it stays the same height. This is because we currently do not detect which chapter the video is at.
image

@Informatic
Copy link
Member

Informatic commented Nov 2, 2023 via email

@alyyousuf7
Copy link
Author

@Informatic did you get a change to look at it?

@cremor
Copy link

cremor commented Mar 16, 2024

@Informatic @throwaway96 I've tested this on an LG OLED C9 (webOS 4.9.7) and it works fine. Would be great if it could be merged.

@throwaway96
Copy link
Member

Anyone know of a good video to test this on?

@alyyousuf7
Copy link
Author

Anyone know of a good video to test this on?

May be try any of Linus Tech Tip videos? Their videos have pretty well defined chapters and full of sponsors.

One sample video from their channel: https://www.youtube.com/watch?v=i9TJWsuzBLU&ab_channel=LinusTechTips

@throwaway96
Copy link
Member

It does appear to work. Changes to the SponsorBlock settings don't seem to take effect on the current video, but that has always been the case, right?

@alyyousuf7
Copy link
Author

Changes to the SponsorBlock settings don't seem to take effect on the current video, but that has always been the case, right?

Do you mean toggling off SponsorBlock to show/hide the relevant timestamps and/or changing the different categories?
Yes, this was always the case.

@throwaway96 throwaway96 merged commit 44f7c82 into webosbrew:main Mar 25, 2024
1 check passed
@alyyousuf7 alyyousuf7 deleted the chapter-sponsorblock branch March 25, 2024 03:39
throwaway96 added a commit that referenced this pull request Mar 25, 2024
@throwaway96
Copy link
Member

throwaway96 commented Mar 27, 2024

I just got a chance to take a closer look at this after noticing a lot of log activity. Can you please explain why you changed the interval for starting the MutationObserver to 100ms? Is there a perceptible difference? 10 times per second seems like a lot.

It would be nice to get rid of the setInterval() part altogether.

@alyyousuf7
Copy link
Author

It's been a while I wrote this - IIRC, YouTube renders a slider without chapters first, only after a while it finishes loading the data for chapters, it unmounts the non-chapter slider (progress-bar) and renders another slider (multi-markers-player-bar) with chapters after a while.

So when the slider gets unmounted, it tries to look for a slider, so that we can place our segment overlay on top of it.

I'll take a second look at it and see if we can get rid of the setTimeout.

@throwaway96
Copy link
Member

Is it supposed to be running continuously during playback though? I didn't look into it that deeply, but I'm pretty sure I was seeing messages logged every 100ms throughout most of the time a video was playing.

@alyyousuf7
Copy link
Author

It isn't supposed to run during playback.

@throwaway96
Copy link
Member

It seems like this may be causing issues on some videos (#148).

@alyyousuf7

Any ideas on what could be causing this? I don't really have time to look into it at the moment.

I want to do a release soon—as in today or tomorrow—with the other recent fixes, so I may (temporarily) revert this.

@throwaway96
Copy link
Member

Seems like this wasn't the cause of #148. I had assumed it was since it was pretty much the only thing to touch SponsorBlock, but this issue already existed.

So I'll leave this in for the release candidate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants