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

win: New PaWinUtil_GetOsVersion() function for getting Windows OS version #830

Merged
merged 1 commit into from
Aug 25, 2023

Conversation

dmitrykos
Copy link
Collaborator

@dmitrykos dmitrykos commented Aug 5, 2023

As per discussion in #796 introducing a single API for getting Windows OS version in PA that will improve client code readability and reliability.

Additionally:

  • refactored WASAPI, DS, MME and WDMKS host back-ends to use PaWinUtil_GetOsVersion() instead of direct OS API

@dmitrykos dmitrykos added the src-os-win Windows-specific sources in /src/os/win label Aug 5, 2023
@dmitrykos dmitrykos self-assigned this Aug 5, 2023
@dmitrykos dmitrykos force-pushed the win_get_os_version branch 2 times, most recently from bd45895 to 0c9de1f Compare August 6, 2023 07:19
Copy link
Collaborator

@philburk philburk left a comment

Choose a reason for hiding this comment

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

Nice. Thanks for doing this.

src/os/win/pa_win_version.h Outdated Show resolved Hide resolved
Copy link
Collaborator

@philburk philburk left a comment

Choose a reason for hiding this comment

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

LGTM, I'll wait for Ross to review.

@dmitrykos
Copy link
Collaborator Author

Overlooked that WDMKS is also using GetVersion, just added it into the PR.

Copy link
Collaborator

@RossBencina RossBencina left a comment

Choose a reason for hiding this comment

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

Hi Dmitry,

In addition to my one review comment, could you please let us know what tests you have run on this new code. Thankyou!

src/hostapi/wasapi/pa_win_wasapi.c Show resolved Hide resolved
…sion. Refactored WASAPI, DS, MME and WDMKS host back-ends to use PaWinUtil_GetOsVersion() instead of direct OS API.
@dmitrykos
Copy link
Collaborator Author

dmitrykos commented Aug 12, 2023

In addition to my one review comment, could you please let us know what tests you have run on this new code.

I tested on Windows 10:

PortAudio Test: output sine wave. SR = 44100, BufSize = 64
before paHostApiInitializers[0].
getting Windows version with RtlGetVersion(): major=10, minor=0, build=19045, platform=2
Windows version=10

also Windows UWP and the lowest I have is Windows Server 2003 R2. I did not test on Windows Vista, 7, 8.

Windows Server 2003 R2 is practically latest Windows XP with 3790 build (https://en.wikipedia.org/wiki/List_of_Microsoft_Windows_versions) which is detected correctly:

PortAudio Test: output sine wave. SR = 44100, BufSize = 64
before paHostApiInitializers[0].
getting Windows version with RtlGetVersion(): major=5, minor=2, build=3790, platform=2
Windows version=5

Unfortunately I did not implement testing of every Windows version that could be done with Docker I believe due to lack of time resource. The underlying implementation is a copy-paste from the WASAPI's implementation which is time-tested on Windows Vista, 7, 8.

@philburk philburk merged commit 6ee9836 into PortAudio:master Aug 25, 2023
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
src-os-win Windows-specific sources in /src/os/win
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants