Fix waveform overview lenght. #11359
Conversation
…ed during waveform analysis.
|
I rebased this on top of the main branch and I can confirm that this also fixes the problem I posted about in #11280 (comment). |
src/widget/woverview.h
Outdated
| double getTrackSamples() const { | ||
| return m_trackSamplesControl->get(); | ||
| if (m_trackLoaded) { | ||
| return m_trackSamplesControl->get(); | ||
| } else { | ||
| return 0.0; | ||
| } | ||
| } |
There was a problem hiding this comment.
IMO this only fixes the symptom of a more complex issue. If there is no track loaded, the CO should already be 0.0...
There was a problem hiding this comment.
Do you have a better solution?
The complex issue is that the track change is propagated via the Qt signal queue while the track samples control is changed instantly as an atomic.
m_trackLoaded bool ensures that the value is not used during the transition.
There was a problem hiding this comment.
I see, so its basically a data-race.
The problem I still have with this solution is that the waveform assumes that CO values <= 0.0 mean that no track is loaded. Or rather, it relies on the fact that that is always the case (which it isn't due to the data-race). I think it would be better to move the m_trackLoaded != nullptr check to callers where they should actually check that.
There was a problem hiding this comment.
The track length is set here to an invalid value from the worker thread:
mixxx/src/engine/enginebuffer.cpp
Line 526 in ceaedb8
in a responds from a new track loaded into the player.
The Waveform overview visualizes the track loading process. It has already the new track before the engine had evicted the old one. In this situation is guarded by the m_trackLoaded boolean.
I will add a comment.
There was a problem hiding this comment.
I think a outdated waveform update signal cause a paint event with these inconstant data.
|
@Swiftb0y done. |
src/widget/woverview.h
Outdated
| // Ignor the value, because the engine can still have the old track | ||
| // during loading |
There was a problem hiding this comment.
| // Ignor the value, because the engine can still have the old track | |
| // during loading | |
| // Ignore the value, because the engine can still have the old track | |
| // during loading |
There was a problem hiding this comment.
hmm, codespell didn't catch that
There was a problem hiding this comment.
I have amended the last commit
|
Done |
|
Two users confirmed this fixes #11344 so this is ready to go without a second review? |
This aims to fix #11344 where the waveform overview is rendered with the wrong length.
A possible regression from #11162
mixxx-2.3.3-145-gcfcd265afd-win64.msi