-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add option for clearing track's waveform #841
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
Closed
Closed
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
835308e
Add option for clearing waveforms of tracks (not fully functional)
ninomp c49508b
Introduce clear waveform flag in TIO
ninomp 40deedf
Merge branch 'master' into clearwaveform
ninomp 1bfe8d6
Fix build
ninomp 8fcee61
Merge branch 'master' into clearwaveform
ninomp d50015b
Follow changes from 1bce32333946a3e3e1b15fb5f9b85558b0231236
ninomp de9ff3f
Address review comments about using locks and nullptr
ninomp 2107d6a
Rename TIO's waveform members
ninomp 331672b
Fix a potential bug when clear waveform flag is not cleared because a…
ninomp 6078672
Merge branch 'master' into clearwaveform
ninomp a115872
Fix a possible race condition in TIO
ninomp File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,6 +56,7 @@ TrackInfoObject::TrackInfoObject( | |
| m_dateAdded(QDateTime::currentDateTime()), | ||
| m_bHeaderParsed(false), | ||
| m_bBpmLocked(false), | ||
| m_bClearWaveformRequested(false), | ||
| m_analyzerProgress(-1) { | ||
| } | ||
|
|
||
|
|
@@ -653,21 +654,45 @@ QString TrackInfoObject::getURL() const { | |
| } | ||
|
|
||
| ConstWaveformPointer TrackInfoObject::getWaveform() { | ||
| return m_waveform; | ||
| return m_pWaveform; | ||
| } | ||
|
|
||
| void TrackInfoObject::setWaveform(ConstWaveformPointer pWaveform) { | ||
| m_waveform = pWaveform; | ||
| emit(waveformUpdated()); | ||
| QMutexLocker lock(&m_qMutex); | ||
| if (m_pWaveform != pWaveform) { | ||
| m_pWaveform = pWaveform; | ||
| markDirtyAndUnlock(&lock); | ||
| emit(waveformUpdated()); | ||
| } | ||
| } | ||
|
|
||
| ConstWaveformPointer TrackInfoObject::getWaveformSummary() const { | ||
| return m_waveformSummary; | ||
| return m_pWaveformSummary; | ||
| } | ||
|
|
||
| void TrackInfoObject::setWaveformSummary(ConstWaveformPointer pWaveform) { | ||
| m_waveformSummary = pWaveform; | ||
| emit(waveformSummaryUpdated()); | ||
| QMutexLocker lock(&m_qMutex); | ||
| if (m_pWaveformSummary != pWaveform) { | ||
| m_pWaveformSummary = pWaveform; | ||
| markDirtyAndUnlock(&lock); | ||
| emit(waveformSummaryUpdated()); | ||
| } | ||
| } | ||
|
|
||
| bool TrackInfoObject::isClearWaveformRequested() const { | ||
| QMutexLocker lock(&m_qMutex); | ||
| return m_bClearWaveformRequested; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here a lock is required, even if it is only a primitive boolean value! |
||
| } | ||
|
|
||
| void TrackInfoObject::setClearWaveformRequested(bool requested) { | ||
| QMutexLocker lock(&m_qMutex); | ||
| m_bClearWaveformRequested = requested; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here, locking is required |
||
| } | ||
|
|
||
| void TrackInfoObject::clearWaveform() { | ||
| setClearWaveformRequested(true); | ||
| setWaveform(WaveformPointer()); | ||
| setWaveformSummary(WaveformPointer()); | ||
| } | ||
|
|
||
| void TrackInfoObject::setAnalyzerProgress(int progress) { | ||
|
|
||
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
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.
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 in both AnalyzerWaveform and AnalysisDao for clearing the waveform if requested looks very similar. I would recommend to encapsulate the code from AnalysisDao in a function:
bool AnalysisDao::clearWaveformIfRequested(TrackInfoObject* pTrack)
Then from AnalyzerWaveform::loadStored() you could simply invoke pAnalysisDao->clearWaveformIfRequested(tio) before obtaining the waveform pointers from the TIO.
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.
Yes, you are right, but I'm planning to implement automatic clear of waveforms, beats and keys of track being loaded if Mixxx detects that audio of the track has changed, so I need a name that will cover both (automatic and manual) clearing. Do you mind if I do this then?
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.
Sure :) Thanks for your work, Nino! It's a pleasure to review your commits,
well done.
On 01/11/2016 11:06 PM, Nino MP wrote:
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.
Thank you! 😄 I'm glad you like it.