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

frontend: Add support for TEB/Multitrack Video to macOS (arm64 only) #11440

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dsaedtler
Copy link
Contributor

Description

Adds TEB support for macOS on Apple Silicon.

Motivation and Context

Want to support all users, including macOS/Linux (Linux tbd).
The small number of SKUs makes Apple Silicon easy, and since Intel Macs are losing support it seems fine to only support ARM.

Note: GetClientConfiguration API changes have not yet rolled out into production.

How Has This Been Tested?

Locally with Twitch staging environment.

Types of changes

  • New feature (non-breaking change which adds functionality)

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.

@dsaedtler dsaedtler force-pushed the rodney/multitrack-macos branch from 36502f5 to 467ade6 Compare October 22, 2024 21:13
@WizardCM WizardCM added macOS Affects macOS New Feature New feature or plugin labels Oct 26, 2024
@dsaedtler
Copy link
Contributor Author

Configuration API support for macOS is now rolling out into production, within the next few days this PR should be testable by anyone.

@dsaedtler
Copy link
Contributor Author

The server-side changes have now rolled out, so this PR can be tested by anybody who wishes to do so!

@dsaedtler dsaedtler force-pushed the rodney/multitrack-macos branch from 467ade6 to 037e056 Compare November 6, 2024 16:00
@dsaedtler dsaedtler force-pushed the rodney/multitrack-macos branch from 037e056 to 0ee9591 Compare November 19, 2024 14:05
@dsaedtler dsaedtler force-pushed the rodney/multitrack-macos branch from 0ee9591 to fa66f51 Compare January 22, 2025 21:08
Copy link
Member

@RytoEX RytoEX left a comment

Choose a reason for hiding this comment

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

We should probably add a license header to frontend/utility/system-info-macos.mm. That need not be done here, but it would be fine to include here.

Prefix should be frontend: .

@PatTheMav I'd appreciate a quick glance to make sure the includes/imports are as preferred.

frontend/utility/system-info-macos.mm Outdated Show resolved Hide resolved
frontend/utility/system-info-macos.mm Outdated Show resolved Hide resolved
@dsaedtler dsaedtler force-pushed the rodney/multitrack-macos branch from fa66f51 to aed7abe Compare January 23, 2025 23:18
@@ -1,6 +1,96 @@
#include "system-info.hpp"

#ifdef __aarch64__
#include <util/platform.h>
Copy link
Member

Choose a reason for hiding this comment

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

You can just use #import everywhere, there is no technical reason not to for ObjC except for possible cosmetic reasons.

Also correct order per the new rules (case-sensitive sort if there is no technical reason for a different order) should be:

#import <Foundation/Foundation.h>
#import <Foundation/NSProcessInfo.h>
#import <sys/sysctl.h>
#import <sys/types.h>

Copy link
Contributor Author

@dsaedtler dsaedtler Jan 24, 2025

Choose a reason for hiding this comment

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

You can just use #import everywhere, there is no technical reason not to for ObjC except for possible cosmetic reasons.

Yeah I just kept using include for C and import for ObjC, but I can change it for both.

Also correct order per the new rules (case-sensitive sort if there is no technical reason for a different order) should be:

Are these "new rules" documented anywhere?

Copy link
Member

Choose a reason for hiding this comment

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

Only by example, all new frontend files' includes have been arranged that way (and if they aren't it'd be an error).

The general order:

  1. Header associated with source file
  2. Other header files from the same project
  3. Header files from non-standard library
  4. Header files from standard library

Is more or less an established good-practice anyway. Sorting case-sensitive has the additional benefit of putting C++ classes (or in this case ObjC frameworks) as high-level language objects before low-level language objects almost automatically.

Thus the order of includes on a micro-level align neatly with the order of includes on a macro-level:

  1. Headers associated with larger frameworks or classes, mostly OO
  2. Headers associated with bare code and type definitions, mostly non-OO

So the further down in the includes, the more "low-level" the include is.

@dsaedtler dsaedtler force-pushed the rodney/multitrack-macos branch from aed7abe to 6395e7a Compare January 24, 2025 15:09
@dsaedtler dsaedtler changed the title UI: Add support for TEB/Multitrack Video to macOS (arm64 only) frontend: Add support for TEB/Multitrack Video to macOS (arm64 only) Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
macOS Affects macOS New Feature New feature or plugin
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

5 participants