magnus-ISU / videospeed

HTML5 video speed controller (for Firefox). WebExtensions port of Video Speed Controller Chrome extension.
24 stars 3 forks source link

Add 0.00x option #30

Closed Rijaja closed 4 months ago

Rijaja commented 5 months ago

Add the option to go down to 0.00x, effectively pausing the media, which is useful on some websites that won't let you do it. It is off by default, keeping the default behavior of the extension. I also renamed a few variables that weren't very clear in the addSpeed function.

I'd also like to bring attention to the min_speed and max_speed. They are set to 0.07x and 16x respectively. This is because of a limitation in how Chromium handles media, but this limitation does not exist on Firefox. I'd like to suggest two new ideas for future features:

Rijaja commented 5 months ago

The addSpeed function also had this:

// Make sure no matter how small s is, we can increase it
if (s < MIN_SPEED) s = MIN_SPEED
// Clamp to max and min
/* ... */
s = Math.max(s, MIN_SPEED)

Which seems redundant.

Rijaja commented 5 months ago

@magnus-ISU Can you take a look at my PR? This is my first ever public PR so please tell me if I did something wrong

magnus-ISU commented 5 months ago

Somehow I didn't get tagged.

I don't like configuration options, so I'd like it to be always on; I'll test if that breaks anything, which I doubt.

magnus-ISU commented 5 months ago

Though I guess custom min/max would maybe make sense and be easy to do.

Rijaja commented 5 months ago

I have removed the configuration option. Though I'm not a fan of how the speed becomes relative to 0.07x. For example with 0.05 increments, if you go down from 0.10, you skip 0.05 and go straight to 0.00, but then if you go back up you skip 0.05 and go to 0.07 instead, and then it's 0.12, 0.17, 0.22, etc. This is not new from my PR but I could look into changing that, to always be relative to 1.00, if you think it matters. There's already code in place to clamp to 1.00 when nearing that value so it's not catastrophic either.

Rijaja commented 4 months ago

@magnus-ISU is my PR good? sorry for pinging again

magnus-ISU commented 4 months ago

If I don't merge this tonight or make comments, ping me again. I've been busy, but forgotten to look at this when I've had time. I do tonight

magnus-ISU commented 4 months ago

I've worked on projects with impossible-to-reach maintainers, upstream on this project was one. So I know it's frustrating. I'm sorry

magnus-ISU commented 4 months ago

Merged