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.

Sign up for GitHub

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

Jump to bottom

Oversampling changes pitch for AFP #5218

Closed
softrabbit opened this issue Oct 3, 2019 · 8 comments
Closed

Oversampling changes pitch for AFP #5218

softrabbit opened this issue Oct 3, 2019 · 8 comments
Labels
bug critical
Milestone
1.2.1

Comments

@softrabbit
Copy link
Member

See picture, count the waves.
Upper track: drums in AFP, 1x oversampling
Lower track: the same with 2x oversampling, sounds an octave too low.
oversampling-broken

LMMS version: 1.2.0 in Windows 8.1 pro / 64 bit. Didn't find any fitting reported issue, and I think I've used oversampling successfully in the past, so this might be a recent bug.
@softrabbit
Copy link
Member Author

softrabbit commented Oct 3, 2019 •
edited
Loading

Master (checked both Windows and Linux) has this bug, too. 1.1.90 (Win64) seems OK.

@PhysSong
Copy link
Member

PhysSong commented Oct 6, 2019

The bug was introduced in #4991. It seems like SampleBuffer::m_sampleRate doesn't update on sample rate changes while the actual data does. I think update() or normalizeSampleRate() should handle the change.
@Reflexe Any ideas? BTW, it seems like #4994 removes those functions.

@PhysSong PhysSong added the bug label Oct 6, 2019
@PhysSong PhysSong added this to the 1.2.1 milestone Oct 6, 2019
@PhysSong
Copy link
Member

PhysSong commented Oct 6, 2019

It seems like this bug makes the situation even worse: even changing the sample rate on export breaks stuff. This applies to sample tracks as well...

@Reflexe Reflexe added bug-master stable branch This issue applies to the latest stable labels Oct 6, 2019
@Reflexe
Copy link
Member

Reflexe commented Oct 6, 2019

I think I will have to cherry pick a few more changes from the recording branch. I don't want to duplicate my work.

softrabbit added a commit to softrabbit/lmms that referenced this issue Oct 6, 2019
@softrabbit
Make SampleBuffer adjust its members when resampling. Fixes LMMS#5218.
cd642b1
@Reflexe
Copy link
Member

Reflexe commented Oct 19, 2019

I'm retargeting it for 1.3 as a fix for 1.2.1 has been merged.

@Reflexe Reflexe modified the milestones: 1.2.1, 1.3.0 Oct 19, 2019
@Reflexe Reflexe removed the stable branch This issue applies to the latest stable label Oct 19, 2019
@PhysSong PhysSong modified the milestones: 1.3.0, 1.2.1 Oct 21, 2019
@Reflexe
Copy link
Member

Reflexe commented Oct 21, 2019

@PhysSong Please comment when you're undoing something.

@PhysSong
Copy link
Member

Sorry, I was going to sync branches and make this auto-closed.

@zonkmachine
Copy link
Member

zonkmachine commented Oct 21, 2019 •
edited
Loading

and make this auto-closed

The short form for auto closing only works on the default branch. I think you could use a longer form looking something like closes lmms/lmms/#5218.

@PhysSong PhysSong removed the master label Aug 17, 2020
sdasda7777 pushed a commit to sdasda7777/lmms that referenced this issue Jun 28, 2022
@softrabbit @Reflexe
Make SampleBuffer adjust its members when resampling
6de449f 
Fixes LMMS#5218.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug critical
Projects
None yet
Milestone
1.2.1
Development

No branches or pull requests
4 participants
@zonkmachine @softrabbit @PhysSong @Reflexe