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

windows: move ffmpeg bins to submodule #10530

Merged
merged 3 commits into from Mar 31, 2022
Merged

windows: move ffmpeg bins to submodule #10530

merged 3 commits into from Mar 31, 2022

Conversation

shuffle2
Copy link
Contributor

@shuffle2 shuffle2 commented Mar 23, 2022

windows: move ffmpeg bins to submodule
udpate ffmpeg to 28d011516b72a2401b2289a4be34c6e8ab611a0b
enable ffmpeg on arm64
plumb ffmpeg logging to dolphin logging

contains the change from #10249 to enable utvideo, since that was the motivation to fixup ffmpeg stuff.

I also experimented with adding h264/hevc encoders (the Media Foundation backends are nice), but realized users that want that sort of functionality will probably just use xbox game bar recording or something.

@AdmiralCurtiss
Copy link
Contributor

Yeah, I agree this is the better way to do this.

@shuffle2
Copy link
Contributor Author

it seems to make git mad when a directory is removed and a submodule is created in the same location in the same commit.
i'm going to try breaking it into two commits. If that doesn't work, might have to name the submodule something else

@JosJuice
Copy link
Member

Are you sure the problem isn't that you didn't append .git to the URL?

@shuffle2
Copy link
Contributor Author

shuffle2 commented Mar 23, 2022

Are you sure the problem isn't that you didn't append .git to the URL?

I don't think it's a problem? The submodule works in a local clone of the shuffle2/dolphin repo. But there is a problem like:

>git checkout win-ffmpeg
Switched to a new branch 'win-ffmpeg'
Branch 'win-ffmpeg' set up to track remote branch 'win-ffmpeg' from 'origin'.
>git submodule update
Cloning into 'C:/src/dolphintest/Externals/FFmpeg'...
starting fsmonitor-daemon in 'C:/src/dolphintest/Externals/FFmpeg'
Submodule path 'Externals/FFmpeg': checked out '8cc3c8847438521c89e6e84b349009978e58f8b5'
>git checkout master
warning: unable to rmdir 'Externals/FFmpeg': Directory not empty

...git leaves Externals/FFmpeg with submodule contents instead of deleting it. I wonder if that is the problem?

edit: this is probably from fsmonitor-deamon, but not sure.
edit2: this seems to have been fixed locally after updating git
edit3: actually, there's still this problem - it only appeared to be solved because fsmonitor-daemon hadn't started in the submodule yet when I tested. anyway, it's been worked around in this PR by just renaming the dir

@shuffle2
Copy link
Contributor Author

shuffle2 commented Mar 23, 2022

hrm, after looking at the FS on the windows buildbot, it seems .git/modules/Externals/ffmpeg is the content of the submodule introduced by #10249 (case insensitive FS, remember...). Maybe that is the problem on the buildbot. The problem in my previous comment would still apply, I think.

@shuffle2 shuffle2 changed the title windows: move ffmpeg bins to submodule windows: move ffmpeg bins to submodule [wip] Mar 24, 2022
@shuffle2 shuffle2 changed the title windows: move ffmpeg bins to submodule [wip] windows: move ffmpeg bins to submodule Mar 24, 2022
@AdmiralCurtiss
Copy link
Contributor

So is this good to go? Looks fine to me.

@shuffle2
Copy link
Contributor Author

from my side it is

@JosJuice
Copy link
Member

@shuffle2 I would like to copy the contents of https://github.com/shuffle2/ext-win-ffmpeg to https://github.com/dolphin-emu/ext-win-ffmpeg before merging this, so that building Dolphin doesn't rely on the GitHub accounts of individuals still existing. Does this sound fine to you? I would create the repo and you would have to change the .gitmodules file in this PR.

@shuffle2
Copy link
Contributor Author

@shuffle2 I would like to copy the contents of https://github.com/shuffle2/ext-win-ffmpeg to https://github.com/dolphin-emu/ext-win-ffmpeg before merging this, so that building Dolphin doesn't rely on the GitHub accounts of individuals still existing. Does this sound fine to you? I would create the repo and you would have to change the .gitmodules file in this PR.

yes that's fine

@JosJuice
Copy link
Member

It has now been created at https://github.com/dolphin-emu/ext-win-ffmpeg.

I'm not sure exactly how the permissions are set up. I'm assuming there isn't more you need to do inside that repository for now, but if you want to do something with the repository in the future and the permissions seem to restrictive, just let me know.

@shuffle2
Copy link
Contributor Author

it's switched over, and i checked that windows buildbot switched to the correct remote for the submodule as well (it did; didn't need any intervention)

@AdmiralCurtiss
Copy link
Contributor

Tested, works. Let's merge this!

@AdmiralCurtiss AdmiralCurtiss merged commit 4957b2e into dolphin-emu:master Mar 31, 2022
@shuffle2 shuffle2 deleted the win-ffmpeg branch March 31, 2022 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants