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 #317

Merged
merged 1 commit into from
Feb 26, 2022

Conversation

easyteacher
Copy link
Contributor

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.
@easyteacher easyteacher changed the title Port away from removed APIs in mpv 2.0 Port away from deprecated/removed APIs in mpv 2.0 Feb 21, 2022
@easyteacher
Copy link
Contributor Author

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
Copy link
Owner

u8sand commented Feb 21, 2022

@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
Copy link
Owner

u8sand commented Feb 26, 2022

@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. 🎉

@u8sand u8sand merged commit 7864f24 into u8sand:master Feb 26, 2022
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.

None yet

2 participants