GhostNaN / mpvpaper

A video wallpaper program for wlroots based wayland compositors.
GNU General Public License v3.0
765 stars 24 forks source link

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

Closed easyteacher closed 2 years ago

easyteacher commented 2 years ago

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

See also: https://github.com/mpv-player/mpv/pull/9541

GhostNaN commented 2 years ago

Thanks for reporting this, but unfortunately this won't work for many reasons. As I will show: https://github.com/GhostNaN/mpvpaper/blob/d3d58fb8f28e15c41cb1ec54faad45b351526cf5/src/main.c#L375-L378 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.

https://github.com/GhostNaN/mpvpaper/blob/d3d58fb8f28e15c41cb1ec54faad45b351526cf5/src/main.c#L391-L392 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);.

https://github.com/GhostNaN/mpvpaper/blob/d3d58fb8f28e15c41cb1ec54faad45b351526cf5/src/main.c#L380 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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

Could you tell me if this helped: https://github.com/GhostNaN/mpvpaper/commit/50c0c8def6d96bb2f992fad6c77b86fa85e1a596

easyteacher commented 2 years ago

It works, thanks!