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

don't wipe inapplicable sound config immediately #4544

Merged
merged 1 commit into from
Nov 27, 2021

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Nov 26, 2021

When SoundManager is initialized and previously configured output devices aren't available the config is wiped and the default config is written to disk before the user had a chance to intervene.

With this commit the new config (empty default or reconfigured) is written to disk only if the user clicked either Continue (with no outputs) or Reconfigure (and actually set an output device).


WIP though I'd appreciate feedback is this approach is acceptable until Mixxx supports multiple sound config profiles.
I didn't yet manage to comprehend how all sound devices config steps and sound error dialogs play together. I have the feeling that the inapplicable config should be discovered earlier, not just when configured outputs are missing.

@github-actions github-actions bot added the ui label Nov 26, 2021
@daschuer
Copy link
Member

Thank you for picking this up. This part can make use of an overhaul anyway.

src/mixxxmainwindow.cpp Outdated Show resolved Hide resolved
src/widget/wtrackmenu.cpp Outdated Show resolved Hide resolved
@daschuer
Copy link
Member

Seeing that the Jack ApI becoming more and more popular, we may consider the external routing use case in all our future steps.

We have for instance the issue that you start Mixxx, connect to an JACK output gapping through the routing stage. If later the user uses QJackControl to re-route the sound, Mixxx is not aware from it and displays wrong Info.

I think that can be done by separate the "stream enable" task from the "connect to physical output" task.

@ronso0 ronso0 force-pushed the sound-config-error branch 3 times, most recently from 84e42de to 0b95424 Compare November 26, 2021 20:14
@ronso0
Copy link
Member Author

ronso0 commented Nov 26, 2021

Ready!

@ronso0 ronso0 added this to the 2.4.0 milestone Nov 26, 2021
@ronso0
Copy link
Member Author

ronso0 commented Nov 26, 2021

oh, actually this should go to 2.3

@ronso0 ronso0 changed the base branch from main to 2.3 November 26, 2021 20:38
@ronso0
Copy link
Member Author

ronso0 commented Nov 26, 2021

Rebased, ready again.

@ronso0
Copy link
Member Author

ronso0 commented Nov 26, 2021

No CI except Pull Request Labeler??

@ronso0 ronso0 closed this Nov 26, 2021
@ronso0 ronso0 reopened this Nov 26, 2021
Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

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

LGTM, Thank you.

@daschuer daschuer merged commit bbb70de into mixxxdj:2.3 Nov 27, 2021
@ronso0 ronso0 deleted the sound-config-error branch November 27, 2021 00:59
@ronso0 ronso0 modified the milestones: 2.4.0, 2.3.2 Nov 28, 2021
@ronso0 ronso0 mentioned this pull request Jan 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants