-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[WIP] Remove beatmap and beatgrid #2861
Closed
Closed
Changes from 1 commit
Commits
Show all changes
162 commits
Select commit
Hold shift + click to select a range
148e216
Improvement in proto for beats blob
JaviVilarroig 39a90b4
Added Signature class for storing track time signature information
JaviVilarroig dc8fbe7
Added Signature class
JaviVilarroig 9ee4695
Added Beats::getSignature
JaviVilarroig 05a4720
Merge branch 'bars_phrases_and_variable_BPM_in_beats_class' of https:…
JaviVilarroig 2d38e75
Fix repo
JaviVilarroig c1428f0
Merge remote-tracking branch 'upstream/master' into bars_phrases_and_…
JaviVilarroig 590be5d
Added Signature manipulation to BeatGrid.
JaviVilarroig b2c9eca
BeatGrid/BeatMap Signature support
JaviVilarroig 0084ddc
Removed forgotten qDebug unneeded trace
JaviVilarroig c8b8ae4
- Added button in Tango skin for setting bar beat
JaviVilarroig fa68e2d
Presentaion of Bar beats in Waveform for BeatMap.
JaviVilarroig 87cc607
Merge remote-tracking branch 'upstream/master' into bars_phrases_and_…
JaviVilarroig 01583e8
Adds null_signature and default_signature constants.
JaviVilarroig b533ac4
Rename Signature to TimeSignature in protobuff and wrapper class.
JaviVilarroig 72e030a
Fixes in TimeSignature class.
JaviVilarroig 9d652ca
Protect naked pointers in BpmControl.
JaviVilarroig 91c9930
BeatMap::SetBar to use the real Signature of the track to calculate the
JaviVilarroig df0f083
Fix issue with set bar button nor properly reacting to press an hover.
JaviVilarroig 26b358f
Multiple small fixes on Beats. Added default value to TimeSignature
JaviVilarroig f97cd1e
BeatGrid removal and BeatMap consolidation into Beats
JaviVilarroig 48ec71b
Forgoten update on cmake list for previous commit
JaviVilarroig 1b4a0d8
Merge commit 'f1e13f93ad35dc1de34a6e8741f369cbb95bc71e' into bars_phr…
daschuer 769e922
Improve variable names for Track pointers.
JaviVilarroig 3647af9
Improve variable names for Track pointers (2)
JaviVilarroig fcdc116
Document Beats class for Doxygen
JaviVilarroig 4c54b7c
Fix if's without closing brackets.
JaviVilarroig dccf044
Ads operator<< to Beats and cleanup of misc malformed make_shared
JaviVilarroig 331ef5a
Improvements in Timesignature
JaviVilarroig 0a7baad
Use mixxx::kEngineChannelCount for consistency
JaviVilarroig 04d89ea
Rename Beats::setBar to Beats::setDownBeat
JaviVilarroig 800adf6
Rename Beats::setBar to Beats::setDownBeat and some adpaters
JaviVilarroig a7cf0d0
Remove ascii art from Beats header
JaviVilarroig 865caf5
Replace BeatsPointer constructor by make_shared
JaviVilarroig 730abaa
Adapt comment to clang-format
JaviVilarroig 7f857a6
Fix BeatsFactory method signatures to use references.
JaviVilarroig 9f1039d
Remove anonymous namespace from beats.h
JaviVilarroig b3ecb4e
Remove spurious property from Beats
JaviVilarroig 1485e8a
Fixes in BeatIterator
JaviVilarroig 58dbf80
Waveformrenderbeat to interact with beats using frames
JaviVilarroig 49e3d09
pre-commit configuration taken from master
JaviVilarroig c628235
Small general fixes in BeatFactory.
JaviVilarroig 1b90cc3
Comments added to const_cas in BeatIterator
JaviVilarroig 3512947
Change name of method toByteArray to toProtobuff
JaviVilarroig 20cd34c
Fix in indentation
JaviVilarroig d663f04
Add comments to clearly mark temporary methods
JaviVilarroig cbdba12
BeatTrtanslateTest is now OK.
JaviVilarroig 22d6d19
Beats to use Bpm type
JaviVilarroig f81f1fd
Merge commit '18e6e1ed60af1fc9a39bda7302f48743085d8844' into bars_phr…
daschuer b7c8f7b
Fix segfault during tests
daschuer dde8b8f
fix some tests
daschuer 85c85f5
Use double precision for beat positions
daschuer 10715cd
Beats: Fix build
uklotzde 0ef78f3
Fix dependency ordering issues
uklotzde 03afb56
beats.proto: rename beats to beats_per_bar
hacksdump f432e59
beats.h: make class final
hacksdump 0dc057d
beats.h: remove version names from macros
hacksdump 51fdd42
remove adapter for calculateBpm
hacksdump 0f8009b
remove adapter for setGrid
hacksdump 2a4bffb
add frame class to avoid sample/frame ambiguity
hacksdump 56e9051
use single f in protobuf
hacksdump 8e6625a
remove all adapters from main source
hacksdump ec62228
adapt tests to frame class
hacksdump 91eb3d8
remove residual beats subclasses and corresponding tests
hacksdump f9fd333
add channel count to beatstest
hacksdump 9b962d7
frame: add multiply and divide operations
hacksdump 480f7f7
beats: use Frame vector in constructor
hacksdump 65a0d84
beatfactory: write adapter to store beatgrid in beats
hacksdump a49ebe0
beatfactory: use common serialized data for all beatgrid version
hacksdump 3e4664e
beats: remove unused code | rename proto
hacksdump a7b6d01
beats: remove unused function getMaxBpm
hacksdump 919e1b9
beats: remove extraneous helper functions
hacksdump 84d689c
beats: remove derivable state variable
hacksdump fe7fd32
beats: remove capability flags
hacksdump 8957a08
beats:doc: describe source of data in constructor
hacksdump e6b0383
beats: remove optional sample rate parameter
hacksdump b156cd5
bpm: remove redundant parantheses
hacksdump e67686f
Merge branch 'ambigous-sample-rate' into remove-beatmap-beatgrid
hacksdump eab8bb0
beats: simplify bpm scale function
hacksdump 43acd8f
timesignature: modify getters setters
hacksdump df01cce
beats: remove test logic from main source
hacksdump bff9917
fix typo
hacksdump 754ecfe
beatutils: don't use framerate for audio
hacksdump 4b515f2
beats: use double instead of rounding
hacksdump a1dc778
beats: define iterator
hacksdump e32a19c
timesignature: move symmetrical operations outside class
hacksdump 454ba2c
remove redundant copying of member variable
hacksdump 7eb655c
timesignature: use protobuf object to store signature
hacksdump 9c981fb
beats: enable setBpm for all Beat detection modes
hacksdump 4329021
frame: replace forward declaration with include
hacksdump 1356ccb
frame.h refactor: use FramePos and FrameDiff_t
hacksdump 9e04891
beats: update outdated constructor documentation
hacksdump 43e16c1
bpmcontrol: use parented pointers
hacksdump f46860f
bpmcontrol: use unique pointers
hacksdump ea4447a
beatiterator: pointer comparison and exclude end
hacksdump 33b6a4f
change control name
hacksdump 79ecd2a
bpm: pass trivially copyable types by value
hacksdump c013576
bpm:use double for multiple and divide operations
hacksdump 0576be8
beatiterator: remove destructor
hacksdump cbc7977
beats.proto: make fields required
hacksdump d255364
beatutils: remove unnecessary const
hacksdump 397b338
beatutils: remove rounding for frame sample conversion
hacksdump a0ff6d9
beats: use correct range while returning iterator
hacksdump cf75244
beatstest: fix time signature test
hacksdump 074a921
beatstest: remove redundant state variable
hacksdump 136060b
waveformrenderbeat: enable beat pen
hacksdump 86b834a
beatstest: fix NthBeatWhenNotOnBeat
hacksdump cab96a2
track: remove obsolete comments and debug info
hacksdump be68912
bpmcontrol_test: fix BeatContext_BeatGrid
hacksdump a9707f8
beatstest: fix BpmAround
hacksdump 33bf4c9
searchqueryparsertest: fuzzy comparison to fix tests
hacksdump 34c35d7
bpm: pass qstring by reference
hacksdump fe266d0
beats: cleaner debug output
hacksdump 4d8f2f2
engine: use inline functions for frame conversion
hacksdump 0534db3
remove unnecessary frame quantization
hacksdump 915a671
beats: rename method
hacksdump ca4425a
beats: remove copy to self
hacksdump 540c749
engine: update function to use FramePos instead of samples
hacksdump c00da80
engine: refactor to use FramePos instead of sample
hacksdump 5f0ce46
beats: don't delete beats on translate
hacksdump fa86f2a
frame: use constant for invalid frame instead of -1
hacksdump 38555e0
beatstranslatetest: fix SimpleTranslateMatch
hacksdump 7f89649
frame: use C++ style limits
hacksdump 68a1949
use inline functions for sample frame conversion
hacksdump c19d2f2
fix wrong assignment to beats
hacksdump 55d7784
loopingcontrol: use inline functions for sample frame conversion
hacksdump 1fff80c
Merge branch 'master' into remove-beatmap-beatgrid
hacksdump 4f28348
loopingcontroltest: beat size within track duration
hacksdump 8506d0b
use correct epsilon for finding nearby beat
hacksdump 71bf183
fix enginesynctest failing due to typo
hacksdump 3933e76
fix searchqueryparser test by reducing double precision
hacksdump 85e75c9
remove unused functions
hacksdump 425c7fd
remove unnecesary debug function
hacksdump 2ca2be4
searchquery: improve comment on precision downgrading
hacksdump 0710437
beats: use namespaced constants
hacksdump 006cd74
factor out frame sample adapter functions
hacksdump cd80c28
loopingcontol: replace wrong variable
hacksdump bcd32c1
move frameadapter to util
hacksdump a7a7edd
use frameadapter in track
hacksdump 6268b30
fix scons build
hacksdump b2a9bcf
frameadapter: change comment
hacksdump 792c682
beats: temporary dirty workaround for copy constructor
hacksdump 8452657
beatsproto: make all fields optional
hacksdump 8134eae
create new internal beats class independent of QObject
hacksdump a60beee
simplify copy constructor
hacksdump 9a08d05
beats: move findNthBeat to plain class
hacksdump 13538d3
beats: move more code to plain class
hacksdump 0a5fba3
beats: move scale functions to plain class
hacksdump 274c9fa
beats: move all implementation to plain class
hacksdump 49686eb
beats: use function to calculate beat length
hacksdump eb3b6c9
beats: add back old protobuf types for backwards compatibility
hacksdump d3b7a5a
beats: add version to distinguish from earlier versions
hacksdump 79f5b2a
beats: add migration code
hacksdump 6a52cab
merge from master and resolve conflicts
hacksdump abf9694
update comments
hacksdump 7302922
revert all UI changes
hacksdump 77c8d82
use beat indices as marker basis
hacksdump 1e2565a
change section message
hacksdump cc0d8ef
remove redundant ctor initializer
hacksdump de26dd4
remove legacy name from comment
hacksdump e76bfc3
add debug asserts
hacksdump 1967c0f
add placeholder parameters
hacksdump File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Document Beats class for Doxygen
- Loading branch information
commit fcdc11619240e48d26b1c15c7721d9ac74aa0072
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A QObject is not copy-able. I am unsure how this works for inherited classes. At least this has some code smell.
So we should either remove to QObject parent or the copy constructor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's why I mentioned that we need to break up this class into an inner functional class that doesn't inherit from QObject and an outer QObject wrapper.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@uklotzde How do I proceed with this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beats
toBeatsObject
Beats
Storing a pointer to the
Track
object should not be necessary. Instead the track should actively take control of theBeatsObject
.Storing the plain
Track
pointer is unsafe and may cause crashes. The only valid option would be to store a weak pointer if necessary. But try to avoid those cyclic dependencies in the first place.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Appending "Object" to a class name says nothing about what the class does. I am still in favor of "BeatVector", but I'm open to other ideas for the class name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer
BeatVectorWrapper
orBeatVectorQObject
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no need to introduce another confusing and meaningless term, then just "...QObject".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran into a few problems while trying to remove the track member pointer from the Beats class.
The track pointer is currently used to dynamically find the track duration (since it can be updated later, which is a common scenario in tests)
Resolution:
Use TrackWeakPointer in Beats
This is semantically not wrong and could possibly work but in
Track::setBpm
a new Beats object is created and the constructor needs a shared pointer to Track itself. So weak pointer can't be used. An outer agent would need to setup the ownership between track and beats which is not doable.Entirely remove track member pointer.
This would need the creation of new signals in the track class and connections would need to be established on construction to update data in Beats that can change with track.
Keep the current configuration
We are storing a plain pointer to track in Beats class. This means there is no ownership per se. So we don't really run into a cyclic dependency here. Track has active control of Beats and in addition to that, Beats stores a plain pointer to the track.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am leaning towards option 2, but I'd like to hear @uklotzde's opinion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I get the issue with option 1 with transforming
this
into a smart pointer. Ruled out.Let's stick to option 3. Extending
Track
could prove difficult, because it is a very central place with lots of dependencies. Don't open the Pandora's box now ;) Later we can decide about how to move on. This does not only affect the beat grid class.The
Track*
should only be stored in the upper levelQObject
derived class. If you need the duration or sample rate in the plain C++ class either store them as members or pass them as parameters only when needed. If they are only needed for certain operations pass them as parameters. They don't really belong to the basic concept of a beat grid and maintaining less state avoids potential inconsistencies.