Allow a speed multiplier for L and J keys #729

mklaber · 2021-04-18T14:29:34Z

This PR addresses an issue identified in #81 and alluded to in #254: speeding up playback via the keyboard at an exponential rate. It does this by adding a SHIFT modifier key to the existing J and L commands. When in use (by default) a series of L presses will go from stopped → playing at 100%, 200%, 400%, 800%, and stop at 1600%. J will go from stopped → playing at 100%, 50%, 25%, 13%, and stop at 10%.

Adds a SHIFT modifier to L and J
Adds a setting that allows the user to pick the multiplier rate. The two options are 2x/0.5x and 1.1x/0.9091x. These reflect two sets of rates available in the key binding defaults of IINA which attributes the values to mpv and Movist. (There's a third set attributed to VLC which is 1.5x/0.6667x; if LC's settings UI had a convention for selecting a third option, this could be added.)
If speeding up or slowing down (whether using the modifier or not) brings the user from slo-mo to sped-up (or vice-versa), it makes sure to stop at 1x/100% along the way. (This allows the user to use a combination of multiplied speeds and the existing 15% boost without getting stuck unable to return to real-time.)
Adds documentation for the new commands to the HelpSheet

smurfix · 2021-06-21T08:41:05Z

I'm more a fan of an 1.25 multiplier (or rather 1.25992105, i.e. ∛2, thus hitting it three times would get you 2.0). 1.1 is hardly noticeable. 1.5 is too much for speech.
I'm also a fan of dropping the linear advance entirely. Instead SHIFT+JL should use a factor of 0.5/2 instead of 1.25.
Another suggested improvement: when speed changes from <0.98 to >0.98 (or vice versa 1.02), pin it to 1.0 so that rounding errors and/or stepping off the clamping value don't prevent you from going back to exactly 1.0.

Other than that, is there anything blocking this PR? I'd love to have it.

mklaber · 2021-06-21T09:52:16Z

I'm more a fan of an 1.25 multiplier (or rather 1.25992105, i.e. ∛2, thus hitting it three times would get you 2.0). 1.1 is hardly noticeable. 1.5 is too much for speech.

If I were only implementing for myself, I'd have just done the 2x/0.5x. The other option exists because there does not seem to be a consensus across existing tools. I'd be more than happy to accept a pull request that adds a third set of options. (Note that not everyone cares about speech when jogging through content.)

I'm also a fan of dropping the linear advance entirely. Instead SHIFT+JL should use a factor of 0.5/2 instead of 1.25.

Not sure what you mean here?

Another suggested improvement: when speed changes from <0.98 to >0.98 (or vice versa 1.02), pin it to 1.0 so that rounding errors and/or stepping off the clamping value don't prevent you from going back to exactly 1.0.

See bullet point #3.

Other than that, is there anything blocking this PR? I'd love to have it.

Not that I know of :-)

smurfix · 2021-06-21T13:05:22Z

Not sure what you mean here?

Currently the 15% increment is borderline useful when going from 100% to 115% but completely useless at 400%, you don't even notice. On the other hand at 15% it doubles the video's speed.

So instead of using the Shift key to use a factor, IMHO we should switch to an entirely multiplicative setup and use the Shift key to increase the factor from 1.26 to 2. Or something along these lines.

See bullet point #3.

Bah. I need to not write bug reports when not really awake.

mklaber · 2021-06-21T13:34:24Z

So instead of using the Shift key to use a factor, IMHO we should switch to an entirely multiplicative setup and use the Shift key to increase the factor from 1.26 to 2. Or something along these lines.

Ah so basically behaviour similar to this PR should be the default rather than whatever it is now. (And by "whatever it is" I mean "whatever irritated me enough to submit a PR.")

Bah. I need to not write bug reports when not really awake.

So say we all.

mklaber · 2021-08-09T13:35:21Z

@mifi any chance this will get merged?

mifi

Thanks for your PR! I agree with @smurfix that multiplicative could be the default behavior. Then maybe we don't need a shift-modifier? Or the shift modifier could add another multiplier on top of the user-configured multiplier?

mifi · 2021-08-23T05:54:34Z

public/configStore.js

@@ -38,6 +38,7 @@ const defaults = {
  keyboardNormalSeekSpeed: 1,
  enableTransferTimestamps: true,
  outFormatLocked: undefined,
+  speedMultiplier: '2x',


Why is this stored as a string with an 'x'-suffix? Can't we just store it as a number? If we use a number, I believe we can remove the whole switch/case logic and just write calculatedRate = video.playbackRate * (dir > 0 ? speedMultiplier : 1 / speedMultiplier)

The string '2x' represents a selection in Settings.jsx which has both a "speed up" and a "slow down" value. This string could better be labeled '2x/0.5x' or something.

...checks math...

So... yea... (r ÷ 2) == (r × 0.5)... so, should be able to store as a number.

But that does lead to a question about how this setting should be presented (if at all)... I'll add another comment about that.

I believe we can still have discrete options in the settings panel (e.g. { label: '2x / 0.5x', value: '2' }, { label: '1.1x / 0.9091x', value: '1.1' }), just parse it to float when storing it.

mifi · 2021-08-23T05:55:18Z

src/App.jsx

@@ -1088,12 +1088,38 @@ const App = memo(() => {
    if (!playing) {
      video.play();
    } else {
+      let calculatedRate;
+      if (useMultiplier) {
+        switch (true) {


Why not use if/else?

Style, entirely.

In my mind, at least, if/else implies some kind of cascading hierarchy of conditions. "Act on this condition before anything else." With a switch, you're basically saying "here are 4 mutually exclusive conditions that should be treated with equal weight." (Yea, nothing in a switch enforces mutual exclusivity and switch cases get handled top down in the same way that if/else would, but... again... style.)

But, this is likely moot based on other comments.

gotcha. I never really used the switch (true) style, so was a bit confused

mifi · 2021-08-23T05:56:15Z

src/App.jsx

@@ -1088,12 +1088,38 @@ const App = memo(() => {
    if (!playing) {
      video.play();
    } else {
+      let calculatedRate;


This logic is quite long so I would pull it out into a util function in another file

mifi · 2021-08-23T06:12:14Z

src/App.jsx

+            calculatedRate = video.playbackRate * 0.9091;
+            break;
+          default:
+            calculatedRate = 1.0;


When does this happen?

config file: speedMultiplier: "fooBar", this avoids multiplying with null 😬

I think in the default case it would be better to default to a multiplier value of maybe 1.1, instead of just setting playback rate to 1 all the time

smurfix · 2021-08-23T06:55:49Z

src/App.jsx

+      // if the multiplier causes us to go faster than real time or slower than real time, stop along the way at 1.0
+      if ((calculatedRate > 1.0 && video.playbackRate < 1.0) || (calculatedRate < 1.0 && video.playbackRate > 1.0)) {
+        calculatedRate = 1.0;
+      }


This logic risks skipping a step. Consider what happens when rounding errors cause the rate, while stepping down, to be sliiiightly above 1.0. Now you step down and end up at 1.0.

Better: set the rate to 1.0 if it is between multiplier^(-1/2) and multiplier^(1/2).

Oh you mean that if playbackRate is high and the user is pressing J-J-J, they may end up at 1.00001 which shows up in the UI as 100%, and then they press J again, and end up at 1, which also shows as 100%?

smurfix · 2021-08-23T07:04:33Z

src/App.jsx

      // https://github.com/mifi/lossless-cut/issues/447#issuecomment-766339083
-      const newRate = clamp(Math.round((video.playbackRate + (dir * 0.15)) * 100) / 100, 0.1, 16);
+      const newRate = clamp(Math.round(calculatedRate * 100) / 100, 0.1, 16);


Is there any good reason to do rounding here? This pretty much guarantees that multiplicative speed changes won't end up where you started from (unless you cross 1.0).

I'd do the rounding in the code that displays the value. I'd also change the display to show a factor (w/ two significant digits) instead of a percentage.

I think I added that because I wanted it to come back to 100% instead of 99%, see referenced issue #447 (comment)

The clamping code above this already takes care of that, at least if you rewrite it the way I sketched above. 😀

In any case, when you display the value you should probably round off to nearest integer (or fraction) instead of towards zero.

mattack1 · 2021-08-23T18:23:58Z

I'm more a fan of an 1.25 multiplier (or rather 1.25992105, i.e. ∛2, thus hitting it three times would get you 2.0). 1.1 is hardly noticeable. 1.5 is too much for speech.

I'm not sure if you mean actually playing/watching at this speed (or just trying to skim through to find something). As an aside, there's a backdoor speedup on more recent Tivos.. The default speedup is only 30%. I use the backdoor speedup VERY often (on most news/reality/documentary type shows). 50% is about the lowest I ever go to, but use 70% and 90% faster on different kinds of content... and the shows I use it on, 90% faster is intelligible to me. (The way the backdoor speedup works, it uses 1-9 keys to use that many tens of percent, so 1.9x is the fastest it goes.)

I also have used VLC to play faster than realtime many times, and use its 2x too.

mklaber · 2021-08-24T11:26:02Z

@mifi @smurfix if I am grokking your comments correctly, this PR should be modified such that:

A speed multiplier is applied by default (rather than the current +15%). This would be a breaking change but perhaps a desirable one. The need for modifier keys would be removed.
The multiplier applied should still be a setting but can be represented as a "tuner" between 1.1 and 2.0. This would allow for 1.1, 2.0, and anywhere in between.

Is that right?

Is there any use case for retaining the current "+15%" rate?

If that's how we should proceed, I'll address the comments from @smurfix regarding the 1.0 rate questions/rounding there.

smurfix · 2021-08-24T12:36:50Z

Correct.

While there is no need for a modifier, you might want the Shift key to change the stepping so that if j/l divides multiplies by some factor f, Shift+j/l uses f³.

No, I don't think a fixed 15% stepping is useful for anybody. I challenge you to discern any difference between 400% and 415%. At the same time, a change from 10% to 25% is a speed-up by 2.5.

mklaber · 2021-08-27T08:26:07Z

No, I don't think a fixed 15% stepping is useful for anybody. I challenge you to discern any difference between 400% and 415%. At the same time, a change from 10% to 25% is a speed-up by 2.5.

lol, challenge not accepted.

Unless @mifi weighs in with a preference before I get around to updating the PR, I'll just remove the multiplier setting altogether (I'll do ∛2 with a shift modifier of √2).

smurfix · 2021-08-27T08:36:25Z

(I'll do ∛2 with a shift modifier of √2).

ITYM a shift modifier of 2? Otherwise "press tree times without shift but twice with shift" to achieve the same effect doesn't look like enough of a difference to implement the shift mod in the first place.

mifi · 2021-08-28T07:40:53Z

I believe quite a few people have asked for a configurable setting, but i'm not sure if that's because they were frustrated by the slow default linear speed up, or if they geniunely need to configure it. i'm open for no setting first, then maybe add a setting later

mklaber · 2021-08-30T12:31:02Z

@smurfix and @mifi I created a new PR for this: #840

Mind taking a look? I think I've addressed @smurfix 's suggestions though I did not change how the rate change is displayed--can you elaborate more on what you meant by "probably round off to nearest integer (or fraction) instead of towards zero." ?

Allow a speed multiplier for <kbd>L</kbd> and <kbd>J</kbd>

cd2558f

mifi reviewed Aug 23, 2021

View reviewed changes

smurfix reviewed Aug 23, 2021

View reviewed changes

mifi mentioned this pull request Aug 23, 2021

Two suggestions #725

Closed

mklaber mentioned this pull request Aug 30, 2021

Change playback speed exponentially #840

Merged

mifi closed this in #840 Nov 14, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow a speed multiplier for L and J keys #729

Allow a speed multiplier for L and J keys #729

mklaber commented Apr 18, 2021

smurfix commented Jun 21, 2021

mklaber commented Jun 21, 2021

smurfix commented Jun 21, 2021

mklaber commented Jun 21, 2021

mklaber commented Aug 9, 2021

mifi left a comment

mifi Aug 23, 2021

mklaber Aug 24, 2021

mifi Aug 24, 2021

mifi Aug 23, 2021

mklaber Aug 24, 2021

mifi Aug 24, 2021

mifi Aug 23, 2021

mifi Aug 23, 2021

smurfix Aug 23, 2021

mifi Aug 23, 2021

smurfix Aug 23, 2021

mifi Aug 24, 2021

smurfix Aug 24, 2021

smurfix Aug 23, 2021

mifi Aug 23, 2021

smurfix Aug 23, 2021

mattack1 commented Aug 23, 2021

mklaber commented Aug 24, 2021

smurfix commented Aug 24, 2021

mklaber commented Aug 27, 2021

smurfix commented Aug 27, 2021

mifi commented Aug 28, 2021

mklaber commented Aug 30, 2021

Allow a speed multiplier for L and J keys #729

Allow a speed multiplier for L and J keys #729

Conversation

mklaber commented Apr 18, 2021

smurfix commented Jun 21, 2021

mklaber commented Jun 21, 2021

smurfix commented Jun 21, 2021

mklaber commented Jun 21, 2021

mklaber commented Aug 9, 2021

mifi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattack1 commented Aug 23, 2021

mklaber commented Aug 24, 2021

smurfix commented Aug 24, 2021

mklaber commented Aug 27, 2021

smurfix commented Aug 27, 2021

mifi commented Aug 28, 2021

mklaber commented Aug 30, 2021