-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Sound proxy fix and clean up #4584
Conversation
Please rebase on main |
As a first step I suggest to get rid of m_pAudioSource that is not needed. I will prepare a PR. Bigger refactorings should be done afterwards, step by step. |
963ff03
to
8b18970
Compare
Done |
8b18970
to
bffe627
Compare
This fixes lp1955840 for more than two SoundSources per file extension
bffe627
to
095110a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some minor suggestions
src/sources/soundsourceproxy.h
Outdated
@@ -196,11 +200,12 @@ class SoundSourceProxy { | |||
// provider and is initialized with -1 if no | |||
int m_providerRegistrationIndex; | |||
|
|||
void initSoundSource( | |||
const mixxx::SoundSourceProviderPointer& pProvider); | |||
void findAndInitSoundSource(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
findProviderAndInitSoundSource()
src/sources/soundsourceproxy.cpp
Outdated
VERIFY_OR_DEBUG_ASSERT(m_pTrack) { | ||
return nullptr; | ||
} | ||
if (openSoundSource(params)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer a linear control flow for the regular path:
if (!openSoundSource(params)) {
return nullptr;
}
...
src/sources/soundsourceproxy.h
Outdated
const mixxx::SoundSourceProviderPointer& pProvider); | ||
void findAndInitSoundSource(); | ||
|
||
bool initSoundSource( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
initSoundSourceWithProvider()
Done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code now better reveals the dependencies and states.Thank you! LGTM
This fixes https://bugs.launchpad.net/mixxx/+bug/1955840 and improves the readability of the initializing phase.
This is the refactoring for https://bugs.launchpad.net/mixxx/+bug/1955331 moved to the main branch to lower the risk for the stable 2.3 branch.