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

Implementing an official All Pass #78

Closed
pixmusix opened this issue Jan 14, 2023 · 5 comments
Closed

Implementing an official All Pass #78

pixmusix opened this issue Jan 14, 2023 · 5 comments
Labels
enhancement New feature or request
Milestone

Comments

@pixmusix
Copy link
Contributor

Hello.

According to the documentation there is not an official AllPass filter for processing-sound available to the user.
There does appear to be some code for one in the source from 2018 used to implement other effects.

If we are interesting in creating one, I think it should be based on this object from jsyn.
http://www.softsynth.com/jsyn/docs/javadocs/com/jsyn/unitgen/FilterOnePoleOneZero.html

Using the already implemented lowpass filter as a guide, I propose something like the below.

package processing.sound;

import com.jsyn.unitgen.FilterOnePoleOneZero;

import processing.core.PApplet;

public class AllPass extends Effect<FilterOnePoleOneZero> {

	public AllPass(PApplet parent) {
		super(parent);
		//init some reasonable values for a0, a1, b1.
	}

	@Override
	protected FilterOnePoleOneZero newInstance() {
		return new FilterOnePoleOneZero();
	}

	public void coeffs(float inputCoeff, float delayedInputCoeff, float delayedOutputCoeff) {
		//Some safety checks to ensure inputs are in a reasonable range.
		this.left.a0.set(inputCoeff);
		this.left.a1.set(delayedInputCoeff);
		this.left.b1.set(delayedOutputCoeff);
	}

	public void process(SoundObject input, float a0, float a1, float b1) {
		this.coeffs(a0, a1, b1);
		this.process(input);
	}
}

Would the processing-sound team be interested in an allpass filter for processing-sound?

Thank you for your time and I'm looking forward to hearing from you.
Warm Regards, PixMusix.

@kevinstadler
Copy link
Collaborator

Hello,

thanks for your message, an all-pass filter would certainly be a welcome addition to the library! If you wanted to make a pull-request based on your code I'd be happy to merge it. If you're too busy to do it yourself, we will probably commit a good chunk of time to overhauling the library and adding new features over the next few months, in which case I'd be able to add your code (with attribution of course) myself.

Best,
Kevin

@pixmusix
Copy link
Contributor Author

pixmusix commented Jan 15, 2023

Hi Kevin.

Thanks for getting back to me.

I'd be happy to write the code.
I'll also make a simple example for the documentation if you like.

With Processing-Sound being a popular library I'd appreciate it if you, or someone experienced with the package, ran a second pair of eyes over my the code before it is merged. Would that be OK?

@kevinstadler
Copy link
Collaborator

That would be great if you could also add an example! No worries we'd certainly go over all the code before it's packaged up and distributed.

As you might have noticed, the library documentation on the Processing website is actually automatically generated from the Javadoc comments in the source files, so if you were feeling inspired you could already add appropriate Javadoc comments to your code, broadly following other effects classes e.g. https://github.com/processing/processing-sound/blob/master/src/processing/sound/BandPass.java

Absolutely no worries if not, as I said we have a substantial codebase overhaul coming up, where documentation would be addressed anyway.

@pixmusix
Copy link
Contributor Author

pixmusix commented Jan 15, 2023

Fantastic.

Docs

Ah, I was wondering what @webref was all about. That's cool!
I will happily put some of those comments in for the repo. I won't be able to test it before submitting the pull request.
I also noticed These htmls files here.
Will my comments @webref and @webBrief automatically generate an AllPass.html or would you like me to make one?

Using a different jsyn object

Choosing musical coeffs for OnePoleOneZero was quite challenging even when I knew where to start.
I found a jsyn second order allpass filter which is in my opinion a more user-friendly choice for the processing-sound library.
Still very functional; less technical knowledge required. Are you happy with this change?

Advice for naming a method.

Jsyn's FilterAllPass calls its unitport "gain" which makes sense from a digital signal processing perspective. I think gain may be confused with amplitude, especially since many processing users are not expected to have background in audio.
I'd like to use "drive" instead. Is it OK that I break with the jsyn documentation?

package processing.sound;

import com.jsyn.unitgen.FilterAllPass;
import processing.core.PApplet;

public class AllPass extends Effect<FilterAllPass> {

	public AllPass(PApplet parent) {
		super(parent);
	}

	@Override
	protected FilterAllPass newInstance() {
		return new FilterAllPass();
	}

	public void drive(float g) {
		this.left.gain.set(g);
		this.right.gain.set(g);
	}

	public void process(SoundObject input, float g) {
		this.drive(g);
		this.process(input);
	}
}

@kevinstadler kevinstadler added this to the 2.4 milestone May 15, 2023
@pixmusix
Copy link
Contributor Author

pixmusix commented Aug 7, 2023

Received note that this Allpass has been merged into main.
Stoked to see what people will make with it.

I'm keen to have a look at how the documentation turns out based on the files from my branch. I'm still not quite sure how that's meant to update to the website but I'm sure it will turn up soon.

Thanks again @kevinstadler for all your hard work.

@pixmusix pixmusix closed this as completed Aug 7, 2023
@kevinstadler kevinstadler added the enhancement New feature or request label Sep 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants