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

SampleBuffer -> Samplerate - Fix two sample rate issues: #4991

Merged
merged 3 commits into from
Jun 2, 2019

Conversation

Reflexe
Copy link
Member

@Reflexe Reflexe commented May 25, 2019

  1. Resampling was the wrong (src_rate was invalid)
  2. SampleBuffer was using baseSampleRate as the default samplerate
    instead of the actual processingSampleRate.

Closes #4786

@Reflexe Reflexe added the needs code review A functional code review is currently required for this PR label May 26, 2019
@Reflexe Reflexe added this to the 1.2.0 milestone May 27, 2019
@Reflexe Reflexe requested a review from BaraMGB May 27, 2019 19:04
src/core/SampleBuffer.cpp Outdated Show resolved Hide resolved
src/core/SamplePlayHandle.cpp Outdated Show resolved Hide resolved
@BaraMGB
Copy link
Contributor

BaraMGB commented May 28, 2019

Closes #4975

Should be #4786

@BaraMGB
Copy link
Contributor

BaraMGB commented May 28, 2019

#4786 is not fixed with this PR. 😕

I guess, the sample on the sample track is played with its own samplerate. I exported a clicktrack (with 44100) and loaded it to the sample track. My Jack runs with 48000 Hz. The click and the sample aren't in sync anymore.

@Reflexe
Copy link
Member Author

Reflexe commented May 28, 2019

Ok, I've found the right commit. However, I can't understand how your solution has solved it? :\
Reflexe@7b49167
However, we currently DON'T save SampleBuffer's sample rate to project files (fixed it in the recording branch)
maybe we should merge it instead :D

@Reflexe Reflexe force-pushed the fix/sample_rate_fix branch 2 times, most recently from 6f314c2 to f0e344f Compare May 28, 2019 19:48
@Reflexe Reflexe requested a review from BaraMGB May 28, 2019 19:49
src/core/SampleBuffer.cpp Outdated Show resolved Hide resolved
@Reflexe
Copy link
Member Author

Reflexe commented May 29, 2019 via email

@Reflexe
Copy link
Member Author

Reflexe commented May 29, 2019

Oky dooky, I think we are good now (I fixed that in another commit probably :) Thanks for your help!

Copy link
Contributor

@BaraMGB BaraMGB left a comment

Choose a reason for hiding this comment

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

The issue is fixed with this PR, now. After fixing these 2 style issues I vote for merge.

include/Engine.h Outdated Show resolved Hide resolved
src/core/Engine.cpp Outdated Show resolved Hide resolved
SampleBuffer was using baseSampleRate as the default samplerate instead of the actual processingSampleRate.
played correctly after saving and loading a project due to sample rate
not getting saved in the project file.
right place when it not played from the begining.

That has created a difference between the ticks and the metronome and
the sample track.

The cause of the problem was that the calculation of the frame to play
was wrong: we had calculated `framesPerTick` according to the current
engine's sample rate instead of the SampleBuffer's sample rate.
@Reflexe
Copy link
Member Author

Reflexe commented Jun 2, 2019

Since it isn't a single issue, I'll rebase it instead of squashing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs code review A functional code review is currently required for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sample track out of sync with jack
2 participants