Video playback issues

[mrEuler]

Description

Just in case moving my comments from #1326
I've tried already video feature today, looks good! Only few things find a bit off:

1. some videos are rotated incorrectly (portrait becomes landscape)
2. incorrectly FPS detection (60 fps video is 2x time slower, than it should)
3. I changed useVideo hook to get the "video" variable outside of that hook, called .seek(t:ms) function with custom scrubber -- only few frames were changed in 10 seconds video. I've also put 'true' value in if statement, where video.nextImage(); is executed. Also quick check, if I make with ffmpeg video all frames being keyframes -- I can scrub back and forward.
   BUT!: After draging here and there app can rendomly crash (android logs point on skia)

Anyway waiting for the new version! If scrubbing on Android will work, this going to be killer feature to use this, instead of RNVideo!

Thanks!

Version

1.3.0

Steps to reproduce

Some issues are random, but everything is in the description

Snack, code example, screenshot, or link to a repository

Standard example from website can show this issues.

[mrEuler]

Maybe something like this can be achieved using some flag, when needed?
google/ExoPlayer#2191 (comment)

[mrEuler]

@wcandillon ^^

[wcandillon]

I've fixed the framerate issue, now looking into exposing/testing the seek command.

About the rotation, do you believe that there is some metadata that should be read and isn't? could you provide a sample video file to test?

[wcandillon]

I can confirm that seek seems to work as expected, but please let me know if you have more test cases for me. I'm also interested about the orientation issue.

[mrEuler]

@wcandillon Thanks!
Is there a possibility to have currentTime property as a sharedValue?
For the rotation will send you today.
For the seeking -- RNVideo is able to extract the preview every second of the video, with Skia I was able to do that only every 5~ seconds. Will send you that as well. I was able to test only on android.
So do you think there is ability to have scrubbing feature with seeking (currentTime value change) for every frame, if video has iframe every second (every 30th frame for example)? Like in the link I shared above?

[wcandillon]

@mrEuler would you like us to add a currentTime option where we would write the current time of the video in ms?

The scrubbing API is suggested at #2448, seems to work well on iOS and Android.

[mrEuler]

@wcandillon I think usage of currentTime is just common way how to control videos across different (JS) platforms. Checking the PR also.
Again many thanks!

[wcandillon]

just to be sure is currentTime just to check the time but not to control the scrubbing right?

[mrEuler]

@wcandillon I would go with both getter and setter there. So to have it working with gesture handlers under worklets with useSharedValue.

[wcandillon]

ok 👍

…

On Tue, May 28, 2024 at 3:05 PM OleksiiMaksymov ***@***.***> wrote: @wcandillon I would go with both getter and setter there. So to have it working with gesture handlers under worklets with useSharedValue. — Reply to this email directly, view it on GitHub or unsubscribe. You are receiving this email because you were mentioned. Triage notifications on the go with GitHub Mobile for iOS or Android.

[github-actions]

🎉 This issue has been resolved in version 1.3.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[mrEuler]

@wcandillon Update to 1.3.1

1. Upd: my bad here

2. Here is the video captured on regular android phone (it's rotated in skia):
   https://drive.google.com/file/d/1x8HinlYotSnc-nqa91i3jo5JbNbbv352/view?usp=sharing

3. Also tried code for seeking like this:

        <Pressable
            style={{ flex: 1 }}
            onPress={() => (seek.value = currentTime.value + 200)}
        >
            <Canvas style={{ flex: 1, width: width }}>
                <Fill>
                    <ImageShader
                        image={frame}
                        x={0}
                        y={0}
                        width={width}
                        height={height}
                        fit="contain"
                    />
                </Fill>
            </Canvas>
        </Pressable>

Seeking doesn't work as it should (in 90% of the time video is seeked to previous (not next) iframe: here is the video recording:

video_2024-05-28_19-40-44.mp4

Also checked, the currentTime value is reset to 0 almost every time, even though video is somewhere in the middle of playback duration

4. I am not sure, but on recording you may see, that video isn't played at 60 fps..

[mrEuler]

Regarding current time here is the issue:

    if (seek.value !== null) {
      video.seek(seek.value);
      seek.value = null;
      lastTimestamp.value = -1;
      startTimestamp.value = -1; // <---- you shouldn't reset the start time
    }
    if (isPaused.value && lastTimestamp.value !== -1) {
      return;
    }
    const { timestamp } = frameInfo;

    // Initialize start timestamp
    if (startTimestamp.value === -1) {
      startTimestamp.value = timestamp;
    }

    // Calculate the current time in the video
    const currentTimestamp = timestamp - startTimestamp.value; // <-- because here after the reset the current time is 0 and it starts the "count" from the seeked moment.
    currentTime.value = currentTimestamp;

So IMHO we just need to change it to:

      const seekValue = seek.value;
      video.seek(seek.value);
      seek.value = null;
      lastTimestamp.value = -1;
      startTimestamp.value = startTimestamp.value + (currentTime.value - seekValue);

But also I would get the real currentTime value from native, because there could be a difference because of native selecting iframe (mp4 keyframe) instead of seeked time.

@wcandillon 👀

[mrEuler]

@wcandillon also I am checking this code:

    @DoNotStrip
    public void seek(long timestamp) {
        // Seek to the closest sync frame at or before the specified time
        extractor.seekTo(timestamp * 1000, MediaExtractor.SEEK_TO_PREVIOUS_SYNC);
        // Flush the codec to reset internal state and buffers
        if (decoder != null) {
            decoder.flush();
        }
    }

Maybe here we should use SEEK_TO_PREVIOUS_SYNC only when the timestamp is smaller value, than a current timestamp, but if larger use SEEK_TO_NEXT_SYNC.
I will try this locally.

[mrEuler]

@wcandillon Also one more thing regarding rotated videos. When I use file picker to get the video from gallery (I use import { launchImageLibrary } from 'react-native-image-picker';) -- the dimmensions are not swapped, so for this issue I was able to make workaround (I hope app will never get rect video).

PS: Thanks for reopening this!

[github-actions]

🎉 This issue has been resolved in version 1.3.2 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀