GoogleChromeLabs / web-audio-samples

Web Audio API samples by Chrome Web Audio Team
https://bit.ly/web-audio-samples
Apache License 2.0
686 stars 196 forks source link

Updates to PR353 implemented stop button #362

Closed duojet2ez closed 1 month ago

duojet2ez commented 4 months ago

[DO NOT MERGE] - we will break this down to multiple PRs.

I updated PR 353 with necessary changes for merge. Edited main.js files in:

noise-generator, one-pole-filter, volume-meter, hello-audio-worklet, audio-worklet-node-options

Specifically, I used audioContext.suspend() to allow the user to pause the sound with a stop button. And I added an isModuleLoaded check to avoid redundantly calling addModule. Also added isPlaying to toggle the playing state.

duojet2ez commented 4 months ago

@hoch ok all changes should be good. I tested locally... I am in the process of setting up a live test web page. Does github host that? Also... hopefully i am following protocol with git/github for making these changes

duojet2ez commented 4 months ago

@hoch Just wondering if you can give clarification on a live test page for this?

hoch commented 2 months ago

Apologies for missing your comment from May 31! Have you found a way of doing the live testing?

If you already have a fork, you should be able to deploy the tip-of-tree from the GitHub repo.

Lastly, this PR touches too many files and that makes it the progress more difficult. We should break this down to multiple smaller PRs and incrementally review/merge them. If you can minimize this PR down to 1 example project, what would it be?

duojet2ez commented 2 months ago

@hoch Thanks for the feedback! I tried to deploy my forked web-audio-samples website with heroku but was running into problems. I am assuming that's what you mean by "deploy" and "live testing"? I wasn't sure so I didn't spend too much time on that.

I think it's a great idea to break up into smaller PR's as this touches many files as you noticed. Can I make a PR just for the Hello Audio Worklet example?

hoch commented 2 months ago

I didn't meant to use the external tool like "heroku". Just like this repository, you should be able to serve the your forked repository on GitHub.

Yes. Focusing on a single example is definitely my preference.

duojet2ez commented 2 months ago

Great! I'll work on this

hoch commented 1 month ago

@duojet2ez Since now we merged the first part, I think we can break this down to multiple PRs and try to review/merge one by one. There will be small diffs on them, so the process will be much faster!

I'll also change the description/title to reflect the current state of this PR.

tarunsinghofficial commented 1 month ago

@duojet2ez Since now we merged the first part, I think we can break this down to multiple PRs and try to review/merge one by one. There will be small diffs on them, so the process will be much faster!

I'll also change the description/title to reflect the current state of this PR.

Hi @hoch!

I had a busy schedule earlier and saw that the original PR was closed. So, I'm interested in contributing to the PR's small parts. Let me know where to start with.

Thanks

duojet2ez commented 1 month ago

@tarunsinghofficial

The stop feature for the hello-audio-worklet example was just merged in. We have the following examples left:

Which would you like to work on?

Also the hello-audio-worklet example was approved with the following code

const audioContext = new AudioContext();
let isModuleLoaded = false;
let isPlaying = false;
let isGraphReady = false;
let oscillatorNode = null;

const loadGraph = (context) => {
  oscillatorNode = new OscillatorNode(context);
  const bypasser = new AudioWorkletNode(context, 'bypass-processor');
  oscillatorNode.connect(bypasser).connect(context.destination);
  oscillatorNode.start();
};

const startAudio = async (context) => {
  if (!isModuleLoaded) {
    await context.audioWorklet.addModule('bypass-processor.js');
    isModuleLoaded = true;
  }
  if (!isGraphReady) {
    loadGraph(audioContext);
    isGraphReady = true;
  }
};

// A simplem onLoad handler. It also handles user gesture to unlock the audio
// playback.
window.addEventListener('load', async () => {
  const buttonEl = document.getElementById('button-start');
  buttonEl.disabled = false;

  buttonEl.addEventListener('click', async () => {
    if (!isPlaying) {
      await startAudio(audioContext);
      isPlaying = true;
      buttonEl.textContent = 'Playing...';
      buttonEl.classList.remove('start-button');
      audioContext.resume();
    } else {
      audioContext.suspend();
      isPlaying = false;
      buttonEl.textContent = 'START';
      buttonEl.classList.add('start-button');
    }
  });
});

So that's the general format/naming convention we should probably continue to use. It shouldn't be too hard to apply that now to the other examples with minor changes Thanks!! Your idea for this was great.

hoch commented 1 month ago

@tarunsinghofficial Thanks for your continued interest. As @duojet2ez suggested, it would be great if we can work on each example one by one - and it would be much easier to make progress faster.

tarunsinghofficial commented 1 month ago

@tarunsinghofficial Thanks for your continued interest. As @duojet2ez suggested, it would be great if we can work on each example one by one - and it would be much easier to make progress faster.

Yes, got it. We can work one by one on example.

tarunsinghofficial commented 1 month ago

@tarunsinghofficial

The stop feature for the hello-audio-worklet example was just merged in. We have the following examples left:

  • noise-generator
  • one-pole-filter
  • volume-meter
  • audio-worklet-node-options

Which would you like to work on?

Also the hello-audio-worklet example was approved with the following code

const audioContext = new AudioContext();
let isModuleLoaded = false;
let isPlaying = false;
let isGraphReady = false;
let oscillatorNode = null;

const loadGraph = (context) => {
  oscillatorNode = new OscillatorNode(context);
  const bypasser = new AudioWorkletNode(context, 'bypass-processor');
  oscillatorNode.connect(bypasser).connect(context.destination);
  oscillatorNode.start();
};

const startAudio = async (context) => {
  if (!isModuleLoaded) {
    await context.audioWorklet.addModule('bypass-processor.js');
    isModuleLoaded = true;
  }
  if (!isGraphReady) {
    loadGraph(audioContext);
    isGraphReady = true;
  }
};

// A simplem onLoad handler. It also handles user gesture to unlock the audio
// playback.
window.addEventListener('load', async () => {
  const buttonEl = document.getElementById('button-start');
  buttonEl.disabled = false;

  buttonEl.addEventListener('click', async () => {
    if (!isPlaying) {
      await startAudio(audioContext);
      isPlaying = true;
      buttonEl.textContent = 'Playing...';
      buttonEl.classList.remove('start-button');
      audioContext.resume();
    } else {
      audioContext.suspend();
      isPlaying = false;
      buttonEl.textContent = 'START';
      buttonEl.classList.add('start-button');
    }
  });
});

So that's the general format/naming convention we should probably continue to use. It shouldn't be too hard to apply that now to the other examples with minor changes Thanks!! Your idea for this was great.

Great. I'll work on the leftover examples. Will check out all of them one by one.

Thanks for appreciating!

tarunsinghofficial commented 1 month ago

Hi @hoch @duojet2ez, I have updated the one-pole filter and noise-generator examples with the required changes. Let me know if they're correct. I hope it resolves the issues.

Also, will update the other two examples.

Thanks

tarunsinghofficial commented 1 month ago

Hi @hoch @duojet2ez

I have made the required changes to all 4 examples, and PR #390 has been created with all the updations. It's now ready to be merged.

Thanks