Stop Waveforms: Lp1740403 #1547
Conversation
src/waveform/visualplayposition.cpp
Outdated
| @@ -7,6 +7,13 @@ | |||
| #include "util/math.h" | |||
| #include "waveform/vsyncthread.h" | |||
|
|
|||
| namespace { | |||
| constexpr int kMaxOffsetRange = 2; // The offset is limited to two callback sizes | |||
There was a problem hiding this comment.
It's not obvious what unit of measurement applies here and why this constant is multiplied by 1000 at the two places where it is actually used.
|
Good idea to separate unrelated issues, much easier to review. Just one finding that should be fixed before we are ready to merge. |
|
Thank you for pushing me to better code. I had thought about it as well, but I was to lazy ;-) |
src/widget/woverview.cpp
Outdated
| paintText(tr("Finalizing .."), &painter); | ||
| } else if (!m_trackLoaded) { | ||
| // This happens if the track samples are not loaded, but we have | ||
| // a cashed track |
src/waveform/visualplayposition.h
Outdated
|
|
||
| private: | ||
| ControlValueAtomic<VisualPlayPositionData> m_data; | ||
| ControlProxy* m_audioBufferSize; | ||
| double m_dAudioBufferSize; // Audio buffer size in ms | ||
| double m_dAudioBufferUs; // Audio buffer size in µs |
There was a problem hiding this comment.
"Us" is very confusing, used here as both a suffix and a prefix. Legacy code, I know. The code would be easier to read if we consequently explicitly append "Millis" and "Micros" as a suffix for all durations that are used here. With this convention even the trivial constant kMicrosPerMilli would be obsolete, because then the factor 1000 is obvious and even more concise.
Labelling a duration as "size" is misleading too, but let's just keep it consistent ;)
|
Done. |
|
This small change has improved the readability a lot 👍 And it also prevents mistakes and misunderstandings in the future. |
|
LGTM |
This PR is the "easy" part from #1446
Fixing the endless moving waveforms https://bugs.launchpad.net/mixxx/+bug/1740403