Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libmpv is very hard to work with #15253

Open
Gieted opened this issue Nov 3, 2024 · 25 comments
Open

libmpv is very hard to work with #15253

Gieted opened this issue Nov 3, 2024 · 25 comments

Comments

@Gieted
Copy link

Gieted commented Nov 3, 2024

Expected behavior of the wanted feature

Hi, I'm currently developing a player frontend using libmpv, and I'm facing a lot of problems, because of how the api is designed, particularly that the synchronous calls (even simple property read and writes) can block thread for arbitrary amount of time (in practice can be up to 500ms).

One example is that when you implement a seeable progress bar, you cannot just call mpv_command(['seek', ...]) in the UI thread, because this call doesn't always complete immediately, so instead you have to do mpv_command_async(.... But because the UI progress bar position needs to update instantly after being pressed (for the UI to feel responsive) and because mpv_command_async doesn't update the property instantly, I have to create my own copy of player state (which I can update without any delay) and try to mirror the actual player state via MPV_EVENT_PROPERTY_CHANGE. This even on its own is already bad, because it makes the program more fragile, because the 2 states can get out of sync if something unexpected happens.

But even worse part is that there is no easy way to know whenever a particular event was produced before or after a call to mpv_command_async, this creaes teribble race conditions, e.g.:

  1. User presses the progress bar, the position (of my copy of the state) is now 10:00:00, mpv_command_async(['seek', ...] is called
  2. MPV_EVENT_PROPERTY_CHANGE event is processed, which in fact took place before the seek started, so the position reverts back to where it was before the seek, e.g. 12:00:12, which is incorrect behaviour
  3. Only now there is another MPV_EVENT_PROPERTY_CHANGE with value of 10:00:00

I know there are some workarounds, that can be done, to mitigate these issues, however they too cause risk of incorrect behaviour in case the state would get corrupted for some reason and introduce unncecesary complexity.

I think the DOM video API is a good example of how this should work, if you call video.currentTime = 12371 the state is updated to the new value instantly (even if the player hasn't actually completed the seek yet), if one needs to know when the seek actually finishes then he can listen to seeked event. But more importantly, no timeupdate event will fire for playback progression that happened before this call was made.

So my suggestion would be, to make mpv_get_property(), mpv_set_property() and the majority of commands (like seek) complete immediately and just signal if needed a more detailed completion timing through events.

Alternative behavior of the wanted feature

No response

Log File

No response

Sample Files

No response

@Gieted
Copy link
Author

Gieted commented Nov 3, 2024

So TLDR the problem is:

  1. Property read/update is not an immediate action and it makes everything 1000x harder.
  2. Seemingly the only solution to this issue is to create your own copy of the player state, which can get out-of-sync with one owned by MPV.
  3. There is no way to know whenever an event happened before or after a call to mpv_command_async, which makes it extra hard to maintain your copy of the player state consistent.

@sfan5
Copy link
Member

sfan5 commented Nov 3, 2024

[...], particularly that the synchronous calls (even simple property read and writes) can block thread for arbitrary amount of time (in practice can be up to 500ms).

One example is that when you implement a seeable progress bar, you cannot just call mpv_command(['seek', ...]) in the UI thread, because this call doesn't always complete immediately, [...]

This is not a problem in my experience, mpv-android has been calling mpv_command from the UI thread for years and it is working fine.
500ms sounds more like a bug. Can you show your code?

@kasper93
Copy link
Contributor

kasper93 commented Nov 3, 2024

One example is that when you implement a seeable progress bar, you cannot just call mpv_command(['seek', ...]) in the UI thread

This is unrelated to mpv. You should never call any external API/RPC that may take an unknown amount of time on the UI thread. Create your own dispatch thread, add a message queue, whatever works, as this has been a solved problem for years. Never stall the UI thread. It is not libmpv's job to handle this for you; it is your application's job to manage messages and their dispatch in a way that avoids issues. For example, you should never perform any I/O on the UI thread.

Property read/update is not an immediate action and it makes everything 1000x harder.

It is your application's responsibility to handle it correctly.

Seemingly the only solution to this issue is to create your own copy of the player state, which can get out-of-sync with one owned by MPV.

I don't know what this means, but it sounds scary and not at all like something you'd want to do.

There is no way to know whenever an event happened before or after a call to mpv_command_async, which makes it extra hard to maintain your copy of the player state consistent.

Each call to mpv_command_async triggers an MPV_EVENT_COMMAND_REPLY event in the mpv event loop, which you can treat as a callback. The command reply will be sent after your command is executed. You can use reply_userdata to perform any additional bookkeeping for your commands.

I think the DOM video API is a good example of how this should work

You are free to design and implement an API wrapper that will work better for your use case/application. libmpv is a C API, and many things are more complex than what you would expect from high-level bindings like JavaScript or Lua. For example, mpv_node can be annoying to work with in C, but it is a low-level primitive that you should adopt or wrap for your language of choice.

I can't speak for everyone, but I believe there are no current plans to change the official libmpv API. And for what it's worth, the C interface should remain as is; it has worked with multiple projects for years. You are free to use any existing wrapper or bindings, or to develop your own that makes interfacing with libmpv easier. I would do that too as a first step in implementing a libmpv-based application.

500ms sounds more like a bug. Can you show your code?

I agree with @sfan5 here. It doesn't sound like the delay is coming from mpv itself, to be quite honest. We have a g-r binding that lists all properties available in mpv, and this doesn't take long. Performing a single command shouldn't take that long either. In fact, all commands that take a long time, like the screenshot command, are forcefully asynchronous. If you see any commands that are stalling for 500 ms, please share more details so we can fix that. Thanks.

@wbtcpip2
Copy link

wbtcpip2 commented Nov 3, 2024

in my app i use mpv_set_property(mpvhandle, "time-pos", MPV_FORMAT_DOUBLE, wantedposition) and it looks fast. only occasionally it's slow when hardware decoding is enabled and the gpu performances are poor.
not sure if it's correct to use "time-pos" but it's working fine for me.

@Gieted
Copy link
Author

Gieted commented Nov 3, 2024

It's great that we all agree that a 500ms for a property set is not a normal thing, but to be honest, I'm kind of surprised that you don't have similar problems in your use. This is time measurement for mpv_command calls executed one right after another in my program. Only the first one completes in a reasonable time, the second one is already 9ms (probably because there is some locking mechanism that blocks command completion until previous seek finishes or something).

mpv_command [seek, 27.9370, absolute, keyframes] 0ms
mpv_command [seek, 27.9370, absolute, keyframes] 9ms
mpv_command [seek, 28.0390, absolute, keyframes] 212ms
mpv_command [seek, 28.1420, absolute, keyframes] 97ms
mpv_command [seek, 28.2440, absolute, keyframes] 213ms
mpv_command [seek, 28.3460, absolute, keyframes] 99ms
mpv_command [seek, 28.4480, absolute, keyframes] 410ms
mpv_command [seek, 28.7540, absolute, keyframes] 97ms
mpv_command [seek, 28.9590, absolute, keyframes] 212ms
mpv_command [seek, 29.1630, absolute, keyframes] 98ms
mpv_command [seek, 29.7760, absolute, keyframes] 211ms
mpv_command [seek, 30.0820, absolute, keyframes] 107ms
mpv_command [seek, 30.4910, absolute, keyframes] 407ms
mpv_command [seek, 30.8990, absolute, keyframes] 101ms
mpv_command [seek, 31.8180, absolute, keyframes] 408ms
mpv_command [seek, 32.2270, absolute, keyframes] 103ms
mpv_command [seek, 33.0440, absolute, keyframes] 408ms
mpv_command [seek, 33.4520, absolute, keyframes] 101ms
mpv_command [seek, 33.7590, absolute, keyframes] 411ms
mpv_command [seek, 34.1670, absolute, keyframes] 110ms
mpv_command [seek, 34.3720, absolute, keyframes] 400ms
mpv_command [seek, 34.8820, absolute, keyframes] 98ms
mpv_command [seek, 35.1890, absolute, keyframes] 213ms
mpv_command [seek, 35.3930, absolute, keyframes] 98ms
mpv_command [seek, 35.4950, absolute, keyframes] 411ms
mpv_command [seek, 35.8010, absolute, keyframes] 101ms
mpv_command [seek, 36.0060, absolute, keyframes] 410ms
mpv_command [seek, 36.1080, absolute, keyframes] 98ms

@kasper93
Copy link
Contributor

kasper93 commented Nov 3, 2024

If you don't want to wait for seek to complete synchronously, you should use mpv_command_async. Seeking involves setting demuxer state to new position, it is expected to take non-zero time. Depending on the hardware and decoding it may take significant time.

@Gieted
Copy link
Author

Gieted commented Nov 3, 2024

But why it's the case? Why cannot this command just set some variable, notify the video engine, and complete immediately?
obraz.

Why does "queuing" seek involve waiting for all other "work items" to finish?

@kasper93
Copy link
Contributor

kasper93 commented Nov 3, 2024

Why cannot this command just set some variable, notify the video engine, and complete immediately?

This is what mpv_command_async does, no?

@Gieted
Copy link
Author

Gieted commented Nov 3, 2024

No, mpv_command_async doesn't set new time-pos immediately.

@kasper93
Copy link
Contributor

kasper93 commented Nov 3, 2024

No, mpv_command_async doesn't set new time-pos immediately.

Yes, because seeking takes time. time-pos reflect current player state.

@Gieted
Copy link
Author

Gieted commented Nov 4, 2024

time-pos is actually a writable property, if you write it directly using mpv_set_property, then it's updated immediately. So why seek (which conceptually is the same thing, just with extra options) does behave differently and doesn't set time-pos instantly?

@kasper93
Copy link
Contributor

kasper93 commented Nov 4, 2024

time-pos is actually a writable property, if you write it directly using mpv_set_property, then it's updated immediately.

What is updated? Settings time-pos property queues seek operation. If you get_property before this seek ends it will return old time-pos value. Exactly the same as with mpv_command_async which directly does the seek. Commands are not properties, there is difference between setting property and running command. But that's not really important here.

@sfan5
Copy link
Member

sfan5 commented Nov 4, 2024

I don't know what this means, but it sounds scary and not at all like something you'd want to do.

It's not ideal but I do this in mpv-android too: https://github.com/mpv-android/mpv-android/blob/045b7f176858ac6ca3c40c318c3016a1084d80e2/app/src/main/java/is/xyz/mpv/Utils.kt#L247-L320

for these reasons:

  • I need it all at once to update the media session on the android side
  • many parts of the player need to know the same things. it would be inefficient to read the properties 10 times.
  • properties can become unavailable and this is annoying to handle in UI code. you'd rather have an outdated value than no value.

@kasper93
Copy link
Contributor

kasper93 commented Nov 4, 2024

I don't know what this means, but it sounds scary and not at all like something you'd want to do.

It's not ideal but I do this in mpv-android too: https://github.com/mpv-android/mpv-android/blob/045b7f176858ac6ca3c40c318c3016a1084d80e2/app/src/main/java/is/xyz/mpv/Utils.kt#L247-L320

for these reasons:

* I need it all at once to update the media session on the android side

* many parts of the player need to know the same things. it would be inefficient to read the properties 10 times.

* properties can become unavailable and this is annoying to handle in UI code. you'd rather have an outdated value than no value.

Oh, my bad. I think I misread the "copy of player state" part. I though it is somehow clone of mpv. If it it like in your case some cache of current player state, it is good.

I would call it "observe and cache" approach and this is common in .lua scripts also. To observe properties and cache its state to have easier access to this data and be able to aggregate it to call downstream APIs.

many parts of the player need to know the same things. it would be inefficient to read the properties 10 times.

I agree, if it is possible and easier to cache observed values it should be better this way. Although depending on the amount of properties involved it is also possible to get_properties(), each time we need them. Some props are very light to query. But like said "observe and cache" is good way to work with it too.

properties can become unavailable and this is annoying to handle in UI code. you'd rather have an outdated value than no value.

Yeah, I guess it depends, on the UI design and how to handle dynamic player state changes.

@Gieted
Copy link
Author

Gieted commented Nov 5, 2024

If you get_property before this seek ends it will return old time-pos value.

You're right, I wasn't aware that this works this way.


It's not ideal but I do this in mpv-android too

Great to hear that we're in fact on the same page here. May I ask how you handle that particular case where you do execute an async seek command, and before it finishes a time-pos MPV_EVENT_PROPERTY_CHANGE event (read by mpv_wait_event()) pops out, with a position recorded before the seek was made? How do you avoid it overriding your state with an outdated value? The only solution I can think of is to have some flag which disables event processing until the async command is finished, but it sounds too damn fragile to be the only solution.


I would call it "observe and cache" approach

I would say that it's a reasonable approach when you only read those "cached" values, but if you also write them to kind of guess what player state will be after the async command finishes, then it becomes more tricky, because if your predictions about future state aren't right, then your state can get out-of-sync with the real one. One solution would be to read the properties again when the async command finishes, but that's a lot of things to think about for such a simple task.

@sfan5
Copy link
Member

sfan5 commented Nov 5, 2024

How do you avoid it overriding your state with an outdated value?

While the user is operating the seek bar (holding down his finger) I simply block changes to the bar position. It could theoretically jump back after the touch is released but I haven't noticed that in practice.

I can't think of any other situations with the same problem.

but if you also write them to kind of guess what player state will be after the async command finishes, then it becomes more tricky

(I don't do that)

It's great that we all agree that a 500ms for a property set is not a normal thing, but to be honest, I'm kind of surprised that you don't have similar problems in your use.

As for this:

11-05 19:51:01.861 17878 17878 V mpv     : mpv_command(loadfile): 0ms
11-05 19:51:01.862 17878 17878 V mpv     : mpv_set_property(android-surface-size): 0ms
11-05 19:51:02.447 17878 17878 V mpv     : mpv_set_property(android-surface-size): 1ms
11-05 19:51:15.870 17878 17878 V mpv     : mpv_set_property(pause): 1ms
11-05 19:51:15.870 17878 17878 V mpv     : mpv_set_property(time-pos): 1ms
11-05 19:51:15.901 17878 17878 V mpv     : mpv_set_property(time-pos): 7ms
11-05 19:51:15.949 17878 17878 V mpv     : mpv_set_property(time-pos): 30ms
11-05 19:51:15.987 17878 17878 V mpv     : mpv_set_property(time-pos): 18ms
11-05 19:51:16.034 17878 17878 V mpv     : mpv_set_property(time-pos): 31ms
11-05 19:51:16.076 17878 17878 V mpv     : mpv_set_property(time-pos): 23ms
11-05 19:51:16.116 17878 17878 V mpv     : mpv_set_property(time-pos): 25ms
11-05 19:51:16.167 17878 17878 V mpv     : mpv_set_property(time-pos): 29ms
11-05 19:51:16.213 17878 17878 V mpv     : mpv_set_property(time-pos): 27ms
11-05 19:51:16.256 17878 17878 V mpv     : mpv_set_property(time-pos): 30ms
11-05 19:51:16.300 17878 17878 V mpv     : mpv_set_property(time-pos): 31ms
11-05 19:51:16.343 17878 17878 V mpv     : mpv_set_property(time-pos): 24ms
11-05 19:51:16.387 17878 17878 V mpv     : mpv_set_property(time-pos): 30ms
11-05 19:51:16.450 17878 17878 V mpv     : mpv_set_property(time-pos): 47ms
11-05 19:51:16.495 17878 17878 V mpv     : mpv_set_property(time-pos): 25ms
11-05 19:51:16.544 17878 17878 V mpv     : mpv_set_property(time-pos): 35ms
11-05 19:51:16.585 17878 17878 V mpv     : mpv_set_property(time-pos): 27ms
11-05 19:51:16.628 17878 17878 V mpv     : mpv_set_property(time-pos): 24ms
11-05 19:51:16.672 17878 17878 V mpv     : mpv_set_property(time-pos): 32ms
11-05 19:51:16.722 17878 17878 V mpv     : mpv_set_property(time-pos): 34ms
11-05 19:51:16.773 17878 17878 V mpv     : mpv_set_property(time-pos): 36ms
11-05 19:51:16.826 17878 17878 V mpv     : mpv_set_property(time-pos): 38ms
11-05 19:51:16.885 17878 17878 V mpv     : mpv_set_property(time-pos): 47ms
11-05 19:51:16.934 17878 17878 V mpv     : mpv_set_property(time-pos): 29ms
11-05 19:51:16.944 17878 17878 V mpv     : mpv_set_property(pause): 6ms
11-05 19:51:19.869 17878 17878 V mpv     : mpv_set_property(time-pos): 0ms
11-05 19:51:19.931 17878 17878 V mpv     : mpv_set_property(time-pos): 1ms
11-05 19:51:19.983 17878 17878 V mpv     : mpv_set_property(time-pos): 3ms
11-05 19:51:20.035 17878 17878 V mpv     : mpv_set_property(time-pos): 38ms
11-05 19:51:20.045 17878 17878 V mpv     : mpv_set_property(time-pos): 7ms
11-05 19:51:20.086 17878 17878 V mpv     : mpv_set_property(time-pos): 38ms
11-05 19:51:20.129 17878 17878 V mpv     : mpv_set_property(time-pos): 38ms
11-05 19:51:20.177 17878 17878 V mpv     : mpv_set_property(time-pos): 31ms
11-05 19:51:20.221 17878 17878 V mpv     : mpv_set_property(time-pos): 35ms
11-05 19:51:20.265 17878 17878 V mpv     : mpv_set_property(time-pos): 37ms
11-05 19:51:20.304 17878 17878 V mpv     : mpv_set_property(time-pos): 24ms
11-05 19:51:20.339 17878 17878 V mpv     : mpv_set_property(time-pos): 26ms
11-05 19:51:20.379 17878 17878 V mpv     : mpv_set_property(time-pos): 31ms
11-05 19:51:20.417 17878 17878 V mpv     : mpv_set_property(time-pos): 20ms
11-05 19:51:20.458 17878 17878 V mpv     : mpv_set_property(time-pos): 29ms
11-05 19:51:20.503 17878 17878 V mpv     : mpv_set_property(time-pos): 36ms
11-05 19:51:20.552 17878 17878 V mpv     : mpv_set_property(time-pos): 43ms
11-05 19:51:20.588 17878 17878 V mpv     : mpv_set_property(time-pos): 25ms
11-05 19:51:20.631 17878 17878 V mpv     : mpv_set_property(time-pos): 33ms
11-05 19:51:20.671 17878 17878 V mpv     : mpv_set_property(time-pos): 24ms
11-05 19:51:20.712 17878 17878 V mpv     : mpv_set_property(time-pos): 31ms
11-05 19:51:20.755 17878 17878 V mpv     : mpv_set_property(time-pos): 25ms
11-05 19:51:20.795 17878 17878 V mpv     : mpv_set_property(time-pos): 29ms
11-05 19:51:20.841 17878 17878 V mpv     : mpv_set_property(time-pos): 19ms
11-05 19:51:23.476 17878 17878 V mpv     : mpv_set_property(time-pos): 3ms
11-05 19:51:23.534 17878 17878 V mpv     : mpv_set_property(time-pos): 28ms
11-05 19:51:23.549 17878 17878 V mpv     : mpv_set_property(time-pos): 1ms
11-05 19:51:23.586 17878 17878 V mpv     : mpv_set_property(time-pos): 29ms
11-05 19:51:23.630 17878 17878 V mpv     : mpv_set_property(time-pos): 37ms
11-05 19:51:23.669 17878 17878 V mpv     : mpv_set_property(time-pos): 28ms
11-05 19:51:23.710 17878 17878 V mpv     : mpv_set_property(time-pos): 32ms
11-05 19:51:23.752 17878 17878 V mpv     : mpv_set_property(time-pos): 37ms
11-05 19:51:23.802 17878 17878 V mpv     : mpv_set_property(time-pos): 37ms
11-05 19:51:23.849 17878 17878 V mpv     : mpv_set_property(time-pos): 35ms
11-05 19:51:23.893 17878 17878 V mpv     : mpv_set_property(time-pos): 33ms
11-05 19:51:23.944 17878 17878 V mpv     : mpv_set_property(time-pos): 39ms
11-05 19:51:23.996 17878 17878 V mpv     : mpv_set_property(time-pos): 41ms
11-05 19:51:24.042 17878 17878 V mpv     : mpv_set_property(time-pos): 35ms
11-05 19:51:24.086 17878 17878 V mpv     : mpv_set_property(time-pos): 36ms
11-05 19:51:26.533 17878 17878 V mpv     : mpv_set_property(time-pos): 1ms

(hardware decoding, local file playback)

¯\_(ツ)_/¯

@Gieted
Copy link
Author

Gieted commented Nov 10, 2024

It could theoretically jump back after the touch is released but I haven't noticed that in practice.

Yeah, that seems to be ok for the progress bar case, but unfortunately my use case is far more complicated than this. The bottom line is, that I need a variable that stores time-pos and can be updated immediately when seek happens, and is not overwritten by any events until the seek is finished (just like DOM's video.currentTime).

I've managed to eliminate the outdated events case by setting a flag to ignore time-pos property change events until MPV_EVENT_COMMAND_REPLY is received, but there is another problem. What sometimes happens (about 10% of cases) is that, the time-pos property changes twice (probably because seek target isn't a keyframe), e.g.:

[DEBUG] mpv_command_async(user_data: 1731671481,  ["seek", "0:05:27.966215", "absolute", "keyframes"])
[DEBUG] MPV_EVENT_COMMAND_REPLY(user_data: 1731671481)
[DEBUG] MPV_EVENT_PROPERTY_CHANGE(time-pos: 0:05:27.966000)
[DEBUG] MPV_EVENT_PROPERTY_CHANGE(time-pos: 0:05:26.250000)

This makes it impossible to know whenever a given time-pos is a final resulting position after the seek or not. Looking at the API reference I cannot find anything that would allow circumventing that.


What I would rather expect is that the time-pos should already be updated with the final position when the seek command finishes, so the order should be:

[DEBUG] mpv_command_async(user_data: 1731671481,  ["seek", "0:05:27.966215", "absolute", "keyframes"])
[DEBUG] MPV_EVENT_PROPERTY_CHANGE(time-pos: 0:05:27.966000)
[DEBUG] MPV_EVENT_PROPERTY_CHANGE(time-pos: 0:05:26.250000)
[DEBUG] MPV_EVENT_COMMAND_REPLY(user_data: 1731671481)

@sfan5
Copy link
Member

sfan5 commented Nov 10, 2024

The bottom line is, that I need a variable that stores time-pos and can be updated immediately when seek happens

And why do you need that?

@llyyr
Copy link
Contributor

llyyr commented Nov 10, 2024

just like DOM's video.currentTime

What does video.currentTime do when you try to seek to a timestamp but there's no keyframe at that timestamp, so the seek is at an earlier or a later timestamp?

@Gieted
Copy link
Author

Gieted commented Nov 10, 2024

What does video.currentTime do when you try to seek to a timestamp but there's no keyframe at that timestamp, so the seek is at an earlier or a later timestamp?

If I remember correctly, the currentTime will stay at whatever value you set (even though the playback've snapped to the nearest keyframe, so theoretically it's slightly incorrect). Maybe it's not perfect, but at least it's manageable.


And why do you need that?

My player has a built-in watch together style remote playback synchronization feature. One of the things it does, is continuously exchanging a ReportPosition packets between the users. Which is as follows:

struct ReportPosition {
  Duration position;
  int seekCounter;
  DateTime timestamp;
  Ulid? userId;
}

the seekCounter is incremented each time a seek happens, this allows detecting whenever a given user have already acknowledged the latest seek when he produced this packet or not. If a large time difference is detected, despite having the same seekCounter value, the playback rewinds to the smallest position with the biggest (latest) seekCounter. In order for this to function correctly I must make sure, that position and seekCounter are always in sync (e.g. it's incorrect if seekCounter is already incremented, but position is still from before the seek). And that's why I need to know to which seek a given time-pos belongs (and also if it's already the keyframe aligned one or not).

@llyyr
Copy link
Contributor

llyyr commented Nov 10, 2024

If I remember correctly, the currentTime will stay at whatever value you set (even though the playback've snapped to the nearest keyframe, so theoretically it's slightly incorrect). Maybe it's not perfect, but at least it's manageable.

Then just setting the UI element to the timestamp sent with the seek command, and updating it after the seek went through should achieve this no?

@Gieted
Copy link
Author

Gieted commented Nov 10, 2024

Then just setting the UI element to the timestamp sent with the seek command, and updating it after the seek went through should achieve this no?

But the timeline of events looks something like (things on the same line happen in parallel):

User1                                                     |  User2
------------------------------------------------------------------------------------
seekCounter = 1                                           | seekCounter = 1
                                                          |
*seeks to 00:05:25*                                       |
mpv_command_async([seek, 00:05:25, absolute, keyframes])  |
position = 00:05:25                                       |
isSeekReplyRecieved = false                               |
isSeeking = true                                          |
seekCounter  = 2                                          |
SeekPacket(newPosition: 00:05:25, seekCounter: 2) ->      | *receives seek request*
                                                          | mpv_command_async([seek, 00:05:25, absolute, keyframes])
MPV_EVENT_COMMAND_REPLY                                   | position = 00:05:25
isSeekReplyRecieved = true                                | isSeeking = true // position reporting is disabled while seeking to avoid reporting incorrect position
                                                          | seekCounter  = 2  
MPV_EVENT_PROPERTY_CHANGE(time-pos: 00:05:25)             | SeekPacket(newPosition: 00:05:25, seekCounter: 2)
position = 00:05:25                                       | 
isSeeking = false                                         | *seek will be considered finished, when a new time-pos is emitted*
                                                          | MPV_EVENT_COMMAND_REPLY
*this user has a faster internet,                         | isSeekReplyRecieved = true
 he receives keyframe aligned time-pos first*             |
MPV_EVENT_PROPERTY_CHANGE(time-pos: 00:04:12)             | MPV_EVENT_PROPERTY_CHANGE(time-pos: 00:05:25)
position = 00:04:12                                       | position = 00:05:25
                                                          | isSeeking = false // I assume seeking is now fully finished and position is correct, position reporting is resumed. I have no way of knowing whenever this position is keyframe aligned or not.
                                                          |
                                                          | *ReportPosition is sent in regular intervals, and it happened that it was sent before the position was updated to the accurate keyframe (a race condition)*
*because User1 got the keyframe aligned position first    | <- ReportPosition(position: 00:05:25, seekCounter: 2)
a high difference is incorrectly detected*                |
                                                          | *and only now we receive keyframe-aligned position, but it's already too late*
                                                          | MPV_EVENT_PROPERTY_CHANGE(time-pos: 00:04:12)
                                                          | position = 00:04:12    

@Gieted
Copy link
Author

Gieted commented Nov 10, 2024

[DEBUG] mpv_command_async(user_data: 1731671481, ["seek", "0:05:27.966215", "absolute", "keyframes"])
[DEBUG] MPV_EVENT_COMMAND_REPLY(user_data: 1731671481)
[DEBUG] MPV_EVENT_PROPERTY_CHANGE(time-pos: 0:05:27.966000)
[DEBUG] MPV_EVENT_PROPERTY_CHANGE(time-pos: 0:05:26.250000)

And I've just noticed that this order appears to actually be random, sometimes the property is updated before the command finishes, so there is literally no way to know if it's even the correct one.

@llyyr
Copy link
Contributor

llyyr commented Nov 10, 2024

But the timeline of events looks something like (things on the same line happen in parallel):

Scripts trying to synchronize multiple players over network have a master player which is considered the source of truth for the play state for a reason. If you're going to let any player trigger a seek, then I don't believe libmpv is going to offer the kind of control you want. There could be small differences between the players that can't be reconciled, depending on versions of dependencies like ffmpeg for example. A libmpv player with a ffmpeg 6.1 for example is going to seek to different frames on a HEVC mov/mp4 file than a libmpv with ffmpeg 7.1 will; this is just one example that I know of I'm sure there are others.

There are many issues with what you're trying to do, but if you want to attempt to do it anyway then I think you'll be better served using ffmpeg directly since you need far more control over the player than libmpv can offer at this time.

@Gieted
Copy link
Author

Gieted commented Nov 10, 2024

No, but I've had a working web version for years, and it was working flawlessly. I ship the program with all dependencies bundled, so version differences shouldn't matter, and also I've put multiple mechanisms in place to make sure that even if something wrong happens, players will still be able to resync. The only thing I need is somewhat accurate information about current position and which seek was already applied when this position was produced. Unfortunately I cannot find a reliable way to do that in libmpv, the seek command completion seems to be completely detached from time-pos property update. Sometimes the property is updated first, and then the event completes, and sometimes the event completes first. This makes this API very hard to use for anything more than a simple GUI integration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants