Architecture: adding windows on ARM support
Introduces Windows on ARM support to the OBS-studio application. This includes modifications that are guarded with ARM-specific macros to ensure compatibility without affecting the existing codebase.
Description
This pull request introduces several code refactoring changes to enhance WOA support and resolve conflicts in the OBS-Studio application. The key changes include:
- UI: Added
shellapi.hto fix compilation error, AddedNOMINMAXflag to solve compilation errors, EnsuredWIN32_LEAN_AND_MEANis defined conditionally if not defined. - w32-pthreads: Added additional definitions of ARM macros
_M_ARMand_M_ARM64. - libobs: updated
obs-win-crash-handler.cto handle instruction pointers and stack traces, improving crash reporting on ARM64 systems . - obs-filters: Undefined
S_THRESHOLDdue to a conflicting existing macro with a numeric value. - obs-outputs: Replaced
__tzcnt_u32()with_CountTrailingZeros()for ARM.
Motivation and Context
The primary motivation for these changes is to expand compatibility with ARM architecture (windows) by addressing conflicts with existing macros and functions. By introducing ARM-specific macros and guarding ARM-specific code, we ensure that the application runs smoothly without affecting the existing codebase.
-
UI Changes:
- Added shellapi.h in obs-app.cpp to fix compilation errors.
- Updated min/max values to use limits header for VolumeMeter and VolumeSlider.
- Ensured WIN32_LEAN_AND_MEAN is included conditionally in window-basic-filters, window-basic-interaction, and window-basic-properties.
- Updated min/max values in the DrawStripedLine function in window-basic-preview.
- Updated min/max values for preset setting in window-basic-settings-a11y.cpp.
-
Additional ARM definitions in deps/w32-pthreads/context.h:
- Update w32-pthreads/context.h to include definitions for _M_ARM and _M_ARM64 in addition to existing ARM definitions.
-
Libobs changes:
- Adjust obs-win-crash-handler.c to handle instruction pointers & stack traces for crash reporting.
-
Obs-filters changes:
- Undefines the macro
S_THRESHOLDif previously defined, then redefines it with a new value"threshold".
- Undefines the macro
-
Obs-outputs: Use _CountTrailingZeros() in ctz32
- Replaced
tzcnt_u32with_CountTrailingZerosto ensure compatibility with WOA machines.
- Replaced
How Has This Been Tested?
The changes have been tested by building OBS Studio on an ARM machine (X Elite machine, Win11). All builds were successful, Basic functionality tests confirmed that the OBS Studio application with the modifications is functional.
Types of changes
- New feature (non-breaking change which adds functionality)
Checklist:
- [x] My code has been run through clang-format.
- [x] I have read the contributing document.
- [x] My code is not on the master branch.
- [x] The code has been tested.
- [x] All commit messages are properly formatted and commits squashed where appropriate.
- [x] I have included updates to all appropriate documentation.
Unless you have tested QSV with your arm devices you should drop the proposed changes there and ensure the plugin is not built for arm.
Please reword the PR as "adding Windows on ARM support", we already support ARM64 on macOS and OBS can be built on Linux aarch64.
Also there is no CI build check which does not help to avoid/find build issue on WoA, which is not good at all since not all contributors will have WoA machines to check if it builds.
Edit: Also, read our contribution guidelines and split your commits.
The commit messages need to follow the guideline. https://github.com/obsproject/obs-studio/blob/master/CONTRIBUTING.rst#commit-guidelines
@RytoEX I would like to know if AJA functionality is planned to be included in the Phase-1 release of the arm64 version of OBS. Currently, in AJA the __cpuid method is not supported on ARM devices, Could you provide some insights into what functionality we will miss in OBS on ARM due to this limitation ?
Edit: Identified an alternative way to retrieve the CPU string from the register (as done in libobs/obs-windows.c log_processor_info()). Can this be a replacement for the __cpuid method on ARM devices?
@RytoEX I would like to know if AJA functionality is planned to be included in the Phase-1 release of the arm64 version of OBS. Currently, in AJA the
__cpuidmethod is not supported on ARM devices, Could you provide some insights into what functionality we will miss in OBS on ARM due to this limitation ?Edit: Identified an alternative way to retrieve the CPU string from the register (as done in
libobs/obs-windows.clog_processor_info()). Can this be a replacement for the__cpuidmethod on ARM devices?
Poking @paulh-aja for AJA's current stats on ARM support. (sorry for double ping here, accidentally replied to the wrong comment!)
@RytoEX I would like to know if AJA functionality is planned to be included in the Phase-1 release of the arm64 version of OBS. Currently, in AJA the
__cpuidmethod is not supported on ARM devices, Could you provide some insights into what functionality we will miss in OBS on ARM due to this limitation ? Edit: Identified an alternative way to retrieve the CPU string from the register (as done inlibobs/obs-windows.clog_processor_info()). Can this be a replacement for the__cpuidmethod on ARM devices?Poking @paulh-aja for AJA's current stats on ARM support. (sorry for double ping here, accidentally replied to the wrong comment!)
My thinking on this would be that if there's an ARM64 equivalent to __cpuid, then it should (probably) be trivial to ifdef appropriately, which should be fine. That said, the linked code is in AJA's SDK, so the changes will have to be made there, not in OBS. It is worth noting that you cannot simply lift existing code from the obs-studio repo to drop into AJA's repo as the licenses, while compatible, are different. The correct way to address this is likely to file an Issue (or PR) on their repo.
It may also be worth noting that the obs-qsv11 plugin also uses __cpuid.
Currently, in AJA the __cpuid method is not supported on ARM devices.
Poking @paulh-aja for AJA's current stats on ARM support.
Please open a PR or at least raise an issue in the libajantv2 repo as RytoEX mentioned.
We do not have Windows ARM support on our current release roadmap. However, if the only thing blocking it is replacing the use of __cpuid in the ajabase code, then it may be trivial to add it in. Our Mac and Linux drivers and SDK do support ARM64 today.
Please open a PR or at least raise an issue in the libajantv2 repo as RytoEX mentioned.
@paulh-aja I've raised a PR in the libajantv2 repo to replace the __cpuid in the ajabase code.
@thirumalai-qcom I did see your PR in libajantv2. I'll test it out and get it merged into our next release if it all looks good. Thank you for your contribution!
The correct way to address this is likely to file an Issue (or PR) on their repo.
@RytoEX
As the AJA PR has been successfully merged into the mainline, I was curious to know if there are any updates on this PR merging for the initial release on Windows on ARM (WOA)? Could you also provide your inputs on which fix would be appropriate to resolve the compilation errors related to std::numeric_limits<int>::min() error and S_THRESHOLDmacro redefinition warning?
With #11385 merged, please drop the NOMINMAX changes from this PR. Additionally, the CMake changes seem to affect more lines than they should. Please drop those and revert the scope of this PR back to just the remaining code changes necessary to make compiling possible
@RytoEX
Thanks for the information regarding NOMINMAX, I have refactored the PR with your suggestions.
Please update the commit messages to reflect the current changes.
Thanks for the feedback @RytoEX, I've updated the commit messages and descriptions to reflect the current changes. Let me know if any further adjustments are needed.