-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add Sample Clip Fading #7900
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
base: master
Are you sure you want to change the base?
Add Sample Clip Fading #7900
Conversation
…nd clamp fade length
// Action::Move, Action::Resize and Action::ResizeLeft | ||
// Action::Split action doesn't disable Clip journalling | ||
if (m_action == Action::Move || m_action == Action::Resize || m_action == Action::ResizeLeft) | ||
{ | ||
m_clip->setJournalling(false); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'mma be real I have no idea what this code is supposed to do :skullemoji:. As far as I can tell, nothing after this code block does anything which would trigger a checkpoint to be created.
I tried to make it simpler by just having if (m_action != Action::Split)
, but I ran into very odd issues with journalling/clip copying. I decided to remove it, and I haven't encountered any problems since, so..... maybe it's fine?
How hard would it be to add a tension dot? What I mean by this is that the curve looks fine but what if I want to change the shape of the curve? Maybe you could add a point in the middle of the curve to change the tension. If you can do this, it would be amazing. |
src/core/SamplePlayHandle.cpp
Outdated
|
||
// Apply crossfade | ||
const int framesPerTick = Engine::framesPerTick(Engine::audioEngine()->outputSampleRate()); | ||
const int startCrossfadeFrames = m_clip->startCrossfadeLength() * framesPerTick; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SamplePlayHandle
is used when there isn't any clip at all, like in the file browser. Have you accounted for this? Wouldn't this just not work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops, I completely forgot to test it with the file browser. You're right, it crashes lol. I've fixed it in the latest commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like SampleClip
should have its own place for processing, because doing it in SamplePlayHandle
feels like there is a problem involving separation of concerns here. Though, I'm not sure where else you would put it.
I've implement something like this in the latest commit. Basically instead of having a hardcoded square root curve, the height of the tension dot now determines the power. This does mean that I have to raise a number a power every frame. I'm currently using |
Adds the ability to fade the start and end of samples.
This PR does a similar thing as #5616, though admittedly much simpler. I currently only have sqrt curves, and the sample drawing does not update as you drag (this may be difficult given that we now use sample thumbnails?)
But, the main benefit is that this PR has no merge conflicts.
How to use
Mouse over the top corner of a sample clip, and a little circle handle will appear. You can drag that around to change the length of the fade. If you have a selection of sample clips, the fade will be applied to all of them.
2025-05-22.17-41-25.mp4
2025-05-21.23-13-35.mp4
PS, I originally planned for this PR to include auto-crossfading, but I have since removed that feature. Should I rename everything to just "fade" instead of "crossfade"?