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

Remove all sample-based Cue methods #4061

Merged
merged 2 commits into from
Jul 12, 2021
Merged

Conversation

Holzhaus
Copy link
Member

@Holzhaus Holzhaus commented Jul 5, 2021

Based on #4049. This changes the Cue API to use frame positions.

Prerequisite PRs:

This is extremely weird. Only the last commit is of interest (the other ones are part of other PRs). That last commit makes a bunch of test cases time out, and I have no idea why.

EDIT The commit seems to cause an endless loop in Track::setCuePointsWhileLocked. Investigating.

EDIT 2 Looks like I'm not the brightest candle on the cake:

    double toEngineSamplePosMaybeInvalid() const {
        if (!isValid()) {
            return kLegacyInvalidEnginePosition;
        }
        return toEngineSamplePosMaybeInvalid();
    }

No wonder there is an endless loop :)

EDIT 3 Fixed.

@Holzhaus Holzhaus force-pushed the frame-refactor branch 4 times, most recently from 8041c41 to e6236cd Compare July 6, 2021 15:44
@Holzhaus Holzhaus changed the title Cue frame refactor [WIP] Cue frame refactor Jul 6, 2021
@Holzhaus Holzhaus mentioned this pull request Jul 6, 2021
@Holzhaus Holzhaus force-pushed the frame-refactor branch 3 times, most recently from a0c8f60 to 8f031b4 Compare July 7, 2021 17:27
@Holzhaus Holzhaus mentioned this pull request Jul 7, 2021
@Holzhaus Holzhaus changed the title Cue frame refactor Remove all sample-based Cue methods Jul 7, 2021
@Holzhaus Holzhaus force-pushed the frame-refactor branch 3 times, most recently from 0d09df3 to 09fe05b Compare July 8, 2021 20:21
@Holzhaus Holzhaus marked this pull request as ready for review July 11, 2021 21:00
@Holzhaus Holzhaus requested a review from uklotzde July 11, 2021 21:01
@coveralls
Copy link

Pull Request Test Coverage Report for Build 1020805526

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.004%) to 28.643%

Totals Coverage Status
Change from base Build 1019395588: 0.004%
Covered Lines: 20099
Relevant Lines: 70170

💛 - Coveralls

Copy link
Contributor

@uklotzde uklotzde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 LGTM

@uklotzde uklotzde merged commit 611a153 into mixxxdj:main Jul 12, 2021
@LeDucDuBleuet
Copy link

Currently AutoDJ crashes Mixxx completely with a segfault when using "Enable random track addition to queue". It started crashing with this commit or the ones referred to in the first comment I believe a couple days after committing #4041...

Let me know if we need to file a new bug on Launchpad. Thx

@Be-ing
Copy link
Contributor

Be-ing commented Jul 15, 2021

That is unrelated to this PR, please report a bug on Launchpad.

@LeDucDuBleuet
Copy link

@Be-ing Thanks for the quick reply, I opened one : https://bugs.launchpad.net/mixxx/+bug/1936396

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

5 participants