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

Include ASIO details in Windows ASIO version #2605

Merged
merged 1 commit into from
May 22, 2022

Conversation

henkdegroot
Copy link
Contributor

@henkdegroot henkdegroot commented Apr 22, 2022

Short description of changes

Add ASIO logo and text to Windows Installer
Add ASIO reference to the About - Libraries page
Add ASIO reference to the CLI version output

All is only shown when the "default/ASIO" version is used.

CHANGELOG: Client: Include ASIO references in Windows ASIO version

Context: Fixes an issue?

Related: #2417

Does this change need documentation? What needs to be documented and how?

No

Screenshots

Installer Welcome Page:
installer-install-welcome-asio

About Screen:
about-libraries

Status of this Pull Request

Completed

What is missing until this pull request can be merged?

Review

Checklist

  • I've verified that this Pull Request follows the general code principles
  • I tested my code and it does what I want
  • My code follows the style guide
  • I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
  • I've filled all the content above

@@ -415,6 +415,11 @@ CAboutDlg::CAboutDlg ( QWidget* parent ) : CBaseDlg ( parent )
", <i><a href=\"https://www.qt.io\">https://www.qt.io</a></i></p>"
"<p>Opus Interactive Audio Codec"
", <i><a href=\"https://www.opus-codec.org\">https://www.opus-codec.org</a></i></p>"
# if defined( _WIN32 ) && !defined( WITH_JACK )
Copy link
Member

Choose a reason for hiding this comment

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

Hmm. Although not related, I think we should hava a "WITH_ASIO" default compile parameter instead of checking WITH_JACK everywhere...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps introduce the "WITH_ASIO" as part of audio redesign? Current reasoning is that the ASIO is just the default and therefore no need to define. Thinking if there will be a "no sound" option, it might help to add a "WITH_ASIO" define.

windows/installer.nsi Outdated Show resolved Hide resolved
@ann0see ann0see self-requested a review May 15, 2022 21:32
@hoffie hoffie added this to the Release 3.9.0 milestone May 21, 2022
Copy link
Member

@hoffie hoffie left a comment

Choose a reason for hiding this comment

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

Thanks, code looks good. I've also tested the installer start pages via wine.
@henkdegroot Could you please rebase to resolve the conflict?

Minor inline comment whitespace nit.

windows/installer.nsi Outdated Show resolved Hide resolved
@pljones pljones mentioned this pull request May 22, 2022
15 tasks
Copy link
Member

@hoffie hoffie left a comment

Choose a reason for hiding this comment

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

Thanks!

@hoffie hoffie merged commit 481bfa7 into jamulussoftware:master May 22, 2022
@henkdegroot henkdegroot deleted the asio_agreement branch May 22, 2022 18:38
@ann0see ann0see removed their request for review May 23, 2022 06:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants