master sync with non-const bpm and quantize isn't calculating beat percentage correctly

[Owen Williams]

This is with 2.2.

* enable master sync and quantize on two decks
* Start with a track that is constant beatgrid, and let it play
* Load a track with non-constant beat grid.
* press play on the second track

Expected:
The tracks should be in sync

Actual:
Sometimes the non-constant track hops to the middle of the beat on play.

This is without touching the jog wheel, which would set the user_tweak value. it might have something to do with hitting the cue button.

Turning off quantize, finding sync, and then turning it back on, sync still seems to be working correctly.

[Owen Williams (ywwg)]

This could be a symptom of the m_pBeats->findNthBeat(dThisPosition, -2) logic that a comment warns about...

[Owen Williams (ywwg)]

It is exactly this problem. If the DJ pushes "play" before the first beat, findNthBeat gets called at a place where there is no data for beat -2.

One workaround would be to have findNthBeat return an artificial value when it goes past the end of the list -- we calculate the bpm around the position of the last beat, and then use that to return a plausible position value for the requested location.

[Owen Williams (ywwg)]

hm, getbpmaround relies on findnthbeat returning -1 so it can know about invalid requests. I'll have to think more about how best to handle this case transparently. (unless we don't want a workaround solution?)

[Owen Williams (ywwg)]

proposed solution:

GetNthBeat is refactored to return the Beat proto object, and if the value is artificial the source of the beat will be set to a new enum of ARTIFICIAL. A wrapper called GetNthBeatPosition will be a simple wrapper that just returns the double position.

GetNthBeat would probably be private / protected, and other classes would call GetNthBeatPosition instead.

[Owen Williams (ywwg)]

(therefore, getbpmaround can note if the position is artificial and know that the beat was out of range)

[Owen Williams (ywwg)]

Alternatively, we can auto-generate an arbitrary number of beats (64?) off the beginning and the end of the track and then we don't need any code changes to fix this bug. However all existing tracks will have this bug

[Daniel Schürmann (daschuer)]

I think this is another aspect of the same bug: https://bugs.launchpad.net/mixxx/+bug/1405010

In many tracks I experience, that the fist beat is placed on some random noise, or the fist sound, that happens in the track. Sometimes the second beat is also effected.

A solution, that replaces the first two beats with the extrapolated beats from the following beats would help here a lot.

I have also the use case that I like to start the track in sync from the preroll. This would be also covered.

In this special case of syncing, maybe we should not use the current measure, but the next measure, or the current and the next.

I have recently fixed a bug that syncs two tracks with different tempo on the next beat, and not on the fraction of the current measure. I think similar behaviour is expected here as well.

[Owen Williams (ywwg)]

yes, correct.

I think we shouldn't try to work around the problem of the beatmap detection working badly on early beats by trying to guess from the next measure. Beatmaps should be editable, and if the first beats are wrong then the user should delete the incorrect beats and then the extrapolated beats would be correct.

In my glorious beatmap future, it should work like DAWs work where you can create bpm milestones that essentially say "from here on, the bpm is X." This means not every beat needs to be manually placed, they are interpolated from a series of these milestones. (The first marker is the exception, where it also extrapolates backwards). Nobody wants to place individual beat markers for every kick in their track.

(A migration path to this future would be to have the current beatmap detector mark each beat with one of these milestones, and users could delete ranges of beats and mixx would interpolate the results. Interpolated beats could be calculated on the fly, or stored in the beatmap store.)

For now I think we should start with trying to implement extrapolation in a robust way, and then work on beatmap editing.

[Daniel Schürmann (daschuer)]

Is the situation improved with:
https://github.com/mixxxdj/mixxx/pull/1716

[Owen Williams (ywwg)]

That PR does appear to fix the issue!

[Daniel Schürmann (daschuer)]

IS this one now fixed?

[Swiftb0y (swiftb0y)]

Mixxx now uses GitHub for bug tracking. This bug has been migrated to:
https://github.com/mixxxdj/mixxx/issues/9475