feat(2d): add playbackRate signal to Video component

[rmhartog]

Added a playbackRate signal to the Video component that mirrors HTMLMediaElement.playbackRate.

Solves #711.

[aarthificial]

Don't have a way to test it rn but shouldn't fastSeekedVideo set the playbackRate attribute on the actual video element too?
Seems like the current implementation would cause the video to constantly get out of sync with the playback.

[rmhartog]

Don't have a way to test it rn but shouldn't fastSeekedVideo set the playbackRate attribute on the actual video element too? Seems like the current implementation would cause the video to constantly get out of sync with the playback.

Whenever the playbackRate is updated, it is also set on the video element. I've tested based on your comment, but did not find a scenario in which they'd get out of sync. Happy to dive into it further, but could you help me understand this scenario a little bit better?

[aarthificial]

Sorry, didn't notice you were using the HTML attribute to store the value.
In that case though, wouldn't the value reset whenever the source changes?
For example:

import videoA from './videoA.mp4';
import videoB from './videoB.mp4';

// ...

const video = createRef<Video>();
view.add(<Video ref={video} src={videoA} playbackRate={2} />);

// this will reset the playback rate back to 1:
video().src(videoB);

[rmhartog]

You're right, the current approach won't work in that scenario. I'll try to rewrite it by removing the getter and setter and reactively set the value on the video instead.

[aarthificial]

@rmhartog
If it helps, you can have a custom setter and getter while still storing the value in the signal like so:

class Example {
  @initial(1)
  @signal()
  public declare readonly playbackRate: SimpleSignal<number, this>;

  protected getPlaybackRate() {
    // invoke the native getter:
    return this.playbackRate.context.getter();
  }

  protected setPlaybackRate(playbackRate: number) {
    // invoke the native setter:
    this.playbackRate.context.setter(playbackRate);
  
    // do some other things:
    const video = this.video();
    video.playbackRate = playbackRate;

    if (playbackRate === 0) {
      this.pause();
    } else {
      const time = useThread().time;
      const start = time();
      const offset = this.time();
      this.time(() => this.clampTime(offset + (time() - start) * playbackRate));
    }
  }
}

[rmhartog]

Thanks @aarthificial, that was the missing piece! I've updated the code to imperatively update the time signal if the playbackRate changes and the video is playing, but the HTMLVideoElement.playbackRate is set reactively in the seekedVideo and fastSeekedVideo respectively.

[rmhartog]

Removed the superfluous getter method, think I've covered all comments @aarthificial!

[aarthificial]

One more thing, setPlaybackRate pauses the video when the value is 0 but this behavior is not reflected in fastSeekVideo so there's a possibility that the video won't get paused after the source is changed. It should be easy to fix by expanding this condition at line 208:

const playing = this.playing() && time < video.duration;

[rmhartog]

Updated based on your feedback, last main question for me is the remarks of the log message. Not sure if it is necessary and/or how detailed it should be.

[aarthificial]

I'd go for something like this:

The `playbackRate` of a `Video` cannot be reactive.

Make sure to use a concrete value and not a function:

```ts
// wrong ✗
video.playbackRate(() => 7);
// correct ✓
video.playbackRate(7);
```

If you're using a signal, extract its value before passing it to the property:

```ts
// wrong ✗
video.playbackRate(mySignal);
// correct ✓
video.playbackRate(mySignal());
```

[screaminc]

@rmhartog @aarthificial Is there any limits on playbackrate values, can we go more than 16x?