-
Notifications
You must be signed in to change notification settings - Fork 87
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
fix: crash when clicking audio insanely fast. #1424
fix: crash when clicking audio insanely fast. #1424
Conversation
a1604f7
to
97a3824
Compare
connect( this, &AudioService::cancelPlaying, thread.get(), [ this ]( bool waitFinished ) { | ||
thread->cancel( waitFinished ); | ||
} ); | ||
thread.reset( new DecoderThread( audioData, this ) ); |
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.
This reset
here has the same meaning as the older thread = std::make_shared
because the older code also ensure the existing one is deleted.
I think the ffmpeg related codes can be removed totally , above qt6.5.* ? ,the ffmpeg has been the backend of qmultimedia module. in another PR? |
Quality Gate passedIssues Measures |
I don't know. Does open source qmultimedia ships proprietary codecs like mp3? Could this be a problem for Window build? For Linux, there is no problem to drop ffmpeg along with Qt5. Even the old qmultimedia is good enough, as the older backend of GStreamer supports adding codecs by adding plugins. |
There is a case that: when users click two different audios ,such as audioA,audioB, when click the second ,the first audio should be stopped. Can this case still be supported by this modification. |
Based on my testing. Yes. This PR just ensure that the older thread is finished before deleting it to avoid crash. 2 super long audio for test Archive.zip |
Maybe we can wait next Qt LTS for consider dropping ffmpeg and utilize QtMultimedia's ffmpeg backend. I reported this bug before. For versions below 6.5.2, QtMultimedia simply fatal crash without a way to recover if no backend is installed 😅 |
This can prevent crash of #1423
There are 2 problems
AudioOutputPrivate
seems to have some special internal).This works, but I am not sure how to fix this perfectly yet.
I think the original code was to spawn multiple audio simultaneously, which makes no sense to humans as nobody can hear mix audios.
So there will be one and only one working audio thread, right?