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

Fix bad phase seek when starting from preroll. #4093

Merged
merged 1 commit into from Jul 9, 2021

Conversation

ywwg
Copy link
Member

@ywwg ywwg commented Jul 9, 2021

We were interpreting all <0 playposition as "invalid", which is not correct.
fixes https://bugs.launchpad.net/mixxx/+bug/1930143

@ywwg
Copy link
Member Author

ywwg commented Jul 9, 2021

This is a great example of where using magic numbers to mean invalid is bad!

@uklotzde uklotzde added this to the 2.3.1 milestone Jul 9, 2021
We were interpreting all <0 playposition as "invalid", which is not correct.
fixes https://bugs.launchpad.net/mixxx/+bug/1930143
@ywwg
Copy link
Member Author

ywwg commented Jul 9, 2021

fwiw I don't think this is minor, this caused some trainwrecks in a set I played because the cue points were just barely before the start of the track.

@Holzhaus
Copy link
Member

Holzhaus commented Jul 9, 2021

@ywwg can you take care of the merge to main after merging this PR?

@ywwg
Copy link
Member Author

ywwg commented Jul 9, 2021

@ywwg can you take care of the merge to main after merging this PR?

yeah sure. I actually wrote it for that and then backported

@Holzhaus
Copy link
Member

Holzhaus commented Jul 9, 2021

Works fine on my machine. Waiting for CI.

@ywwg
Copy link
Member Author

ywwg commented Jul 9, 2021

what's the right way to merge to main? do a "git merge 2.3", or do a cherrypick of the commit?

@Holzhaus
Copy link
Member

Holzhaus commented Jul 9, 2021

$ git checkout --track upstream/main
$ git pull upstream main  # this should be fast-forward or a noop
$ git pull upstream 2.3
$ make -j16 && ctest -j16
$ git push upstream main

@Holzhaus Holzhaus merged commit c29d9f6 into mixxxdj:2.3 Jul 9, 2021
@ywwg
Copy link
Member Author

ywwg commented Jul 10, 2021

pushed to main

@ywwg ywwg added the changelog This PR should be included in the changelog label Jul 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog This PR should be included in the changelog code quality minor bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants