Mastersyc double / half Bpm Fix. by daschuer · Pull Request #6 · ywwg/mixxx

daschuer · 2020-04-21T00:44:31Z

This fixes some nasty double / half Bpm situations.

Please have a look at src/test/enginesynctest.cpp first.
The tests partly tests the old behavior and the code had workarounds to make the test still succeed.

I have adjusted the test to the new assumptions and cleaned up the code accordingly.

The main assumption are:

A master never changes speed due to other decks.
If I enable sync on one deck, only this deck changes tempo.

This works now with playing and paused decks in the same way.

I have also re-enabed the explicit master feature. The state is always accepted for a deck and keept in standby until the deck has a valid tempo. As we know bad things kan happen in the head of the night with it. But since it is only accessible by special mappings, it is OK for me.

…ect calls

…rack pbm nor batgrids

ywwg

I tried to build this and got:

/home/owen/src/github/mixxx/src/mixer/basetrackplayer.cpp: In member function ‘void BaseTrackPlayerImpl::connectLoadedTrack()’:
/home/owen/src/github/mixxx/src/mixer/basetrackplayer.cpp:289:31: error: no matching function for call to ‘BaseTrackPlayerImpl::connect(std::__shared_ptr<Track, __gnu_cxx::_S_atomic>::element_type*, void (Track::*)(double), std::unique_ptr<ControlObject>::pointer, void (ControlProxy::*)(double))’
  289 |             &ControlProxy::set);

ywwg · 2020-04-21T17:02:49Z

          m_mode(SYNC_NONE),
          m_iOldSampleRate(44100),
          m_dOldBpm(124.0),
+          m_baseBpm(124.0),


for consistency this should be m_dBaseBpm

ywwg · 2020-04-21T17:03:27Z

-
-void InternalClock::setMasterBaseBpm(double bpm) {
-    Q_UNUSED(bpm)
+    qDebug() << "InternalClock::getBaseBpm()" << m_baseBpm;


make sure to remove debug comment or put them behind a conditional (preferred)

Depending on the event different slots of bpmconro and synccontrol where called in different order. This leads to various hard to tack issues because the cal heriarchy is unclear when reading signal slot connections. Now bpmconrol is always updated before synccontrol and we onbly look to Beats. Loading a track and changing beats are treated the same.

…ter feature for the internal clock.

This avoids a tempo shift when the internal clock takes over from a perviouse master with half or double matched bpm.

daschuer · 2020-04-21T21:03:21Z

Ups, that is a rebase issue, because I cant build the final branch anymore because I am on Xenial.
I need to rebase again to not produce failing commits.

daschuer · 2020-04-21T21:44:38Z

Done. I hope it builds now.

ywwg · 2020-04-22T19:49:29Z

          m_iOldSampleRate(44100),
          m_dOldBpm(124.0),
-          m_baseBpm(124.0),
+          m_dbaseBpm(124.0),


please fix capitalization: m_dBaseBpm

ywwg · 2020-04-22T19:49:58Z

it builds now! And works. thanks! Just a couple small nits and then I'll merge into my branch

daschuer · 2020-04-24T17:48:14Z

Done.

Hercules DJControl Inpulse 200: Fix some code stye issues

Ddj400

…h sync When loading a track that is not yet present in the library (and thus doesn't have any BPM because it hasn't been analyzed yet) while another deck is playing and both decks have sync enabled, a debug assertion is triggered: DEBUG ASSERT: "isValid()" in function double mixxx::Bpm::value() const at src/track/bpm.h:53 Aborted (core dumped) The backtrace looks as follows: #0 0x00007f175c87234c in __pthread_kill_implementation () at /usr/lib/libc.so.6 #1 0x00007f175c8254b8 in raise () at /usr/lib/libc.so.6 #2 0x00007f175c80f534 in abort () at /usr/lib/libc.so.6 #3 0x00007f175cf05ee4 in qt_assert(char const*, char const*, int) () at /usr/lib/libQt5Core.so.5 #4 0x000055deb2e67e1c in mixxx::(anonymous namespace)::handleMessage(QtMsgType, QMessageLogContext const&, QString const&) (type=<optimized out>, context=<optimized out>, input=<optimized out>) at src/util/logging.cpp:355 #5 0x00007f175cf47128 in () at /usr/lib/libQt5Core.so.5 #6 0x00007f175cf3fd8a in () at /usr/lib/libQt5Core.so.5 #7 0x00007f175cf06526 in QMessageLogger::critical(char const*, ...) const () at /usr/lib/libQt5Core.so.5 #8 0x000055deb2e5c720 in mixxx_debug_assert(char const*, char const*, int, char const*) (assertion=assertion@entry=0x55deb39bd0db "isValid()", file=file@entry=0x55deb39bbf30 "src/track/bpm.h", line=line@entry=53, function=function@entry=0x55deb39bbf08 "double mixxx::Bpm::value() const") at gsrc/util/assert.h:9 #9 0x000055deb2ee7e7e in mixxx_debug_assert_return_true(char const*, char const*, int, char const*) (function=0x55deb39bbf08 "double mixxx::Bpm::value() const", line=53, file=0x55deb39bbf30 "src/track/bpm.h", assertion=0x55deb39bd0db "isValid()") at gsrc/util/assert.h:18 #10 mixxx::Bpm::value() const (this=<synthetic pointer>) at src/track/bpm.h:53 #11 mixxx::operator*(mixxx::Bpm, double) (multiple=1, bpm=...) at src/track/bpm.h:160 #12 SyncControl::setLocalBpm(mixxx::Bpm) (this=<optimized out>, localBpm=...) at src/engine/sync/synccontrol.cpp:567 #13 0x000055deb34c7ba3 in EngineBuffer::postProcess(int) (this=0x55deb56eb060, iBufferSize=2048) at src/engine/enginebuffer.cpp:1318 #14 0x000055deb3139023 in EngineMaster::processChannels(int) (this=0x55deb5449440, iBufferSize=<optimized out>) at src/engine/enginemaster.cpp:383 mixxxdj#15 0x000055deb31394f7 in EngineMaster::process(int) (this=0x55deb5449440, iBufferSize=iBufferSize@entry=2048) at src/engine/enginemaster.cpp:410 mixxxdj#16 0x000055deb2f91d0b in SoundManager::onDeviceOutputCallback(long) (this=<optimized out>, iFramesPerBuffer=iFramesPerBuffer@entry=1024) at src/soundio/soundmanager.cpp:596 mixxxdj#17 0x000055deb32dd794 in SoundDevicePortAudio::callbackProcessClkRef(long, float*, float const*, PaStreamCallbackTimeInfo const*, unsigned long) (this=0x55deb553e6b0, framesPerBuffer=1024, out=<optimized out>, in=<optimized out>, timeInfo=<optimized out>, statusFlags=<optimized out>) at src/soundio/sounddeviceportaudio.cpp:965 This happens because `newLocalBpm` is invalid when `localBpm` is invalid. Trying to do sync decks while no tempo information is available does not make sense, so we only synchronize decks if the local BPM is available.

Silence some float conversion warnings by making conversion explicit

VersionStore: Remove Apple fallback

daschuer added 11 commits April 21, 2020 02:30

Simplify EngineSync::pickMaster()

3352009

Fix ensure only the sync deck is adjusted when pressing sync

3ffb73e

Remove unused playingSyncDeckCount

a0dcf44

Introduces common trackBeatsUpdated function to reduce number of conn…

090f38d

…ect calls

BpmControl: override trackBeatsUpdated()

d65756a

ClockControl: override trackBeatsUpdated()

e40801f

CueControl: override trackBeatsUpdated()

64ff4f4

LoopingControl: override trackBeatsUpdated()

0faf8f3

QuantizeControl: override trackBeatsUpdated()

dba37a0

SyncControl: treat a changed beat grid like a changed track

4bf53fb

Don't use file_bmp during EngineSyncTests. This does not change the t…

4b17d2e

…rack pbm nor batgrids

daschuer mentioned this pull request Apr 21, 2020

Rework master sync internal clock behavior mixxxdj/mixxx#2376

Merged

ywwg requested changes Apr 21, 2020

View reviewed changes

daschuer added 8 commits April 21, 2020 22:38

Get rid of legacy EngineSync::notifyTrackLoaded

2bceb45

Make use of pickMaster() whenever calculate the current master

15e8ae6

sync_master CO means always explicite master. Re-anable explicite mas…

afb1c3c

…ter feature for the internal clock.

Added a failing tests showing the double bpm issue

c4d3a0d

Fix double bpm after syncing a track with low a low bpm valud

9419fdb

get rid of setMasterBaseBpm()

5a26aca

Make the internal clock aware of the baseBpm of the previous master

33097fb

This avoids a tempo shift when the internal clock takes over from a perviouse master with half or double matched bpm.

daschuer added 2 commits April 21, 2020 23:41

Added test for explicite master with and without loaded beats

e8e7af3

Use consistent naming

2c1b8e6

daschuer force-pushed the mastersyc_onemaster2 branch from 5fe1045 to 2c1b8e6 Compare April 21, 2020 21:43

ywwg reviewed Apr 22, 2020

View reviewed changes

Fix capitalizing of m_dBaseBpm

7b88861

ywwg merged commit b0e0f37 into ywwg:mastersync-onemaster Apr 26, 2020

ywwg pushed a commit that referenced this pull request Jun 17, 2020

Merge pull request #6 from Holzhaus/Hercules-Inpulse-200

276da56

Hercules DJControl Inpulse 200: Fix some code stye issues

ywwg pushed a commit that referenced this pull request Feb 6, 2021

Merge pull request #6 from dj3730/ddj400

a5b5d85

Ddj400

daschuer deleted the mastersyc_onemaster2 branch September 26, 2021 17:37

mixxxbot mentioned this pull request Aug 22, 2022

sync working in erratic way mixxxdj/mixxx#8512

Closed

ywwg pushed a commit that referenced this pull request Jun 12, 2023

Merge pull request #6 from ywwg/build-with-qt6-optional-qml

3137224

Silence some float conversion warnings by making conversion explicit

ywwg pushed a commit that referenced this pull request Feb 9, 2024

Merge pull request #6 from fwcd/fixversionstore

cdf87b9

VersionStore: Remove Apple fallback

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mastersyc double / half Bpm Fix. #6

Mastersyc double / half Bpm Fix. #6
ywwg merged 22 commits into
ywwg:mastersync-onemasterfrom
daschuer:mastersyc_onemaster2

daschuer commented Apr 21, 2020

Uh oh!

ywwg left a comment

Uh oh!

ywwg Apr 21, 2020

Uh oh!

ywwg Apr 21, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

daschuer commented Apr 21, 2020

Uh oh!

daschuer commented Apr 21, 2020

Uh oh!

ywwg Apr 22, 2020

Uh oh!

ywwg commented Apr 22, 2020 •

edited

Loading

Uh oh!

daschuer commented Apr 24, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

daschuer commented Apr 21, 2020

Uh oh!

ywwg left a comment

Choose a reason for hiding this comment

Uh oh!

ywwg Apr 21, 2020

Choose a reason for hiding this comment

Uh oh!

ywwg Apr 21, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

daschuer commented Apr 21, 2020

Uh oh!

daschuer commented Apr 21, 2020

Uh oh!

ywwg Apr 22, 2020

Choose a reason for hiding this comment

Uh oh!

ywwg commented Apr 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daschuer commented Apr 24, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ywwg commented Apr 22, 2020 •

edited

Loading