Fix waveform concurrency issues. Possible fix for Bug #1383404 #420
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rryan
force-pushed
the
waveform
branch
2 times, most recently
from
24707ae
to
394f715
Compare
rryan
changed the title
* Use const shared pointers for waveforms. * Move all std::vector sizing operations to the Waveform constructor. No vector state is allowed to change after the constructor finishes. * Clarify comments. * Remove waveform mutex usage in WOverview variants. * Use the track's waveform pointer to atomically swap the waveform rather than mutating the waveform in place. * Remove unused code.
Merged
| @@ -252,13 +258,13 @@ void AnalyserWaveform::process(const CSAMPLE* buffer, const int bufferLength) { | |||
| m_waveformSummary->setCompletion(m_currentSummaryStride); | |||
|
|
|||
| #ifdef TEST_HEAT_MAP | |||
| QPointF point(float(m_strideSummary.m_filteredData[Right][High]), | |||
| float(m_strideSummary.m_filteredData[Right][ Mid])); | |||
| QPointF point(float(m_stride.m_filteredData[Right][High]), | |||
|
pardon my usual ignorant questions |
:) notes addressed. |
|
|
||
| // If stored in the database, the ID of the waveform. | ||
| int m_id; | ||
| bool m_bDirty; | ||
| mutable bool m_bDirty; |
There was a problem hiding this comment.
might as well put a comment about why this is mutable
|
one last note, LGTM |
| m_waveformData = &m_waveform->at(0); | ||
| m_waveformSummaryData = &m_waveformSummary->at(0); | ||
| m_waveformData = m_waveform->data(); | ||
| m_waveformSummaryData = m_waveformSummary->data(); |
There was a problem hiding this comment.
We should get rid of this by moving the m_waveformSummary->data() Into WaveformStride::store()
|
Thank you for picking this up. Since this this solves an annoying bug it LGTM |
rryan
added a commit
that referenced
this pull request
Dec 4, 2014
Fix waveform concurrency issues. Possible fix for Bug #1383404
|
🚬 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
vector state is allowed to change after the constructor finishes.
rather than mutating the waveform in place.