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 CueInfo DTO #2523

Merged
merged 10 commits into from
Mar 2, 2020
Merged

Add CueInfo DTO #2523

merged 10 commits into from
Mar 2, 2020

Conversation

Holzhaus
Copy link
Member

This adds a CueInfo class that does not have any dependencies on Track or Cue. To archieve this, we use positions in milliseconds instead of sample positions, so that we don't need to know the tracks' sample rate.

The Track object gets a new method to import cues from a list of CueInfo objects.

This would be useful for Serato Markers Integration (PR #2499).

Also see related Zulip discussion for details:
https://mixxx.zulipchat.com/#narrow/stream/109171-development/topic/2.2E3.20planning/near/189265725

src/track/cueinfo.cpp Outdated Show resolved Hide resolved
src/track/cueinfo.cpp Outdated Show resolved Hide resolved
src/track/cueinfo.h Outdated Show resolved Hide resolved
src/track/cueinfo.h Outdated Show resolved Hide resolved
src/track/cueinfo.h Outdated Show resolved Hide resolved
src/track/cueinfo.h Outdated Show resolved Hide resolved
src/track/cueinfo.h Outdated Show resolved Hide resolved
src/track/track.cpp Outdated Show resolved Hide resolved
src/track/track.cpp Outdated Show resolved Hide resolved
src/track/cueinfo.h Outdated Show resolved Hide resolved
@Holzhaus
Copy link
Member Author

@uklotzde Sorry, I fixed up the commits. The only thing missing is a decision about moving the constants into global namespace: https://github.com/mixxxdj/mixxx/pull/2523/files/d0fb17ca6faaf4c59d58067f826d32de55eef187#diff-1483dfb71fc243bb08cf3cb33a1d5c5c

@uklotzde
Copy link
Contributor

@Holzhaus Rebuild this PR as needed and deliberately drop any intermediate versions that are not worth keeping in the final commit history.

@Holzhaus Holzhaus changed the title Add CueInfo DTO [WIP] Add CueInfo DTO Feb 28, 2020
@Holzhaus
Copy link
Member Author

@Holzhaus Rebuild this PR as needed and deliberately drop any intermediate versions that are not worth keeping in the final commit history.

Done ;-)

@Holzhaus Holzhaus requested a review from uklotzde February 28, 2020 18:17
@Holzhaus
Copy link
Member Author

By the way, should I put this class into the mixxx namespace? Some classes are and some aren't and I don't know what the pattern is.

@uklotzde
Copy link
Contributor

By the way, should I put this class into the mixxx namespace? Some classes are and some aren't and I don't know what the pattern is.

I've just started that once. It helps to prevent name clashes with external libraries. It is still not consistently applied, yet. Yes, maybe that would be an option for such a foundational component.

@Holzhaus
Copy link
Member Author

By the way, should I put this class into the mixxx namespace? Some classes are and some aren't and I don't know what the pattern is.

I've just started that once. It helps to prevent name clashes with external libraries. It is still not consistently applied, yet. Yes, maybe that would be an option for such a foundational component.

Done.

@Holzhaus Holzhaus changed the title [WIP] Add CueInfo DTO Add CueInfo DTO Feb 28, 2020
@Holzhaus
Copy link
Member Author

If nothing else is missing, let's merge. I can rebase #2499 on master then.

@uklotzde uklotzde added this to the 2.3.0 milestone Feb 29, 2020
src/track/cue.cpp Outdated Show resolved Hide resolved
src/track/cue.cpp Outdated Show resolved Hide resolved
src/track/track.cpp Outdated Show resolved Hide resolved
src/track/track.h Outdated Show resolved Hide resolved
@Holzhaus
Copy link
Member Author

Done.

src/track/track.cpp Outdated Show resolved Hide resolved
@uklotzde
Copy link
Contributor

uklotzde commented Mar 1, 2020

Ok, I guess this is ready now.

@uklotzde
Copy link
Contributor

uklotzde commented Mar 2, 2020

LGTM. Thank you, Jan, for getting this rework started!

@uklotzde uklotzde merged commit 18a134f into mixxxdj:master Mar 2, 2020
@Holzhaus Holzhaus mentioned this pull request Mar 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants