search

u8sand / Baka-MPlayer

The libmpv based media player
https://u8sand.github.io/Baka-MPlayer/
GNU General Public License v2.0
433 stars 93 forks source link

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

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

easyteacher commented 2 years ago

The play button doesn't work. If that's an issue caused by this patch please let me know, because the change looks pretty safe and it could be an existing problem before this patch.

u8sand commented 2 years ago

@easyteacher Thank you for your PR, it may take me a few days to get to it but it's on my radar. I'll have to test it out of course but am happy to accept it when I know it doesn't break anything obvious. I can also look into the play button issue.

u8sand commented 2 years ago

@easyteacher Unfortunately it seems your changes break the play button but also playing/pausing on my system. I'll try to spend some time tomorrow reviewing mpv's changes to address whatever deprecations I can..

--- Edit --- I think you're close, unfortunately idle-active is not 100% equivalent to the old event.. it doesn't get called at the start. If I manually make pause false, things work again so it's an initialization thing. Need to figure out a place to put the initialization setPlayState(Mpv::Idle) which should happen after the mpv engine is initialized..

--- Edit --- Okay this took me a long time to debug but I finally figure it out.. The original code exploited this missing break

// ...
            case MPV_EVENT_FILE_LOADED:
                setPlayState(Mpv::Started);
                LoadFileInfo();
                SetProperties();
                // notice no break here
            case MPV_EVENT_UNPAUSE: // this got removed
                setPlayState(Mpv::Playing);
                break;  // and importantly this
            case MPV_EVENT_END_FILE:
// ...

The new code is missing a break which causes mpv to think the file ended when it just began...

// ...
            case MPV_EVENT_FILE_LOADED:
                setPlayState(Mpv::Started);
                LoadFileInfo();
                SetProperties();
                // should be a break here but there isn't
            case MPV_EVENT_END_FILE:
                if(playState == Mpv::Loaded)
                    ShowText(tr("File couldn't be opened"));
// ...

Note to self; redundant code is probably better than weird control flow that causes hard to track bugs.

Thanks again for taking the initiative on correcting these deprecations which would soon be causing baka not to compile. :tada: