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

Playback doesn't stop immediately in libmpv #4152

Closed
Kagami opened this Issue Feb 15, 2017 · 7 comments

Comments

Projects
None yet
2 participants
@Kagami
Contributor

Kagami commented Feb 15, 2017

mpv version and platform

Linux x86-64, git master.

Reproduction steps

Diff from SDL example:

diff --git a/libmpv/sdl/main.c b/libmpv/sdl/main.c
index b3b5952..dabebcb 100644
--- a/libmpv/sdl/main.c
+++ b/libmpv/sdl/main.c
@@ -95,6 +95,9 @@ int main(int argc, char *argv[])
     //  users which run OpenGL on a different thread.)
     mpv_opengl_cb_set_update_callback(mpv_gl, on_mpv_redraw, NULL);
 
+    mpv_observe_property(mpv, 0, "pause", MPV_FORMAT_FLAG);
+    mpv_observe_property(mpv, 0, "time-pos", MPV_FORMAT_DOUBLE);
+
     // Play this file. Note that this starts playback asynchronously.
     const char *cmd[] = {"loadfile", argv[1], NULL};
     mpv_command(mpv, cmd);
@@ -127,7 +130,16 @@ int main(int argc, char *argv[])
                     mpv_event *mp_event = mpv_wait_event(mpv, 0);
                     if (mp_event->event_id == MPV_EVENT_NONE)
                         break;
-                    printf("event: %s\n", mpv_event_name(mp_event->event_id));
+                    if (mp_event->event_id == MPV_EVENT_PROPERTY_CHANGE) {
+                        mpv_event_property *prop = mp_event->data;
+                        if (prop->format == MPV_FORMAT_FLAG) {
+                            int value = *(int*)(prop->data);
+                            printf("PROP CHANGE: %s %d\n", prop->name, value);
+                        } else if (prop->format == MPV_FORMAT_DOUBLE) {
+                            double value = *(double*)(prop->data);
+                            printf("PROP CHANGE: %s %f\n", prop->name, value);
+                        }
+                    }
                 }
             }
         }

Open some file, let it play, press SPACE.

Expected behavior

Playback should stop right away, there shouldn't be additional time-pos events.

Actual behavior

mpv plays file for some time after you pressed the key which is highly annoying.

Log file

$ ./main video.mkv
PROP CHANGE: pause 0
PROP CHANGE: time-pos 0.080000
PROP CHANGE: time-pos 0.120000
PROP CHANGE: time-pos 0.160000
PROP CHANGE: time-pos 0.200000
PROP CHANGE: time-pos 0.240000
PROP CHANGE: time-pos 0.280000
PROP CHANGE: time-pos 0.320000
PROP CHANGE: time-pos 0.360000
PROP CHANGE: time-pos 0.400000
PROP CHANGE: time-pos 0.440000
PROP CHANGE: time-pos 0.480000
PROP CHANGE: time-pos 0.520000
PROP CHANGE: time-pos 0.560000
PROP CHANGE: pause 1
PROP CHANGE: time-pos 0.640000

Note that I can't reproduce this issue by replacing "cycle pause" with "keypress SPACE" (and mpv_set_option_string(mpv, "input-default-bindings", "yes")).

@wm4

This comment has been minimized.

Contributor

wm4 commented Feb 16, 2017

Isn't this the usual problem of property change notifications being in random order?

@Kagami

This comment has been minimized.

Contributor

Kagami commented Feb 16, 2017

The problem is that I can't pause playback immediately with cycle pause. keypress SPACE works as expected. Property changes log is just to illustrate the issue.

@wm4

This comment has been minimized.

Contributor

wm4 commented Feb 16, 2017

What makes you think playback doesn't pause immediately, and that it isn't the lazy change notification issue I mentioned?

@Kagami

This comment has been minimized.

Contributor

Kagami commented Feb 16, 2017

Because I can see it plays for some more time after SPACE press.

@wm4

This comment has been minimized.

Contributor

wm4 commented Feb 18, 2017

I can reproduce. Not quite sure why it's happening. It looks like after pausing, there's some delay, and then another frame is drawn. Could have to do something with missed wakeups or callback timing.

wm4 pushed a commit that referenced this issue Feb 21, 2017

wm4
player: reduce blocking on VO when switching pause
When pausing, we sent BOCTRL_PAUSE and VOCTRL_RESTORE_SCREENSAVER. These
essentially wait until the video frame has been rendered. This is a
problem with the opengl-cb, if GL rendering is done in the same thread
as libmpv uses. Unfortunately, it's allowed to use opengl-cb this way.

Logically speaking, it's a deadlock situation, which is resolved with a
timeout. This can lead to quite ugly effects, like the on-pause frame
not being rendered until the timeout has passed. It has been interpreted
as video continuing to play.

Resolve this by simply not blocking on pause. Make the screensaver
controls async, and handle sending VOCTRL_PAUSE in the VO thread.

(All this could be avoided by redoing the internal VO API.)

Also see #4152.
@wm4

This comment has been minimized.

Contributor

wm4 commented Feb 21, 2017

Is it better with git master now? Explanations see the commit linked above.

@Kagami

This comment has been minimized.

Contributor

Kagami commented Feb 21, 2017

Yes, it's fixed now. Thank you!

@Kagami Kagami closed this Feb 21, 2017

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