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

Fix producing of NaN from Env/LFO parameter change while playing. #3761

Merged
merged 2 commits into from
Aug 12, 2017

Conversation

PhysSong
Copy link
Member

Fixes #3408.

Guarantee thread safety to ensure fillLevel() not to read value from wrong buffer address, because of buffer reallocation in updateSampleVars().

Guarantee thread safety to ensure fillLevel() not to read value from
wrong buffer address.
@PhysSong PhysSong requested a review from zonkmachine August 10, 2017 06:19
@PhysSong PhysSong mentioned this pull request Aug 10, 2017
Copy link
Contributor

@michaelgregorius michaelgregorius left a comment

Choose a reason for hiding this comment

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

Please use QMutexLockers to ensure safe unlocking in cases of exceptions, etc.

@@ -291,6 +291,8 @@ void EnvelopeAndLfoParameters::fillLevel( float * _buf, f_cnt_t _frame,
const f_cnt_t _release_begin,
const fpp_t _frames )
{
m_paramMutex.lock();
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use a QMutexLocker here to ensure safe unlocking in cases of exceptions, etc.

@@ -402,6 +406,8 @@ void EnvelopeAndLfoParameters::loadSettings( const QDomElement & _this )

void EnvelopeAndLfoParameters::updateSampleVars()
{
m_paramMutex.lock();
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use a QMutexLocker here to ensure safe unlocking in cases of exceptions, etc.

@PhysSong
Copy link
Member Author

@michaelgregorius Done via 667ee11.

Copy link
Member

@zonkmachine zonkmachine left a comment

Choose a reason for hiding this comment

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

Tested. Issue fixed.

@PhysSong
Copy link
Member Author

I'll merge it after 12~24 hours if there's no objection.

@PhysSong PhysSong merged commit 88cc586 into LMMS:stable-1.2 Aug 12, 2017
@PhysSong PhysSong deleted the env-nan branch August 12, 2017 01:16
PhysSong added a commit to PhysSong/lmms that referenced this pull request Aug 19, 2017
…S#3761)

Guarantee thread safety to ensure fillLevel() not to read value from
wrong buffer address.
PhysSong added a commit to PhysSong/lmms that referenced this pull request Aug 25, 2017
…S#3761)

Guarantee thread safety to ensure fillLevel() not to read value from
wrong buffer address.
sdasda7777 pushed a commit to sdasda7777/lmms that referenced this pull request Jun 28, 2022
…S#3761)

Guarantee thread safety to ensure fillLevel() not to read value from
wrong buffer address.
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.

3 participants