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

mac: add macosvk gpu context #12493

Closed
wants to merge 10 commits into from
Closed

mac: add macosvk gpu context #12493

wants to merge 10 commits into from

Conversation

m154k1
Copy link
Contributor

@m154k1 m154k1 commented Sep 26, 2023

This adds support for long awaited vo gpu-next on macOS via Vulkan/MoltenVK.
This is a rebased version of https://github.com/rcombs/mpv/tree/mac_vulkan.

Might improve performance and battery life.
Tested on MacBook Air M2.

closes: #7482
closes: #7857

Screenshot 2023-09-26 at 19 01 35

@m154k1 m154k1 mentioned this pull request Sep 26, 2023
3 tasks
@Dudemanguy
Copy link
Member

Dudemanguy commented Sep 26, 2023

Sorry to make your rebase more painful but could you keep the separate commits of the original branch? Makes it easier to read and also keeps the credit of who wrote what clearer.

@Akemi
Copy link
Member

Akemi commented Sep 26, 2023

#7857 is something different and is not related to this tbh.

like i said before, the TODOs on my original PR should still be done.
the precise timer doesn't work properly on ARM based macs apparently. it needs adjustments.

the hwdec changes should be completely separate.

@m154k1
Copy link
Contributor Author

m154k1 commented Sep 26, 2023

Sorry to make your rebase more painful but could you keep the separate commits of the original branch? Makes it easier to read and also keeps the credit of who wrote what clearer.

Oh, I don't think I can do it again, sorry. I remember I had to do rebase multiple times due to build system and hwdec changes. I might break something if I'll do it again.
Also some commits are defiantly mean to be squashed. You can use that branch as a reference.

I can change commit message or author for credits.

like i said before, the TODOs on my original PR should still be done.
the precise timer doesn't work properly on ARM based macs apparently. it needs adjustments.

Yeah, but even with broken precise timer it still plays media without issues on an ARM mac :)
The point of this PR is merge something working now and finish in the future.

Feel free to edit this PR / commits or tell me how should I change it.

@Dudemanguy
Copy link
Member

If this is an improvement over the status quo (which it sounds like it is), then I'm all for it even if it is incomplete simply because it finally gives us a path forward to stop using the vo_libmpv hack.

Feel free to edit this PR / commits or tell me how should I change it.

I'll do the rebase for you. Give me a second.

Akemi and others added 9 commits September 26, 2023 12:45
add an animation lock to the window to prevent the window from closing
while animating. if this is done while the fs animation is running the
dock will stay hidden. this is not used yet, because it's unnecessary
for cocoa-cb but will be for new vo backends.
use VK_PRESENT_MODE_FIFO_KHR for video-sync

some more changes icc, size

backport some fixes

debugging shit

fix 100% cpu usage, and some wrong frame sizes

generalise moltenvk backend

log cleanup

generalisations

add icc profile (incomplete)

only clear when newly initialised

mac override

mac some cleanups

mac vulkan use layer struct

mac cleanup

mac

mac cleanup

mac: update icc profile on swap
mac remove debug print, manually sync to vsync
@Dudemanguy
Copy link
Member

Dudemanguy commented Sep 26, 2023

Okay rebased now and hopefully I didn't break something. I took the liberty to slightly reword a couple of commit messages as well. I included the hwdec and embedding support commits for now, but I'm thinking we should probably just drop them? The hwdec has nothing to do with this PR and I presume the embedding one doesn't actually work given it has a WIP.

Edit: woops messed up the hwdec rebase. I'll just drop those two commits for now.

@github-actions
Copy link

github-actions bot commented Sep 26, 2023

Download the artifacts for this pull request:

Windows

@m154k1
Copy link
Contributor Author

m154k1 commented Sep 26, 2023

Tested, looks good. Now hwdec isn't working with --vo=gpu-next but it's still working with old backend. I guess hwdec commits can be pushed in the next PR.

@Dudemanguy
Copy link
Member

I accidentally nuked your docs commit btw if you don't mind doing that again.

@m154k1
Copy link
Contributor Author

m154k1 commented Sep 26, 2023

Done.

@Akemi
Copy link
Member

Akemi commented Sep 26, 2023

Yeah, but even with broken precise timer it still plays media without issues on an ARM mac :) The point of this PR is merge something working now and finish in the future.

but it isn't without issues, i can vouch for several. the past 2 years none even tried to attempt to finish those todos. there is like near 0% chance anyone will finish it.

the point should not just be to merge something 'working'. it should adhere to a minimum standard of code quality. which it doesn't, at least the comments within the code should be handled or removed. if something isn't working currently, it should be removed to keep the code base clean and tidy.

if i felt like it was good enough, i could have merged it +2 years ago.

If this is an improvement over the status quo (which it sounds like it is), then I'm all for it even if it is incomplete simply because it finally gives us a path forward to stop using the vo_libmpv hack.

it's not incomplete per se, it's broken in parts. tbh, i wouldn't call it a clear improvement. it works better in some way and worse in others. i wouldn't even call it on par with the current status quo, but rather experimental. that's how it should be marked for now imo.

cocoa-cb isn't really a hack. it's a first party libmpv user.

i would like to veto on merging this for now. it might sound like an empty promise, though i recently got a mac again after mine broke some time ago. i am planning to clean up the mess i created with my absence the past 2 years. currently i am in the process of creating a todo list for all the needed, broken and new things. the first task will be cleaning up the missing things on the moltenvk backend PR.

it would be nice if you could give me some (more) time to cleanup my own mess.

@Dudemanguy
Copy link
Member

To be clear, it's vo_libmpv itself that I consider hack. The way it's designed is just a maintenance nightmare which is why the only backends that exist for it are the opengl one from way back when and a software one.

it would be nice if you could give me some (more) time to cleanup my own mess.

Sure that sounds good. I was just wanting a viable way to get macos onto gpu-next so that way we can push forward with it. The two options would basically be either have a libplacebo backend for the render API or have a moltenvk and/or metal context. I'm of the opinion that the render API is not worth supporting. It's basically rotting code anyways, so something like this is the ultimately the right way to go.

@m154k1
Copy link
Contributor Author

m154k1 commented Sep 27, 2023

currently i am in the process of creating a todo list for all the needed, broken and new things. the first task will be cleaning up the missing things on the moltenvk backend PR.

That's good :) Let's close this PR then.

@m154k1 m154k1 closed this Sep 27, 2023
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.

4 participants