getTimePosition returns wrong value after calling goToPosition more than once

[NiaMori]

Bug Description

After calling goToPosition more than once, getTimePosition returns wrong value

How To Reproduce

await mpv.start()
await mpv.load('test.flv')
await mpv.goToPosition(10)
console.log(await mpv.getTimePosition())
await mpv.goToPosition(20)
// await new Promise((resolve) => setTimeout(resolve, 1000))
console.log(await mpv.getTimePosition())

The output is

10
10

Though the expected output is

10
20

If you call getTimeposition after a while, the output is roughly correct

10
20.433 (roughly)

Software Versions

• Node-Mpv: Node-MPV-2, commit b4e39d1 (latest when writing)
• MPV: mpv-x86_64-20200308-git-28ea1ed from SourceForge
• OS: Windows 10, x64, 10.0.18362

[j-holub]

Hey there, thanks for opening this issue and taking the time to fill out the template.

I just looked at the code and I'm pretty sure I know where the error is coming from, I will try to provide a fix asap.

Cheers

[j-holub]

After a little thinking I decided to push this commit, it fixes your issue, however it still has another issue.

If the seek() or goToPosition() method will go beyond the tracks duration, the promise will be resolved before the next track is started. That means that getTimePosition()will throw an error, because the property will not be available in that short moment, where no track is playing.

I thought a lot about it and unfortunately, it's very complicated to try to check for every edge case. I could check myself if, what is entered as seconds will skip the track, and skip it myself using this.next() but then no seek event will be fired. I could try to check for all possible combinations of events happening, but that makes it really complex and prone to a lot of errors.

I also thought about having the two methods return a bool to mark, if the track was skipped or not by the entered value.

I still have to think about it, by any means, feel free to reopen this issue, if your problem still remains :) it helps a lot.

[NiaMori]

Thanks for your timely response.

It now works fine in the example. I will check edge cases in my code so the issue you mentioned shouldn't be a problem for me.

Yet another issue arises. When a user is dragging the progress bar, goToPosition() will be called frequently. It seems it takes a while for goToPosition() to resolve because it needs to wait for data to arrive. If I call goToPosition() sequentially (something likes below), it will cause lags.

// Just use a loop to represent frequent calls caused by user's dragging action
for (let i = 1; i <= 10; i++) {
    await mpv.goToPosition(i)
}

console.log(await mpv.getTimePosition())

So I call them parallelly like this

let seekingPromises = []

for (let i = 1; i <= 10; i++) {
    seekingPromises.push(mpv.goToPosition(i))
}

await Promise.all(seekingPromises)
console.log(await mpv.getTimePosition())

My problem is, what mpv/node-mpv's behavior is when it receives another goToPosition()call while previous one is pending. Will they finally resolve in proper order?

[j-holub]

My problem is, what mpv/node-mpv's behavior is when it receives another goToPosition()call while previous one is pending. Will they finally resolve in proper order?

That's a good question, to be honest I don't know. Node-MPV is just a wrapper around MPV's json ipc socket API. Since goToPosition is an absolute call, meaning that every subsequent call of goToPosition(100) will point to the 100 second mark, it should be fine.

Have you tried it? What does happen in your case?

I don't think there's a guarantee, that they will resolve in proper order to be honest, since they are executed in parallel.

[NiaMori]

It just works fine at least from the user’s point of view. Though the resolving order is indeed chaotic. I think I should ignore the problem temporarily since it seems harmless.

To sum up, my solution is to use goToPosition() sequentially in case of low frequency to guaratee correctness. In case of high frequency, call it parallelly but wait all seeking promises to resolve before asking node-mpv for time.

[AxelTerizaki]

I use goToPosition() on my app as well and never noted this issue, actually.

We also have some kind of progressbar in place, and you can click that progress bar to go back or forward in the video.

[NiaMori]

@AxelTerizaki You're really active in this project :) I'm writing a video player capable of synchronizing multiple users based on electron and mpv. Your code actually helps me a lot.

[AxelTerizaki]

Is that so? Which part of it did you use? :)

I'm just trying to help because I fiddled with it a lot and it's a central part of Karaoke Mugen so it's normal I help out Jan even if a little.

[NiaMori]

@AxelTerizaki Sorry for the late reply. I'm just a JS beginner so I learn many basic things from your code. For example, how to use async function and the way work with node-mpv.