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

EngineBuffer: Fix broken playback due to rate integer division #4085

Merged
merged 1 commit into from
Jul 8, 2021

Commits on Jul 8, 2021

  1. EngineBuffer: Fix broken playback due to rate integer division

    This is a critical bug that slipped in during the latest semantic type
    refactorings. Hitting play doesn't actually start playback anymore. It's
    unfortunate that there is apparently no test that catches this bug.
    
    Bisecting revealed that commit 9c33e11
    from PR mixxxdj#4065 introduced this issue:
    
        $ git show --oneline --no-patch 9c33e11
        9c33e11 (origin/enginebuffer-samplerate) EngineBuffer: Use mixxx::audio::SampleRate instead of int/double
    
    The root cause is that the `mixxx::audio::SampleRate` is an integer and
    when dividing a sample rate by another sample rate, it implicitly uses
    integer division instead of double division. When we divide the old
    sample rate by the new sample rate to calculate the base playback speed,
    the result is seems to always be 0 due to this issue.
    This is unexpected and not at all apparent from the diff.
    
    This commit is just a band-aid to fix the issue. In the long-term, we
    should rework the `mixxx::audio::SampleRate` type and remove the
    implicit conversion. All operators should have been implemented
    explicitly for the possible operand types, like we do for
    `mixxx::audio::FramePos`.
    
    Furthermore, this shows that our test coverage is too low. We should
    definitely add a test for this.
    
    PR that introduced the issue: mixxxdj#4065
    Holzhaus committed Jul 8, 2021
    Configuration menu
    Copy the full SHA
    fdcb065 View commit details
    Browse the repository at this point in the history