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

Geometer: use a lower-bound threshold for "at level" operation #23

Merged
merged 1 commit into from
Oct 15, 2020

Conversation

sophiapoirier
Copy link
Owner

The only remaining compiler warning for me in our codebase has been this unused variable level in Geometer. Looking at it in context, I think the intention was to provide a lower-bounds audio threshold within which the "between" state for "at level" point generation is detected? I think without this and with the point parameter slider at its minimum, an audio sample is only "between" when it is exactly zero, and then with this scaling anything between -.00005 and .00005 (about -86 dB) will be deemed "between".

I played around both ways, and honestly I couldn't hear a difference with the audio material I was using. But I'm not really sure what the auditory effect of the "between" state is supposed to sound like.

I'm guessing that either you played with having the .00005 threshold and then changed your mind, or wanted the .00005 threshold and then didn't actually integrate it into the logic, but I'm not sure which! If it already is as intended, then I'll just update this PR to delete the unused variable instead.

Anyway, just very minor tidying, but thought to run it by you if you have any memory or opinions on this.

@tom7
Copy link
Collaborator

tom7 commented Oct 15, 2020

Yeah, I think it makes sense to have the pointparam do something for this pointstyle. I think the idea behind BETWEEN was to provide some hysteresis, but yes, no idea what that is "supposed" to sound like :)

@tom7 tom7 merged commit da6f883 into main Oct 15, 2020
@tom7 tom7 deleted the geometer_level branch October 15, 2020 03:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants