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

Plugin release frames depend on sample rate #7211

Closed
zonkmachine opened this issue Apr 18, 2024 · 3 comments · Fixed by #7217
Closed

Plugin release frames depend on sample rate #7211

zonkmachine opened this issue Apr 18, 2024 · 3 comments · Fixed by #7217

Comments

@zonkmachine
Copy link
Member

zonkmachine commented Apr 18, 2024

Bug Summary

First brought up here: #6908 (comment)

When a plugin doesn't use the envelope it still has a fixed number of release frames. Example from Bitinvader:

f_cnt_t desiredReleaseFrames() const override
{
return( 64 );
}

The problem with a fixed number of frames for release is that the decay time now will vary with the sample rate as shown in the release below in a project exported with three different sample rates.

samplerate

Expected Behaviour

The release time should be the same and independent of sample rate.

From @michaelgregorius in the same thread.

It looks like the release should be defined in milliseconds which is then converted into a number of samples based on the sample rate.

@zonkmachine zonkmachine changed the title Plugin release frames depends on sample rate Plugin release frames depend on sample rate Apr 18, 2024
@Rossmaxx
Copy link
Contributor

Would it make sense to return 64 * (sampleRate ÷ 44100) ?

@michaelgregorius
Copy link
Contributor

I think it makes more sense to solve the problem by changing the method to something like desiredReleaseTimeMs. The underlying problem is that the release time is requested in a unit that's independent of the sample rate, so this problem should be fixed.

If you search for desiredReleaseFrames you will find that there are already several overrides. If the method desiredReleaseFrames was only adjusted and kept then the problem might creep in later again even it all overrides are adjusted as well. Or put differently: the problem should be fixed in one place where the release time in milliseconds is converted into sample frames. Keeping desiredReleaseFrames would mean that the problem had to be fixed in several places with the danger of the aforementioned return of the problem, e.g. in new classes.

For the overrides it should also be checked if the number of frames that are used really make sense or if they haven't just been chosen arbitrarily by the implementers. Another potential explanation for the differences in the implementations might be that the implementation in the base class was changed without taking all the overriding implementations into account.

@michaelgregorius
Copy link
Contributor

@zonkmachine, can you please check with the changes of pull request #7217?

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

Successfully merging a pull request may close this issue.

3 participants