Skip to content

Commit

Permalink
Merge pull request #4510 from Holzhaus/newbeats-lastmarker
Browse files Browse the repository at this point in the history 
Beats: Use "last marker" term instead of "end marker"
  • Loading branch information
@ywwg
ywwg authored Nov 11, 2021
2 parents 21dacd6 + 741c8e3 commit f981c8e
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 68 deletions.
4 changes: 2 additions & 2 deletions src/test/beatstest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -446,8 +446,8 @@ TEST(BeatsTest, NonConstTempoFromBeatPositions) {
EXPECT_EQ(16, markers[2].beatsTillNextMarker());

EXPECT_DOUBLE_EQ((kStartPosition + 48 * beatLengthFrames).value(),
pBeats->getEndMarkerPosition().value());
EXPECT_EQ(kBpm * 2, pBeats->getEndMarkerBpm());
pBeats->getLastMarkerPosition().value());
EXPECT_EQ(kBpm * 2, pBeats->getLastMarkerBpm());
}

TEST(BeatsTest, ConstTempoFindNthBeatWhenOnBeat) {
Expand Down
66 changes: 33 additions & 33 deletions src/track/beats.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@ namespace mixxx {

mixxx::audio::FrameDiff_t Beats::ConstIterator::beatLengthFrames() const {
if (m_it == m_beats->m_markers.cend()) {
return m_beats->endBeatLengthFrames();
return m_beats->lastBeatLengthFrames();
}

const auto nextMarker = std::next(m_it);
const mixxx::audio::FramePos nextMarkerPosition =
(nextMarker != m_beats->m_markers.cend())
? nextMarker->position()
: m_beats->m_endMarkerPosition;
: m_beats->m_lastMarkerPosition;
return (nextMarkerPosition - m_it->position()) / m_it->beatsTillNextMarker();
}

Expand Down Expand Up @@ -147,21 +147,21 @@ Beats::ConstIterator::difference_type Beats::ConstIterator::operator-(
void Beats::ConstIterator::updateValue() {
const auto position = (m_it != m_beats->m_markers.cend())
? m_it->position()
: m_beats->m_endMarkerPosition;
: m_beats->m_lastMarkerPosition;
m_value = position + m_beatOffset * beatLengthFrames();
}

// static
mixxx::BeatsPointer Beats::fromConstTempo(
mixxx::audio::SampleRate sampleRate,
mixxx::audio::FramePos endMarkerPosition,
mixxx::Bpm endMarkerBpm,
mixxx::audio::FramePos lastMarkerPosition,
mixxx::Bpm lastMarkerBpm,
const QString& subVersion) {
VERIFY_OR_DEBUG_ASSERT(sampleRate.isValid() &&
endMarkerPosition.isValid() && endMarkerBpm.isValid()) {
lastMarkerPosition.isValid() && lastMarkerBpm.isValid()) {
return nullptr;
}
return BeatsPointer(new Beats({}, endMarkerPosition, endMarkerBpm, sampleRate, subVersion));
return BeatsPointer(new Beats({}, lastMarkerPosition, lastMarkerBpm, sampleRate, subVersion));
}

// static
Expand Down Expand Up @@ -232,12 +232,12 @@ mixxx::BeatsPointer Beats::fromBeatPositions(
mixxx::BeatsPointer Beats::fromBeatMarkers(
audio::SampleRate sampleRate,
const std::vector<BeatMarker>& markers,
const audio::FramePos endMarkerPosition,
const Bpm endMarkerBpm,
const audio::FramePos lastMarkerPosition,
const Bpm lastMarkerBpm,
const QString& subVersion) {
return BeatsPointer(new Beats(markers,
endMarkerPosition,
endMarkerBpm,
lastMarkerPosition,
lastMarkerBpm,
sampleRate,
subVersion));
}
Expand Down Expand Up @@ -342,8 +342,8 @@ QByteArray Beats::toBeatGridByteArray() const {
mixxx::track::io::BeatGrid grid;
grid.mutable_first_beat()->set_frame_position(
static_cast<google::protobuf::int32>(
m_endMarkerPosition.toLowerFrameBoundary().value()));
grid.mutable_bpm()->set_bpm(m_endMarkerBpm.value());
m_lastMarkerPosition.toLowerFrameBoundary().value()));
grid.mutable_bpm()->set_bpm(m_lastMarkerBpm.value());

std::string output;
grid.SerializeToString(&output);
Expand Down Expand Up @@ -416,10 +416,10 @@ bool Beats::findPrevNextBeats(audio::FramePos position,
Beats::ConstIterator Beats::iteratorFrom(audio::FramePos position) const {
DEBUG_ASSERT(isValid());
auto it = cfirstmarker();
if (position > m_endMarkerPosition) {
DEBUG_ASSERT(*clastmarker() == m_endMarkerPosition);
if (position > m_lastMarkerPosition) {
DEBUG_ASSERT(*clastmarker() == m_lastMarkerPosition);
// Lookup position is after the last marker position
const double n = std::ceil((position - m_endMarkerPosition) / endBeatLengthFrames());
const double n = std::ceil((position - m_lastMarkerPosition) / lastBeatLengthFrames());
if (n >= static_cast<double>(std::numeric_limits<int>::max())) {
return cend();
}
Expand Down Expand Up @@ -509,19 +509,19 @@ mixxx::Bpm Beats::getBpmInRange(audio::FramePos startPosition, audio::FramePos e
return {};
}

if (m_markers.empty() || startPosition >= m_endMarkerPosition) {
return m_endMarkerBpm;
if (m_markers.empty() || startPosition >= m_lastMarkerPosition) {
return m_lastMarkerBpm;
}

std::unordered_map<int, audio::FrameDiff_t> map;

auto markerIt = m_markers.crbegin();
auto nextMarkerPosition = m_endMarkerPosition;
auto nextMarkerPosition = m_lastMarkerPosition;

if (endPosition > m_endMarkerPosition) {
DEBUG_ASSERT(startPosition < m_endMarkerPosition);
const auto key = static_cast<int>(std::round(m_endMarkerBpm.value() * 100));
map.emplace(key, endPosition - m_endMarkerPosition);
if (endPosition > m_lastMarkerPosition) {
DEBUG_ASSERT(startPosition < m_lastMarkerPosition);
const auto key = static_cast<int>(std::round(m_lastMarkerBpm.value() * 100));
map.emplace(key, endPosition - m_lastMarkerPosition);
}

while (markerIt != m_markers.crend() && nextMarkerPosition > startPosition) {
Expand Down Expand Up @@ -571,7 +571,7 @@ mixxx::Bpm Beats::getBpmInRange(audio::FramePos startPosition, audio::FramePos e

mixxx::Bpm Beats::getBpmAroundPosition(audio::FramePos position, int n) const {
if (m_markers.empty()) {
return m_endMarkerBpm;
return m_lastMarkerBpm;
}

auto it = iteratorFrom(position);
Expand Down Expand Up @@ -603,10 +603,10 @@ std::optional<BeatsPointer> Beats::tryTranslate(audio::FrameDiff_t offsetFrames)
marker.beatsTillNextMarker());
});

const auto endMarkerPosition = m_endMarkerPosition + offsetFrames;
const auto lastMarkerPosition = m_lastMarkerPosition + offsetFrames;
return BeatsPointer(new Beats(markers,
endMarkerPosition.toLowerFrameBoundary(),
m_endMarkerBpm,
lastMarkerPosition.toLowerFrameBoundary(),
m_lastMarkerBpm,
m_sampleRate,
m_subVersion));
}
Expand Down Expand Up @@ -652,11 +652,11 @@ std::optional<BeatsPointer> Beats::tryScale(BpmScale scale) const {
markers.push_back({marker.position(), beatsTillNextMarker});
}

Bpm endMarkerBpm = m_endMarkerBpm * scaleFactor;
Bpm lastMarkerBpm = m_lastMarkerBpm * scaleFactor;

return BeatsPointer(new Beats(markers,
m_endMarkerPosition,
endMarkerBpm,
m_lastMarkerPosition,
lastMarkerBpm,
m_sampleRate,
m_subVersion));
}
Expand All @@ -667,7 +667,7 @@ std::optional<BeatsPointer> Beats::trySetBpm(mixxx::Bpm bpm) const {
}

bool Beats::isValid() const {
if (!m_endMarkerPosition.isValid() || !m_endMarkerBpm.isValid()) {
if (!m_lastMarkerPosition.isValid() || !m_lastMarkerBpm.isValid()) {
return false;
}

Expand All @@ -685,8 +685,8 @@ mixxx::audio::FrameDiff_t Beats::firstBeatLengthFrames() const {
return it.beatLengthFrames();
}

mixxx::audio::FrameDiff_t Beats::endBeatLengthFrames() const {
return 60.0 * m_sampleRate / m_endMarkerBpm.value();
mixxx::audio::FrameDiff_t Beats::lastBeatLengthFrames() const {
return 60.0 * m_sampleRate / m_lastMarkerBpm.value();
}

audio::FramePos Beats::snapPosToNearBeat(audio::FramePos position) const {
Expand Down
58 changes: 33 additions & 25 deletions src/track/beats.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ namespace mixxx {
class Beats;
typedef std::shared_ptr<const Beats> BeatsPointer;

/// A beat marker is denotes the border of a tempo section inside a track.
class BeatMarker {
public:
BeatMarker(mixxx::audio::FramePos position, int beatsTillNextMarker)
Expand Down Expand Up @@ -52,9 +53,15 @@ inline bool operator==(const BeatMarker& lhs, const BeatMarker& rhs) {
inline bool operator!=(const BeatMarker& lhs, const BeatMarker& rhs) {
return !(lhs == rhs);
}
/// Beats is the base class for BPM and beat management classes. It provides a
/// specification of all methods a beat-manager class must provide, as well as
/// a capability model for representing optional features.

/// This class represents the beats of a track.
///
/// Internally, it uses the following data structure:
/// - 0 - N beat markers, followed by
/// - exactly one tempo marker ("last marker").
///
/// If the track has a constant tempo, there are 0 beat markers, and the last
/// marker is positioned at the first downbeat and is set to the tracks BPM.
///
/// All instances of this class are supposed to be managed by std::shared_ptr!
class Beats : private std::enable_shared_from_this<Beats> {
Expand Down Expand Up @@ -145,28 +152,28 @@ class Beats : private std::enable_shared_from_this<Beats> {
};

Beats(std::vector<BeatMarker> markers,
mixxx::audio::FramePos endMarkerPosition,
mixxx::Bpm endMarkerBpm,
mixxx::audio::FramePos lastMarkerPosition,
mixxx::Bpm lastMarkerBpm,
mixxx::audio::SampleRate sampleRate,
const QString& subVersion)
: m_markers(std::move(markers)),
m_endMarkerPosition(endMarkerPosition),
m_endMarkerBpm(endMarkerBpm),
m_lastMarkerPosition(lastMarkerPosition),
m_lastMarkerBpm(lastMarkerBpm),
m_sampleRate(sampleRate),
m_subVersion(subVersion) {
DEBUG_ASSERT(m_endMarkerPosition.isValid());
DEBUG_ASSERT(!m_endMarkerPosition.isFractional());
DEBUG_ASSERT(m_endMarkerBpm.isValid());
DEBUG_ASSERT(m_lastMarkerPosition.isValid());
DEBUG_ASSERT(!m_lastMarkerPosition.isFractional());
DEBUG_ASSERT(m_lastMarkerBpm.isValid());
DEBUG_ASSERT(m_sampleRate.isValid());
}

Beats(mixxx::audio::FramePos endMarkerPosition,
mixxx::Bpm endMarkerBpm,
Beats(mixxx::audio::FramePos lastMarkerPosition,
mixxx::Bpm lastMarkerBpm,
mixxx::audio::SampleRate sampleRate,
const QString& subVersion)
: Beats(std::vector<BeatMarker>(),
endMarkerPosition,
endMarkerBpm,
lastMarkerPosition,
lastMarkerBpm,
sampleRate,
subVersion) {
}
Expand Down Expand Up @@ -206,8 +213,8 @@ class Beats : private std::enable_shared_from_this<Beats> {

friend bool operator==(const Beats& lhs, const Beats& rhs) {
return lhs.m_markers == rhs.m_markers &&
lhs.m_endMarkerPosition == rhs.m_endMarkerPosition &&
lhs.m_endMarkerBpm == rhs.m_endMarkerBpm && lhs.m_sampleRate &&
lhs.m_lastMarkerPosition == rhs.m_lastMarkerPosition &&
lhs.m_lastMarkerBpm == rhs.m_lastMarkerBpm && lhs.m_sampleRate &&
rhs.m_sampleRate;
}

Expand Down Expand Up @@ -250,8 +257,8 @@ class Beats : private std::enable_shared_from_this<Beats> {
static mixxx::BeatsPointer fromBeatMarkers(
audio::SampleRate sampleRate,
const std::vector<BeatMarker>& beatMarker,
const audio::FramePos endMarkerPosition,
const Bpm endMarkerBpm,
const audio::FramePos lastMarkerPosition,
const Bpm lastMarkerBpm,
const QString& subVersion = QString());

enum class BpmScale {
Expand Down Expand Up @@ -368,11 +375,12 @@ class Beats : private std::enable_shared_from_this<Beats> {
const std::vector<BeatMarker>& getMarkers() const {
return m_markers;
}
mixxx::audio::FramePos getEndMarkerPosition() const {
return m_endMarkerPosition;

mixxx::audio::FramePos getLastMarkerPosition() const {
return m_lastMarkerPosition;
}
mixxx::Bpm getEndMarkerBpm() const {
return m_endMarkerBpm;
mixxx::Bpm getLastMarkerBpm() const {
return m_lastMarkerBpm;
}

////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -417,11 +425,11 @@ class Beats : private std::enable_shared_from_this<Beats> {
QByteArray toBeatMapByteArray() const;

mixxx::audio::FrameDiff_t firstBeatLengthFrames() const;
mixxx::audio::FrameDiff_t endBeatLengthFrames() const;
mixxx::audio::FrameDiff_t lastBeatLengthFrames() const;

std::vector<BeatMarker> m_markers;
mixxx::audio::FramePos m_endMarkerPosition;
mixxx::Bpm m_endMarkerBpm;
mixxx::audio::FramePos m_lastMarkerPosition;
mixxx::Bpm m_lastMarkerBpm;
mixxx::audio::SampleRate m_sampleRate;

// The sub-version of this beatgrid.
Expand Down
8 changes: 4 additions & 4 deletions src/track/serato/beatgrid.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -435,14 +435,14 @@ void SeratoBeatGrid::setBeats(BeatsPointer pBeats,
const auto markers = pBeats->getMarkers();
QList<SeratoBeatGridNonTerminalMarkerPointer> nonTerminalMarkers;

const float bpm = static_cast<float>(pBeats->getEndMarkerBpm().value());
const float bpm = static_cast<float>(pBeats->getLastMarkerBpm().value());

if (markers.size() == 1) {
const auto& marker = markers.back();
const auto lastBeatLengthFrames = 60.0 * pBeats->getSampleRate() /
pBeats->getEndMarkerBpm().value();
pBeats->getLastMarkerBpm().value();
const auto previousBeatLengthFrames =
(pBeats->getEndMarkerPosition() - marker.position()) /
(pBeats->getLastMarkerPosition() - marker.position()) /
marker.beatsTillNextMarker();
// If the following condition holds true, the marker only exists for backwards compatibility with the legacy beatgrid format.
//
Expand Down Expand Up @@ -474,7 +474,7 @@ void SeratoBeatGrid::setBeats(BeatsPointer pBeats,

const float positionSecs =
static_cast<float>(signalInfo.frames2secsFractional(
pBeats->getEndMarkerPosition().value()) -
pBeats->getLastMarkerPosition().value()) -
timingOffsetSecs);

setTerminalMarker(std::make_shared<SeratoBeatGridTerminalMarker>(positionSecs, bpm));
Expand Down
8 changes: 4 additions & 4 deletions src/track/serato/beatsimporter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,19 +59,19 @@ BeatsPointer SeratoBeatsImporter::importBeatsAndApplyTimingOffset(
markers.push_back(BeatMarker{position.toLowerFrameBoundary(), beatsTillNextMarker});
}

const auto endMarkerPosition = audio::FramePos(
const auto lastMarkerPosition = audio::FramePos(
signalInfo.secs2frames(m_pTerminalMarker->positionSecs()) +
timingOffsetFrames);
const auto endMarkerBpm = Bpm(m_pTerminalMarker->bpm());
const auto lastMarkerBpm = Bpm(m_pTerminalMarker->bpm());

m_nonTerminalMarkers.clear();
m_pTerminalMarker.reset();

return Beats::fromBeatMarkers(
signalInfo.getSampleRate(),
std::move(markers),
endMarkerPosition.toLowerFrameBoundary(),
endMarkerBpm);
lastMarkerPosition.toLowerFrameBoundary(),
lastMarkerBpm);
}

} // namespace mixxx

0 comments on commit f981c8e

Please sign in to comment.