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.

Sign up for GitHub

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

Jump to bottom

mac-virtualcam: Build a universal x86_64+arm64 binary for M1 Macs #3779

Merged
merged 1 commit into from
Dec 4, 2020
Merged

mac-virtualcam: Build a universal x86_64+arm64 binary for M1 Macs #3779

merged 1 commit into from
Dec 4, 2020

Conversation

johnboiles
Copy link
Contributor

Description

This patch updates the DAL part of the macOS Virtualcam plugin to be a fat binary that includes both x86_64 and arm64 binaries.

Motivation and Context

This enables universal apps running on Apple Silicon to use the plugin (even while OBS is running as x86_64 under Rosetta).

This patch can probably be removed once all of OBS supports Apple Silicon (since I assume something like this will be the default for the entire build).

> file build/plugins/mac-virtualcam/src/dal-plugin/obs-mac-virtualcam.plugin/Contents/MacOS/obs-mac-virtualcam
build/plugins/mac-virtualcam/src/dal-plugin/obs-mac-virtualcam.plugin/Contents/MacOS/obs-mac-virtualcam: Mach-O universal binary with 2 architectures: [x86_64:Mach-O 64-bit bundle x86_64] [arm64:Mach-O 64-bit bundle arm64]
build/plugins/mac-virtualcam/src/dal-plugin/obs-mac-virtualcam.plugin/Contents/MacOS/obs-mac-virtualcam (for architecture x86_64):	Mach-O 64-bit bundle x86_64
build/plugins/mac-virtualcam/src/dal-plugin/obs-mac-virtualcam.plugin/Contents/MacOS/obs-mac-virtualcam (for architecture arm64):	Mach-O 64-bit bundle arm64

How Has This Been Tested?

There's been some testing in my Repo with a similar change. See:

johnboiles/obs-mac-virtualcam#239
johnboiles/obs-mac-virtualcam#240

Types of changes

  • Cmake build change

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

johnboiles
johnboiles commented Nov 27, 2020
View reviewed changes
plugins/mac-virtualcam/src/dal-plugin/CMakeLists.txt
@@ -1,3 +1,6 @@
# Universal build for Apple Silicon
set(CMAKE_OSX_ARCHITECTURES "x86_64;arm64")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to https://cmake.org/cmake/help/v3.5/variable/CMAKE_OSX_ARCHITECTURES.html

The value of this variable should be set prior to the first project() or enable_language() command invocation because it may influence configuration of the toolchain and flags. It is intended to be set locally by the user creating a build tree.

@WizardCM WizardCM added Enhancement Improvement to existing functionality macOS Affects macOS labels Nov 27, 2020
@johnboiles
Copy link
Contributor Author

cc @PatTheMav

@johnboiles johnboiles mentioned this pull request Nov 27, 2020
Closed
johnboiles
johnboiles commented Nov 27, 2020
View reviewed changes
.github/workflows/main.yml Outdated
@@ -34,6 +34,11 @@ jobs:
uses: actions/checkout@v2.3.3
with:
submodules: 'recursive'
# This will probably not be necessary once GitHub actions
# uses an Xcode that supports macOS arm64 by default.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, we should probably just sit on this since it looks like this will be automatically fixed on Nov 30: actions/runner-images#2056

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@johnboiles Small reminder to remove this if it is truly not required anymore by tomorrow.

@WizardCM WizardCM marked this pull request as draft November 27, 2020 00:40
@johnboiles
Copy link
Contributor Author

Looks like Azure is failing because it doesn't have the most recent Xcode. Some quick Googling didn't turn up any info about setting a different Xcode version, but I bet they update their Xcode soon, so one strategy is to just wait until we do, and then merge this PR.

johnboiles
johnboiles commented Nov 27, 2020
View reviewed changes
azure-pipelines.yml Outdated
@@ -32,6 +32,8 @@ jobs:
steps:
- script: git submodule update --init --recursive
displayName: 'Checkout Submodules'
- script: sudo xcode-select -s "/Applications/Xcode_12.2.app"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't track down a date for when Azure is switching to Xcode 12.2 by default.

@johnboiles
Copy link
Contributor Author

Ok fixed Azure too.

My recommendation is to merge this before we ship an OBS release with the macOS Virtualcam so that users on Apple Silicon can use this plugin in universal apps.

But if it's going to be a couple weeks before we ship an OBS release, we could hold on merging this and it's possible we'll be able to revert the CI changes (because GitHub and Azure make Xcode 12.2 the default).

@WizardCM WizardCM added this to the OBS Studio 26.2 milestone Nov 27, 2020
@johnboiles johnboiles marked this pull request as ready for review November 27, 2020 04:45
@wookayin
Copy link

Could we move this to the 26.1 milestone so that the built-in virtualcam can be used?

@johnboiles
Copy link
Contributor Author

Build failed -- Xcode 12.2 must not have propagated to the actions runners yet.

@johnboiles
Copy link
Contributor Author

Also +1 to @wookayin's comment. That might help us avoid unnecessary support inquiries from folks on M1 Macs and also will allow me to archive github.com/obs-mac-virtualcam (so people don't get confused about which to use)

@PatTheMav
Copy link
Member

As stated in actions/runner-images#2056, propagation of Xcode 12.2 might take 2-3 days.

@johnboiles
Copy link
Contributor Author

Yep, i'm just over eager :)

@WizardCM
Copy link
Member

WizardCM commented Dec 2, 2020

Please squash your commits & correctly prefix them.

@johnboiles
Copy link
Contributor Author

If I bring back the manual xcode 12.2 selection are we cool with going ahead and merging this? If so I'll go ahead and squash commits and we can merge. Then at some point when GH updates the action runners to have an arm64 compatible xcode by default we can remove those CI steps in a different PR.

@johnboiles
Copy link
Contributor Author

In the meantime I'll try Azure also to see if they've updated their default Xcode

@johnboiles johnboiles changed the title Build the mac-virtualcam plugin as a universal x86_64+arm64 binary mac-virtualcam: Build a universal x86_64+arm64 binary for M1 Macs Dec 4, 2020
@johnboiles
Copy link
Contributor Author

Ok the whole build worked (except for linux which is probably a flakey build). Removing the CI changes and rebasing/squashing!

@johnboiles
Copy link
Contributor Author

🎉

@jp9000 jp9000 merged commit 55da44d into obsproject:master Dec 4, 2020
@wookayin wookayin mentioned this pull request Dec 6, 2020
Closed
@hroland
Copy link

hroland commented Dec 8, 2020 •
edited
Loading

@johnboiles Thank you for the quick update! :) ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PatTheMav PatTheMav PatTheMav left review comments

Assignees
No one assigned
Labels
Enhancement Improvement to existing functionality macOS Affects macOS
Projects
None yet
Milestone
OBS Studio 26.1
Development

Successfully merging this pull request may close these issues.
7 participants
@johnboiles @wookayin @PatTheMav @WizardCM @hroland @dodgepong @jp9000