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

Expose sample rate in settings dialog #7125

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

zynskeywolf
Copy link
Contributor

@zynskeywolf zynskeywolf commented Feb 23, 2024

Adds basic ability to manually set the preferred sample rate used by the audio backend. Basic and preferred because the list box is not actually populated based on the backend's report yet.

Fixes #4607

Copy link
Contributor

@sakertooth sakertooth left a comment

Choose a reason for hiding this comment

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

Just tested, everything seems to be working as expected.

void SetupDialog::updateSampleRates()
{
m_sampleRate->clear();
if (m_audioInterfaces->currentText() == "JACK (JACK Audio Connection Kit)")
Copy link
Member

Choose a reason for hiding this comment

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

The strings for audio interface name could be changed due to translations and future code changes. According to line 493 and 536-549, the value will be AudioDeviceSetupWidget::tr(AudioJack::name()).
I think you should update the combo box based on the AudioDeviceSetupWidget child class to be selected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, but as side question, do we really have a good reason to localise the backend names? They are their Names after all.

src/gui/modals/SetupDialog.cpp Show resolved Hide resolved
@DomClark
Copy link
Member

DomClark commented Mar 2, 2024

There are specific configuration widgets for each backend - would it be better to put the configuration option in those, where it can be adapted to each specific backend (e.g. omitted if the backend doesn't support a choice of sample rate, or be populated with the available rates)?

@zynskeywolf
Copy link
Contributor Author

There are specific configuration widgets for each backend - would it be better to put the configuration option in those, where it can be adapted to each specific backend (e.g. omitted if the backend doesn't support a choice of sample rate, or be populated with the available rates)?

My motivation to not put it in there was the fact that it does not actually set the sample rate of the audio backend directly but rather it's a preferred setting for the lmms engine which then TRIES to preferably open the device using the specified frequency IF possible. I intend this PR only as a quick workaround until the audio backend mess actually gets sorted out. (Thinking of the misterious "HQ mode" option and the like)
Putting it in the backed-specific widget would suggest that the device itself will be opened with the specified setting, but for that to actually happen we first need to have the ability to query the supported settings.

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.

Change sample rate option
4 participants