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

Port away from deprecated/removed APIs in mpv 2.0 #19

Closed

Conversation

easyteacher
Copy link

Register observers as MPV_EVENT_IDLE is deprecated and MPV_EVENT_PAUSE/MPV_EVENT_UNPAUSE have been removed.

See also: mpv-player/mpv#9541

Register observers as MPV_EVENT_IDLE is deprecated and
MPV_EVENT_PAUSE/MPV_EVENT_UNPAUSE have been removed.

See also: mpv-player/mpv#9541
@GhostNaN
Copy link
Owner

Thanks for reporting this, but unfortunately this won't work for many reasons. As I will show:

mpvpaper/src/main.c

Lines 375 to 378 in d3d58fb

case MPV_OBSERVE_IDLE: {
exit_mpvpaper(0);
break;
}

At least on my PC, when mpvpaper first runs it thinks it's in a idle state at start. So it immediately quits because of this. (This behavior was not there before)
Even if try to get around that obstacle by clearing the event buffers it won't quit on idle.
Even MPV_EVENT_SHUTDOWN is acting a little weird now.

mpvpaper/src/main.c

Lines 391 to 392 in d3d58fb

if (!halt_info.is_paused && mpv_paused) {
mpv_command_async(mpv, 0, (const char*[]) {"set", "pause", "no", NULL});

This is also not where it should be, as this is how the other pthreads unpause mpv after pausing them for either a watch list or automatically. It should be just before pthread_usleep(10000);.

if (mpv_get_property(mpv, "paused", MPV_FORMAT_FLAG, &mpv_paused) < 0) {

Did you mean "pause" not "paused"? if (mpv_get_property(mpv, "pause", MPV_FORMAT_FLAG, &mpv_paused) < 0) {

Lastly a small nit pick, using a switch case for 2 cases is unneeded a "if" and "else if" is fine here.

The code so far:

static void *handle_mpv_events() {
    pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, NULL);
    bool mpv_paused = 0;
    time_t start_time = time(NULL);

    const int MPV_OBSERVE_IDLE = 1;
    const int MPV_OBSERVE_PAUSE = 2;
    mpv_observe_property(mpv, MPV_OBSERVE_IDLE, "idle-active", MPV_FORMAT_NONE);
    mpv_observe_property(mpv, MPV_OBSERVE_PAUSE, "pause", MPV_FORMAT_FLAG);

    // Clear all events
    mpv_event* event = mpv_wait_event(mpv, 1);
    while (event->event_id != MPV_EVENT_NONE){
        event = mpv_wait_event(mpv, 1);
    }

    while (!halt_info.kill_render_loop) {
        if (SLIDESHOW_TIME) {
            if ((time(NULL) - start_time) >= SLIDESHOW_TIME) {
                mpv_command_async(mpv, 0, (const char*[]) {"playlist-next", NULL});
                start_time = time(NULL);
            }
        }

        mpv_event* event = mpv_wait_event(mpv, 0);

        if (event->event_id == MPV_EVENT_SHUTDOWN)
            exit_mpvpaper(0);
        else if (event->event_id == MPV_EVENT_PROPERTY_CHANGE) {
            if (event->reply_userdata == MPV_OBSERVE_IDLE) {
                exit_mpvpaper(0);
            }
            if (event->reply_userdata == MPV_OBSERVE_PAUSE) {
                mpv_get_property(mpv, "pause", MPV_FORMAT_FLAG, &mpv_paused);
                if (mpv_paused) {
                    // User paused
                    if (!halt_info.is_paused)
                        halt_info.is_paused += 1;
                } else {
                    halt_info.is_paused = 0;
                }
            }
        }

        if (!halt_info.is_paused && mpv_paused) {
            mpv_command_async(mpv, 0, (const char*[]) {"set", "pause", "no", NULL});
        }

        pthread_usleep(10000);
    }

    mpv_unobserve_property(mpv, MPV_OBSERVE_IDLE);
    mpv_unobserve_property(mpv, MPV_OBSERVE_PAUSE);

    pthread_exit(NULL);
}

I threw MPV_OBSERVE_IDLE and MPV_OBSERVE_PAUSE in there as well, because they won't be used outside of this function.
The idea is sound, but getting to work 100% is another matter.
I don't know if it's my terrible code full of race conditions or mpv being finicky for some reason. Either way more needs to be done before this can be committed, either on my end or yours (or mpv?).

Sorry for the long comment, thanks again for bring this to my attention. I appreciate the attempt.

@easyteacher
Copy link
Author

Thank you for testing, because I always got core dumped when running mpvpaper on my laptop so I can't test if it still works. Hope to see your fix soon :D

@easyteacher
Copy link
Author

This is the backtrace:

#0  0x00007ffff7db03d0 in ra_gl_ctx_resize (sw=0x0, w=1920, h=1080, fbo=0) at ../video/out/opengl/context.c:184
#1  0x00007ffff7db369e in wrap_fbo (ctx=<optimized out>, params=<optimized out>, out=0x7fffffffd980) at ../video/out/opengl/libmpv_gl.c:86
#2  0x00007ffff7d8e21b in get_target_size (ctx=<optimized out>, params=<optimized out>, out_w=0x7fffffffd9cc, out_h=0x7fffffffd9c8) at ../video/out/gpu/libmpv_gpu.c:161
#3  0x00007ffff7dc6139 in mpv_render_context_render (ctx=0x67e760, params=params@entry=0x7fffffffdb20) at ../video/out/vo_libmpv.c:343
#4  0x000000000040456b in render (output=0x47bcf0) at ../src/main.c:144
#5  0x00007ffff7a44572 in ffi_call_unix64 () at ../src/x86/unix64.S:105
#6  0x00007ffff7a41296 in ffi_call_int (cif=<optimized out>, fn=<optimized out>, rvalue=<optimized out>, avalue=<optimized out>, closure=<optimized out>) at ../src/x86/ffi64.c:672
#7  0x00007ffff7f859c0 in wl_closure_invoke (closure=closure@entry=0x7a8cb0, target=<optimized out>, target@entry=0x7a6440, opcode=opcode@entry=0, data=<optimized out>, flags=<optimized out>) at ../src/connection.c:1025
#8  0x00007ffff7f86103 in dispatch_event (display=display@entry=0x476e00, queue=<optimized out>, queue=<optimized out>) at ../src/wayland-client.c:1583
#9  0x00007ffff7f862dc in dispatch_queue (queue=0x476ed0, display=0x476e00) at ../src/wayland-client.c:1729
#10 wl_display_dispatch_queue_pending (display=0x476e00, queue=0x476ed0) at ../src/wayland-client.c:1971
#11 0x00007ffff7f87bf3 in wl_display_dispatch_queue (queue=<optimized out>, display=<optimized out>) at ../src/wayland-client.c:1947
#12 0x00007ffff7f87c0c in wl_display_dispatch (display=<optimized out>) at ../src/wayland-client.c:2014
#13 0x0000000000403a7e in main (argc=<optimized out>, argv=<optimized out>) at ../src/main.c:1015

GhostNaN added a commit that referenced this pull request Feb 22, 2022
This commit mainly fixes how mpv events are processed.
See: #19

- Since some mpv event are deprecated, "mpv_observe_property"
is being used instead of MPV_EVENT_PAUSE and MPV_EVENT_UNPAUSE

- mpv's default "idle" option for libmpv is now always set to off/no
as it provided no use and can't be loaded before user options, sorry.

- Pausing functions should now behave a little better with less race conditions.

- Cleaning up mpvpaper has been improved by also fixing some race conditions.

- Turned some mpv_command_async calls into mpv_command as it was not necessary.

- Some error messages have been modified/created

- Stopped creating a pthread for a watch list if the file is empty

- Add signal handlers for SIGQUIT and SIGTERM to exit mpvpaper
@GhostNaN
Copy link
Owner

Could you tell me if this helped:
50c0c8d

@easyteacher
Copy link
Author

It works, thanks!

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

Successfully merging this pull request may close these issues.

2 participants