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

EQ plugin now responds to wet / dry control #3520

Merged
merged 1 commit into from May 2, 2017

Conversation

Projects
None yet
4 participants
@curlymorphic
Contributor

curlymorphic commented May 1, 2017

The native Equalizer plugin didn't respond to the wet dry control.

This has been rectified, fixes #3505

@BaraMGB

This comment has been minimized.

Show comment
Hide comment
@BaraMGB

BaraMGB May 1, 2017

Contributor

@curlymorphic thank you very much! 😁👍

Contributor

BaraMGB commented May 1, 2017

@curlymorphic thank you very much! 😁👍

@Pyrif

This comment has been minimized.

Show comment
Hide comment
@Pyrif

Pyrif May 1, 2017

[Redacted as for whatever reason the pull request failed to assert itself on my machine]

Update: I've figured out that the EQ now ceases to provide output should the input be clipping above a certain amount. Will likely make a bug report for this in the future.

Pyrif commented May 1, 2017

[Redacted as for whatever reason the pull request failed to assert itself on my machine]

Update: I've figured out that the EQ now ceases to provide output should the input be clipping above a certain amount. Will likely make a bug report for this in the future.

@tresf

This comment has been minimized.

Show comment
Hide comment
@tresf

tresf May 1, 2017

Member

@UnityParadox please hand--validate the file changes to make sure they were applied after the checkout.

https://github.com/LMMS/lmms/pull/3520/files

Member

tresf commented May 1, 2017

@UnityParadox please hand--validate the file changes to make sure they were applied after the checkout.

https://github.com/LMMS/lmms/pull/3520/files

@curlymorphic

This comment has been minimized.

Show comment
Hide comment
@curlymorphic

curlymorphic May 1, 2017

Contributor

@UnityParadox

I am unsure of your message.

Have you downloaded built run the pull request, and the D/W control is unworking?

or are you having trouble downloading the patch?

Contributor

curlymorphic commented May 1, 2017

@UnityParadox

I am unsure of your message.

Have you downloaded built run the pull request, and the D/W control is unworking?

or are you having trouble downloading the patch?

@Pyrif

This comment has been minimized.

Show comment
Hide comment
@Pyrif

Pyrif May 1, 2017

@curlymorphic What I'm saying is that I've downloaded and synced the pull request, but the script, for whatever reason, hasn't changed.

Update: As of this, I'm going to attempt to manually copy the script and replace it. I haven't the slightest idea why it didn't work, though.

Pyrif commented May 1, 2017

@curlymorphic What I'm saying is that I've downloaded and synced the pull request, but the script, for whatever reason, hasn't changed.

Update: As of this, I'm going to attempt to manually copy the script and replace it. I haven't the slightest idea why it didn't work, though.

@curlymorphic

This comment has been minimized.

Show comment
Hide comment
@curlymorphic

curlymorphic May 1, 2017

Contributor

@UnityParadox you could try and download from my fork into a new directory

git clone -b"eqWetDry" https://github.com/curlymorphic/lmms

Contributor

curlymorphic commented May 1, 2017

@UnityParadox you could try and download from my fork into a new directory

git clone -b"eqWetDry" https://github.com/curlymorphic/lmms

@Pyrif

This comment has been minimized.

Show comment
Hide comment
@Pyrif

Pyrif May 1, 2017

After manually copying the script, I can now confirm that it does, in fact, remedy the bug report I posted. I'm afraid I'm brand new to testing pull requests, so I'm not fully knowledgeable in the area, so I apologize for any inconveniences I may have caused here.

Additional Details: Output issues are now present for the native EQ. Uncertain if this issue is specific to the pull request. Will need people to test fiddling with the equalizer on this pull request before I can confirm.

The issue is as follows: After the equalizer processes anything that clips over a certain limit, most likely what I can only describe right now as "top of the mixer track," it ceases to process and output information. Can anyone else confirm this issue for me on stable-1.2 and/or the current pull request?

Pyrif commented May 1, 2017

After manually copying the script, I can now confirm that it does, in fact, remedy the bug report I posted. I'm afraid I'm brand new to testing pull requests, so I'm not fully knowledgeable in the area, so I apologize for any inconveniences I may have caused here.

Additional Details: Output issues are now present for the native EQ. Uncertain if this issue is specific to the pull request. Will need people to test fiddling with the equalizer on this pull request before I can confirm.

The issue is as follows: After the equalizer processes anything that clips over a certain limit, most likely what I can only describe right now as "top of the mixer track," it ceases to process and output information. Can anyone else confirm this issue for me on stable-1.2 and/or the current pull request?

@curlymorphic

This comment has been minimized.

Show comment
Hide comment
@curlymorphic

curlymorphic May 1, 2017

Contributor

@UnityParadox

Thank you for taking the time to test.
There is no need to apologize, life is all about learning :)

Contributor

curlymorphic commented May 1, 2017

@UnityParadox

Thank you for taking the time to test.
There is no need to apologize, life is all about learning :)

@tresf

This comment has been minimized.

Show comment
Hide comment
@tresf

tresf May 1, 2017

Member

Output issues are now present for the native EQ. Uncertain if this issue is specific to the pull request.

You can revert the fixes by checking out the master branch again (or hand-reversing the file). If it exists there, it's not related. 👍

Member

tresf commented May 1, 2017

Output issues are now present for the native EQ. Uncertain if this issue is specific to the pull request.

You can revert the fixes by checking out the master branch again (or hand-reversing the file). If it exists there, it's not related. 👍

@Pyrif

This comment has been minimized.

Show comment
Hide comment
@Pyrif

Pyrif May 1, 2017

Update: I have confirmed the output issue to be related to this pull request in particular. Can anyone else confirm?

Pyrif commented May 1, 2017

Update: I have confirmed the output issue to be related to this pull request in particular. Can anyone else confirm?

@curlymorphic

This comment has been minimized.

Show comment
Hide comment
@curlymorphic

curlymorphic May 1, 2017

Contributor

@UnityParadox can you clarify the output issue you are now witnessing please?

Contributor

curlymorphic commented May 1, 2017

@UnityParadox can you clarify the output issue you are now witnessing please?

@Pyrif

This comment has been minimized.

Show comment
Hide comment
@Pyrif

Pyrif May 1, 2017

@curlymorphic The issue is that should the input be louder than a certain amount (which i suspect to be "top of the mixer"), or should the output of the plugin reach that amount, the equalizer altogether ceases to process audio, muting all output from the plugin.

Pyrif commented May 1, 2017

@curlymorphic The issue is that should the input be louder than a certain amount (which i suspect to be "top of the mixer"), or should the output of the plugin reach that amount, the equalizer altogether ceases to process audio, muting all output from the plugin.

@curlymorphic

This comment has been minimized.

Show comment
Hide comment
@curlymorphic

curlymorphic May 1, 2017

Contributor

h The issue is that should the input be louder than a certain amount (which i suspect to be "top of the mixer"), or should the output of the plugin reach that amount, the equalizer altogether ceases to process audio, muting all output from the plugin.

I think I know what is causing this. I can remember this happening in another project. I shall update shortly

Contributor

curlymorphic commented May 1, 2017

h The issue is that should the input be louder than a certain amount (which i suspect to be "top of the mixer"), or should the output of the plugin reach that amount, the equalizer altogether ceases to process audio, muting all output from the plugin.

I think I know what is causing this. I can remember this happening in another project. I shall update shortly

@tresf

This comment has been minimized.

Show comment
Hide comment
@tresf

tresf May 1, 2017

Member

I think I know what is causing this. I can remember this happening in another project. I shall update shortly

@curlymorphic is it at all related to this? #3422

Member

tresf commented May 1, 2017

I think I know what is causing this. I can remember this happening in another project. I shall update shortly

@curlymorphic is it at all related to this? #3422

@curlymorphic

This comment has been minimized.

Show comment
Hide comment
@curlymorphic

curlymorphic May 1, 2017

Contributor

@curlymorphic is it at all related to this? #3422

This may be triggering the bug, but it is not the cause, or solution.

The bi quad filters are sensitive to high inputs, and can oscillate, and what I call "pop" once this has happened the filter buffer needs clearing. I had a similar issue on an android synth I wrote, using very similar code. I handled the error there by checking for a corrupt buffer on every noteOn,

I need some time to think about where/how to implement this fix for this case. I need to spend some time testing other features of lmms that use the same code (Dual Filters spring to mind) to see if the fix should be implemented in the eq plugin, or the filter code.

I know how to fix, it's just where that I need to decide.

Contributor

curlymorphic commented May 1, 2017

@curlymorphic is it at all related to this? #3422

This may be triggering the bug, but it is not the cause, or solution.

The bi quad filters are sensitive to high inputs, and can oscillate, and what I call "pop" once this has happened the filter buffer needs clearing. I had a similar issue on an android synth I wrote, using very similar code. I handled the error there by checking for a corrupt buffer on every noteOn,

I need some time to think about where/how to implement this fix for this case. I need to spend some time testing other features of lmms that use the same code (Dual Filters spring to mind) to see if the fix should be implemented in the eq plugin, or the filter code.

I know how to fix, it's just where that I need to decide.

@tresf

This comment has been minimized.

Show comment
Hide comment
@tresf

tresf May 1, 2017

Member

I know how to fix, it's just where that I need to decide.

Ok, thanks for explanation. How about its impact on this PR? Is it related, or can we merge as-is?

Member

tresf commented May 1, 2017

I know how to fix, it's just where that I need to decide.

Ok, thanks for explanation. How about its impact on this PR? Is it related, or can we merge as-is?

@curlymorphic

This comment has been minimized.

Show comment
Hide comment
@curlymorphic

curlymorphic May 1, 2017

Contributor

I feel there are two seperate issues,

the Wet / Dry control, This is now implemented, and can be merged

The unstable filters. It may be best If i open a new issue for this, as the filters are outside the EQ plugin code

Contributor

curlymorphic commented May 1, 2017

I feel there are two seperate issues,

the Wet / Dry control, This is now implemented, and can be merged

The unstable filters. It may be best If i open a new issue for this, as the filters are outside the EQ plugin code

@tresf

This comment has been minimized.

Show comment
Hide comment
@tresf

tresf May 2, 2017

Member

Tested, works. Not entirely sure how to reproduce the unstable filters, but I don't see evidence that this is influenced by this pull request. Merging.

Member

tresf commented May 2, 2017

Tested, works. Not entirely sure how to reproduce the unstable filters, but I don't see evidence that this is influenced by this pull request. Merging.

@tresf tresf merged commit 0190256 into LMMS:stable-1.2 May 2, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@curlymorphic curlymorphic deleted the curlymorphic:eqWetDry branch May 2, 2017

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