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

Add FramePos type and use it for beatgrid creation #4043

Merged
merged 32 commits into from
Jul 2, 2021
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
18382f5
Frame: Add frame class to avoid sample/frame ambiguity
hacksdump Jun 13, 2020
6df3614
Frame: Add artithmetic and logical operators
hacksdump Jun 15, 2020
34d1f9d
Frame: Add multiply and divide operations
hacksdump Jun 16, 2020
92161c5
Frame: Replace forward declaration with include
hacksdump Jun 20, 2020
4a7bdc5
Frame: Refactor and rename to FramePos and FrameDiff_t
hacksdump Jun 21, 2020
e3124ff
FramePos: Use constant for invalid frame instead of -1
hacksdump Jun 26, 2020
e98439f
FramePos: Use C++ style limits
hacksdump Jun 26, 2020
eb550fa
FramePos: Use constant for starting frame pos
hacksdump Aug 24, 2020
b048478
FramePos: Use NaN instead of negative infinity
hacksdump Aug 31, 2020
9ae96f2
FramePos: Add Qt declarations
hacksdump Sep 9, 2020
3d9df08
FramePos: Define static start and invalid constants
hacksdump Sep 9, 2020
7a05af6
FramePos: Move class alongside other audio helper classes
Holzhaus Jul 1, 2021
db6b5f6
FramePos: Clean up comments
Holzhaus Jul 1, 2021
aa17102
FramePos: Remove d prefix for doubles
Holzhaus Jul 1, 2021
f79e907
FramePos: Make FramePos invalid when no value is given
Holzhaus Jul 1, 2021
4db7cb2
FramePos: Add isValid method to check if the position is valid
Holzhaus Jul 1, 2021
b82ba59
FramePos: Add fromEngineSamplePos helper method
Holzhaus Jul 1, 2021
e7e39e4
FramePos: Rename getValue() to just value()
Holzhaus Jul 1, 2021
0191a50
FramePos: Add helper method to truncate to full sample positions
Holzhaus Jul 1, 2021
af40d4f
Beats: Use frame positions for beatmap/beatgrid construction
Holzhaus Jul 1, 2021
cd8b88c
BeatMap: Reduce back-and-forth conversions between samples/frames
Holzhaus Jul 1, 2021
69c3717
BeatMap: Add inline to internal utility functions
Holzhaus Jul 1, 2021
f808edb
Frame: Add constexpr to some methods
Holzhaus Jul 1, 2021
fb2be0b
FramePos: Fix wrong fractional check
Holzhaus Jul 1, 2021
e4f9b22
FramePos: Use util_isnan to fix broken NaN detection
Holzhaus Jul 1, 2021
9d2811c
FramePos: Add tests
Holzhaus Jul 1, 2021
35a5ccd
BeatMap: Use in const reference in for loop to avoid copy
Holzhaus Jul 1, 2021
e671a6c
AnalyzerQueenMaryBeats: Rename m_stepSize to m_stepSizeFrames
Holzhaus Jul 2, 2021
ad42583
FrameTest: Check denormals and NaN
Holzhaus Jul 2, 2021
c26b018
FramePos: Make infinite frame positions invalid
Holzhaus Jul 2, 2021
70416aa
BeatMap: Add helper function and add assertion for fraction positions
Holzhaus Jul 2, 2021
a555ae4
FramePos: Assert validity in isFractional()
Holzhaus Jul 2, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1508,6 +1508,7 @@ add_executable(mixxx-test
src/test/enginemicrophonetest.cpp
src/test/enginesynctest.cpp
src/test/fileinfo_test.cpp
src/test/frametest.cpp
src/test/globaltrackcache_test.cpp
src/test/hotcuecontrol_test.cpp
src/test/imageutils_test.cpp
Expand Down
4 changes: 2 additions & 2 deletions src/analyzer/analyzerbeats.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ void AnalyzerBeats::storeResults(TrackPointer pTrack) {

mixxx::BeatsPointer pBeats;
if (m_pPlugin->supportsBeatTracking()) {
QVector<double> beats = m_pPlugin->getBeats();
QVector<mixxx::audio::FramePos> beats = m_pPlugin->getBeats();
QHash<QString, QString> extraVersionInfo = getExtraVersionInfo(
m_pluginId, m_bPreferencesFastAnalysis);
pBeats = BeatFactory::makePreferredBeats(
Expand All @@ -232,7 +232,7 @@ void AnalyzerBeats::storeResults(TrackPointer pTrack) {
} else {
float bpm = m_pPlugin->getBpm();
qDebug() << "AnalyzerBeats plugin detected constant BPM: " << bpm;
pBeats = BeatFactory::makeBeatGrid(m_sampleRate, bpm, 0.0f);
pBeats = BeatFactory::makeBeatGrid(m_sampleRate, bpm, mixxx::audio::kStartFramePos);
}

pTrack->trySetBeats(pBeats);
Expand Down
5 changes: 3 additions & 2 deletions src/analyzer/plugins/analyzerplugin.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include <QString>

#include "audio/frame.h"
#include "track/beats.h"
#include "track/keys.h"
#include "util/types.h"
Expand Down Expand Up @@ -71,8 +72,8 @@ class AnalyzerBeatsPlugin : public AnalyzerPlugin {
virtual float getBpm() const {
return 0.0f;
}
virtual QVector<double> getBeats() const {
return QVector<double>();
virtual QVector<mixxx::audio::FramePos> getBeats() const {
return {};
}
};

Expand Down
2 changes: 1 addition & 1 deletion src/analyzer/plugins/analyzerqueenmarybeats.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ bool AnalyzerQueenMaryBeats::finalize() {
for (size_t i = 0; i < beats.size(); ++i) {
// we add the halve m_stepSize here, because the beat
// is detected between the two samples.
double result = (beats.at(i) * m_stepSize) + m_stepSize / 2;
const auto result = mixxx::audio::FramePos((beats.at(i) * m_stepSize) + m_stepSize / 2);
Holzhaus marked this conversation as resolved.
Show resolved Hide resolved
m_resultBeats.push_back(result);
}

Expand Down
4 changes: 2 additions & 2 deletions src/analyzer/plugins/analyzerqueenmarybeats.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class AnalyzerQueenMaryBeats : public AnalyzerBeatsPlugin {
return true;
}

QVector<double> getBeats() const override {
QVector<mixxx::audio::FramePos> getBeats() const override {
return m_resultBeats;
}

Expand All @@ -51,7 +51,7 @@ class AnalyzerQueenMaryBeats : public AnalyzerBeatsPlugin {
int m_windowSize;
int m_stepSize;
std::vector<double> m_detectionResults;
QVector<double> m_resultBeats;
QVector<mixxx::audio::FramePos> m_resultBeats;
};

} // namespace mixxx
147 changes: 147 additions & 0 deletions src/audio/frame.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
#pragma once

#include <QDebug>
#include <cmath>
#include <limits>

#include "engine/engine.h"
#include "util/fpclassify.h"

namespace mixxx {
namespace audio {
/// FrameDiff_t can be used to store the difference in position between
/// two frames and to store the length of a segment of track in terms of frames.
typedef double FrameDiff_t;

/// FramePos defines the position of a frame in a track
/// with respect to a fixed origin, i.e. start of the track.
class FramePos final {
public:
typedef double value_t;
static constexpr value_t kStartValue = 0;
static constexpr value_t kInvalidValue = std::numeric_limits<FramePos::value_t>::quiet_NaN();

constexpr FramePos()
: m_framePosition(kInvalidValue) {
}

constexpr explicit FramePos(value_t framePosition)
: m_framePosition(framePosition) {
}

static constexpr FramePos fromEngineSamplePos(double engineSamplePos) {
return FramePos(engineSamplePos / mixxx::kEngineChannelCount);
}

constexpr double toEngineSamplePos() const {
return value() * mixxx::kEngineChannelCount;
}

bool isValid() const {
return !util_isnan(m_framePosition);
}

void setValue(value_t framePosition) {
m_framePosition = framePosition;
}

constexpr value_t value() const {
return m_framePosition;
}

bool isFractional() const {
value_t integerPart;
return std::modf(value(), &integerPart) != 0;
}

[[nodiscard]] FramePos toFullFrames() const {
Copy link
Member

Choose a reason for hiding this comment

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

From the function mame it is not clear if it is foor or ceil or round.
Let's either remove this function or rename it.
Do we need the other candidates as well?

Copy link
Member

Choose a reason for hiding this comment

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

Floor is not int cast.

Copy link
Contributor

Choose a reason for hiding this comment

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

When I mentioned the naming is not ideal I had something like toLowerFrameBoundary() in mind.

Copy link
Member Author

Choose a reason for hiding this comment

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

@uklotzde Done.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess that for the future we need a more sophisticated solution:

  • If the value is close (< epsilon) to a beat then use std::round
  • Otherwise use either the lower or upper boundary, depending on the use case.

The value of epsilon may also vary.

return FramePos(std::floor(value()));
}

FramePos& operator+=(FrameDiff_t increment) {
m_framePosition += increment;
return *this;
}

FramePos& operator-=(FrameDiff_t decrement) {
m_framePosition -= decrement;
return *this;
}

FramePos& operator*=(double multiple) {
m_framePosition *= multiple;
return *this;
}

FramePos& operator/=(double divisor) {
m_framePosition /= divisor;
return *this;
}

private:
value_t m_framePosition;
};

/// FramePos can be added to a FrameDiff_t
inline FramePos operator+(FramePos framePos, FrameDiff_t frameDiff) {
return FramePos(framePos.value() + frameDiff);
}

/// FramePos can be subtracted from a FrameDiff_t
inline FramePos operator-(FramePos framePos, FrameDiff_t frameDiff) {
return FramePos(framePos.value() - frameDiff);
}

/// Two FramePos can be subtracted to get a FrameDiff_t
inline FrameDiff_t operator-(FramePos framePos1, FramePos framePos2) {
return framePos1.value() - framePos2.value();
}

// Adding two FramePos is not allowed since every FramePos shares a common
Holzhaus marked this conversation as resolved.
Show resolved Hide resolved
// reference or origin i.e. the start of the track.

/// FramePos can be multiplied or divided by a double
inline FramePos operator*(FramePos framePos, double multiple) {
return FramePos(framePos.value() * multiple);
}

inline FramePos operator/(FramePos framePos, double divisor) {
return FramePos(framePos.value() / divisor);
}

inline bool operator<(FramePos frame1, FramePos frame2) {
return frame1.value() < frame2.value();
}

inline bool operator<=(FramePos frame1, FramePos frame2) {
return frame1.value() <= frame2.value();
}

inline bool operator>(FramePos frame1, FramePos frame2) {
return frame1.value() > frame2.value();
}

inline bool operator>=(FramePos frame1, FramePos frame2) {
return frame1.value() >= frame2.value();
}

inline bool operator==(FramePos frame1, FramePos frame2) {
return frame1.value() == frame2.value();
}

inline bool operator!=(FramePos frame1, FramePos frame2) {
return !(frame1.value() == frame2.value());
}

inline QDebug operator<<(QDebug dbg, FramePos arg) {
dbg << arg.value();
return dbg;
}

constexpr FramePos kInvalidFramePos = FramePos(FramePos::kInvalidValue);
constexpr FramePos kStartFramePos = FramePos(FramePos::kStartValue);
} // namespace audio
} // namespace mixxx

Q_DECLARE_TYPEINFO(mixxx::audio::FramePos, Q_MOVABLE_TYPE);
Q_DECLARE_METATYPE(mixxx::audio::FramePos);
3 changes: 2 additions & 1 deletion src/library/dao/trackdao.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1255,7 +1255,8 @@ bool setTrackBeats(const QSqlRecord& record, const int column,
}
} else {
// Load a temorary beat grid without offset that will be replaced by the analyzer.
const auto pBeats = BeatFactory::makeBeatGrid(pTrack->getSampleRate(), bpm, 0.0);
const auto pBeats = BeatFactory::makeBeatGrid(
pTrack->getSampleRate(), bpm, mixxx::audio::kStartFramePos);
pTrack->trySetBeats(pBeats);
}
return false;
Expand Down
11 changes: 8 additions & 3 deletions src/library/dlgtrackinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -581,8 +581,11 @@ void DlgTrackInfo::slotBpmConstChanged(int state) {
// The cue point should be set on a beat, so this seems
// to be a good alternative
CuePosition cue = m_pLoadedTrack->getCuePoint();
m_pBeatsClone = BeatFactory::makeBeatGrid(
m_pLoadedTrack->getSampleRate(), spinBpm->value(), cue.getPosition());
m_pBeatsClone =
BeatFactory::makeBeatGrid(m_pLoadedTrack->getSampleRate(),
spinBpm->value(),
mixxx::audio::FramePos::fromEngineSamplePos(
cue.getPosition()));
} else {
m_pBeatsClone.clear();
}
Expand Down Expand Up @@ -618,7 +621,9 @@ void DlgTrackInfo::slotSpinBpmValueChanged(double value) {
if (!m_pBeatsClone) {
CuePosition cue = m_pLoadedTrack->getCuePoint();
m_pBeatsClone = BeatFactory::makeBeatGrid(
m_pLoadedTrack->getSampleRate(), value, cue.getPosition());
m_pLoadedTrack->getSampleRate(),
value,
mixxx::audio::FramePos::fromEngineSamplePos(cue.getPosition()));
}

double oldValue = m_pBeatsClone->getBpm();
Expand Down
4 changes: 2 additions & 2 deletions src/library/rekordbox/rekordboxfeature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -925,7 +925,7 @@ void readAnalyze(TrackPointer track,
static_cast<rekordbox_anlz_t::beat_grid_tag_t*>(
(*section)->body());

QVector<double> beats;
QVector<mixxx::audio::FramePos> beats;

for (std::vector<rekordbox_anlz_t::beat_grid_beat_t*>::iterator
beat = beatGridTag->beats()->begin();
Expand All @@ -936,7 +936,7 @@ void readAnalyze(TrackPointer track,
if (time < 1) {
time = 1;
}
beats << (sampleRateKhz * static_cast<double>(time));
beats << mixxx::audio::FramePos(sampleRateKhz * static_cast<double>(time));
}

const auto pBeats = mixxx::BeatMap::makeBeatMap(
Expand Down
2 changes: 2 additions & 0 deletions src/mixxxapplication.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <QTouchEvent>
#include <QtDebug>

#include "audio/frame.h"
#include "audio/types.h"
#include "control/controlproxy.h"
#include "library/trackset/crate/crateid.h"
Expand Down Expand Up @@ -97,6 +98,7 @@ void MixxxApplication::registerMetaTypes() {
qRegisterMetaType<mixxx::cache_key_t>("mixxx::cache_key_t");
qRegisterMetaType<mixxx::Bpm>("mixxx::Bpm");
qRegisterMetaType<mixxx::Duration>("mixxx::Duration");
qRegisterMetaType<mixxx::audio::FramePos>("mixxx::audio::FramePos");
qRegisterMetaType<std::optional<mixxx::RgbColor>>("std::optional<mixxx::RgbColor>");
qRegisterMetaType<mixxx::FileInfo>("mixxx::FileInfo");
}
Expand Down
25 changes: 20 additions & 5 deletions src/test/beatgridtest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ TEST(BeatGridTest, Scale) {
double bpm = 60.0;
pTrack->trySetBpm(bpm);

auto pGrid = BeatGrid::makeBeatGrid(pTrack->getSampleRate(), QString(), bpm, 0);
auto pGrid = BeatGrid::makeBeatGrid(pTrack->getSampleRate(),
QString(),
bpm,
mixxx::audio::kStartFramePos);

EXPECT_DOUBLE_EQ(bpm, pGrid->getBpm());
pGrid = pGrid->scale(Beats::DOUBLE);
Expand Down Expand Up @@ -60,7 +63,10 @@ TEST(BeatGridTest, TestNthBeatWhenOnBeat) {
pTrack->trySetBpm(bpm);
double beatLength = (60.0 * sampleRate / bpm) * kFrameSize;

auto pGrid = BeatGrid::makeBeatGrid(pTrack->getSampleRate(), QString(), bpm, 0);
auto pGrid = BeatGrid::makeBeatGrid(pTrack->getSampleRate(),
QString(),
bpm,
mixxx::audio::kStartFramePos);
// Pretend we're on the 20th beat;
double position = beatLength * 20;

Expand Down Expand Up @@ -99,7 +105,10 @@ TEST(BeatGridTest, TestNthBeatWhenOnBeat_BeforeEpsilon) {
pTrack->trySetBpm(bpm);
double beatLength = (60.0 * sampleRate / bpm) * kFrameSize;

auto pGrid = BeatGrid::makeBeatGrid(pTrack->getSampleRate(), QString(), bpm, 0);
auto pGrid = BeatGrid::makeBeatGrid(pTrack->getSampleRate(),
QString(),
bpm,
mixxx::audio::kStartFramePos);

// Pretend we're just before the 20th beat.
const double kClosestBeat = 20 * beatLength;
Expand Down Expand Up @@ -140,7 +149,10 @@ TEST(BeatGridTest, TestNthBeatWhenOnBeat_AfterEpsilon) {
pTrack->trySetBpm(bpm);
double beatLength = (60.0 * sampleRate / bpm) * kFrameSize;

auto pGrid = BeatGrid::makeBeatGrid(pTrack->getSampleRate(), QString(), bpm, 0);
auto pGrid = BeatGrid::makeBeatGrid(pTrack->getSampleRate(),
QString(),
bpm,
mixxx::audio::kStartFramePos);

// Pretend we're just before the 20th beat.
const double kClosestBeat = 20 * beatLength;
Expand Down Expand Up @@ -181,7 +193,10 @@ TEST(BeatGridTest, TestNthBeatWhenNotOnBeat) {
pTrack->trySetBpm(bpm);
double beatLength = (60.0 * sampleRate / bpm) * kFrameSize;

auto pGrid = BeatGrid::makeBeatGrid(pTrack->getSampleRate(), QString(), bpm, 0);
auto pGrid = BeatGrid::makeBeatGrid(pTrack->getSampleRate(),
QString(),
bpm,
mixxx::audio::kStartFramePos);

// Pretend we're half way between the 20th and 21st beat
double previousBeat = beatLength * 20.0;
Expand Down
Loading