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

[step-sequencer] The slider inputs to set the envelope of the "Sweep" sound are not working as expected #117

Open
ManuLintz opened this issue Dec 12, 2023 · 3 comments · May be fixed by #123
Labels
accepting PR Feel free to address this with a pull request

Comments

@ManuLintz
Copy link

ManuLintz commented Dec 12, 2023

What information was incorrect, unhelpful, or incomplete?

The two sliders to set the envelope of the "Sweep" sound are not working as expected.
image

What did you expect to see?

There are actually 2 issues:

  1. For both inputs, the values are not updated properly. The input should update the value progressively with a 0.1 step, but it only updates to 0 or 1. The problem comes from that the value is cast into an int in the event listener callback.
const attackControl = document.querySelector("#attack");
    attackControl.addEventListener(
        "input",
        (ev) => {
             attackTime = parseInt(ev.target.value, 10);
        },
        false
);

Since their value are between 0 and 1 with a step of 0.1, the usage of "parseInt" is inadecuate.

<section class="controls">
  <label for="attack">Att</label>
  <input
    name="attack"
    id="attack"
    type="range"
    min="0"
    max="1"
    value="0.2"
    step="0.1"
  />
  <label for="release">Rel</label>
  <input
    name="release"
    id="release"
    type="range"
    min="0"
    max="1"
    value="0.5"
    step="0.1"
  />
</section>
  1. The way the release input works is not correct. When the slider is set to left, the release should be 0, whereas when it's set to the right, we should be able to hear the release.
    Your can try for yourself here:
    If you set the values like this you hear a sound that is plucked:
image

When you set it like this you hear a longer release:

image

It should be the other way around

The problem comes from the implementation of the envelope:

sweepEnv.gain.linearRampToValueAtTime(
  0,
  time + attackTime - releaseTime // here it shouldn't be a "-" operator
);

I'm currently going through the Web API guide where this step sequencer is used: https://developer.mozilla.org/en-US/docs/Web/API/Web_Audio_API/Advanced_techniques

Since I identified the what causes the issue in the code I'd be happy to work on a fix + updating the doc.

Do you have any supporting links, references, or citations?

No response

Do you have anything more you want to share?

No response

@ManuLintz ManuLintz added the needs triage Triage needed by staff and/or partners. Automatically applied when an issue is opened. label Dec 12, 2023
Copy link

It looks like this is your first issue. Welcome! 👋
One of the project maintainers will be with you as soon as possible. We
appreciate your patience. To safeguard the health of the project, please
take a moment to read our code of conduct.

@bsmth
Copy link
Member

bsmth commented Jan 25, 2024

Thanks for reporting this (along with the other one). Would you like to open a PR to improve it?

@bsmth bsmth added accepting PR Feel free to address this with a pull request and removed needs triage Triage needed by staff and/or partners. Automatically applied when an issue is opened. labels Jan 25, 2024
@ManuLintz
Copy link
Author

Yes I'll do it, thanks for your answer !

@ManuLintz ManuLintz linked a pull request Jan 25, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepting PR Feel free to address this with a pull request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants