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

create node with no output? #265

Closed
kn0ll opened this issue Nov 16, 2020 · 12 comments
Closed

create node with no output? #265

kn0ll opened this issue Nov 16, 2020 · 12 comments

Comments

@kn0ll
Copy link
Contributor

kn0ll commented Nov 16, 2020

is it possible to create a node with no output? it seems like it is explicitly disallowed, but the spec respects the option. for instance an AnalyserNode may exist with no output.

@kn0ll
Copy link
Contributor Author

kn0ll commented Nov 18, 2020

i do see now that mediaStreamDestination allows no output and is handled right above the code i linked to. so i am thinking that virtual-audio-graph treats no output as an exception, and we could handle Analyser the same way? when i get back to it, i will attempt to use StandardVirtualAudioNode directly and see what luck i have. thanks!

@vincerubinetti
Copy link

Did you ever find a way to do this?

In my case, I'm trying to show a frequency analyzer of the user's mic signal, which they've already allowed via browser permissions, but before they've clicked anywhere on the page to allow playing of sounds.

The only way I can think to do this is to send the analyzer to no destination before the user has given a gesture, then hook it up to the audio context destination once they have. I tried the mediaStreamDestination like this, but it didn't work.

{
      noDest: mediaStreamDestination(),
      stream: mediaStreamSource("analyzer", { mediaStream: micStream }),
      analyzer: analyser("noDest", { fftSize }),
}

@benji6
Copy link
Owner

benji6 commented Jan 24, 2025

@vincerubinetti when you say it didn't work what behavior were you looking for? In the example you have shared I don't think there is any way to know whether it has worked or not. Are you trying to access any of the underlying AudioNodes? Something else?

@vincerubinetti
Copy link

vincerubinetti commented Jan 24, 2025

Sorry, it was late at night and I definitely didn't give enough detail.

If I provide no output to any of the nodes, e.g. gain(null/undefined/"", {}), the library errors. If I do the "no destination" method I mentioned above, it seems like the library tries to start the audio context, triggering a The AudioContext was not allowed to start. It must be resumed (or created) after a user gesture on the page. warning, which beyond cluttering the console logs, also seems to prevent any other part of the graph from working. It could be an issue with my code or something else though.

Here's a fuller example of what I'm doing (still much is omitted), using this library with React:

const AudioGraph = () => {
  /** audio nodes */
  const graph = useGraph(sampleRate);

  // coming from a getUserMedia call, MediaStream | null
  const { micStream } = useMicrophone();

  useEffect(() => {
    if (!graph) return;
    if (!micStream) return;
    /** reset audio graph */
    graph.update({});
    /** update audio graph */
    graph.update({
      noDest: mediaStreamDestination(),
      stream: mediaStreamSource("analyzer", { mediaStream: micStream }),
      analyzer: analyser("noDest", { fftSize: 2 ** 10 }),
    });
  }, [graph, micStream]);
  
  return <></>;
}

/** use audio graph */
export const useGraph = (sampleRate: number) => {
  const [graph, setGraph] = useState<VirtualAudioGraph>();

  /** create */
  const init = useCallback(() => {
    const context = new AudioContext({ sampleRate });
    setGraph(
      createVirtualAudioGraph({
        audioContext: context,
        output: context.destination,
      }),
    );
  }, [sampleRate]);

  // init on mount
  useEffect(() => {
    init();
  }, [init]);

  // OR...

  /** init on user gesture */
  // useEventListener("click", init);
  // useEventListener("mousedown", init);
  // useEventListener("touchstart", init);
  // useEventListener("keydown", init);

  return graph;
};

Notice the commented out user-gesture listeners. Adding those in instead of the "init on mount" makes everything work every time (after the user clicks/touches/whatever). If I just try to init on component mount, sometimes it works, sometimes it doesn't.

I am using React, and if you're familiar, strict mode makes useEffects run twice in development, which perhaps is causing some race condition, though I don't get why making two AudioContexts would cause issues.

I also tried suspend()ing the AudioContext immediately after creation (and waiting for the suspend to be successful before passing the context to the library), but I was still getting the user-gesture warning.

Still looking into this issue. Any insight would be appreciated. Does the library attempt to play an AudioContext immediately when it receives one? Perhaps I could set the output of the virtual audio graph later, once I'm ready to actually output audio to the speakers?

@vincerubinetti
Copy link

vincerubinetti commented Jan 24, 2025

Sorry, I believe I've figured it out and it's probably (?) not related to the library.

Basically, the creation of the AudioContext inside useGraph was getting called before the getUserMedia request for the user's mic was complete. I don't understand why the web audio API would require that to be completed even before creating an AudioContext... Note that in the code above I wasn't even hooking up anything to the context with graph.update until micStream is defined.

Maybe the web audio API is just being weird. Maybe the Chrome warning message is misleading. Or maybe something in createVirtualAudioGraph (before any call to graph.update occurs) makes the web audio api throw an error...

Bleh, I feel like I'm losing my mind.

Anyway, I was able to "work around" (maybe it's actually intended order of operations by the designers of the spec) is to wait until the mic permission request completes before even creating the audio context and virtual audio graph.

I guess the browser takes the user granting microphone permission as also a user-gesture that can start an audio context. And if the user approves the mic permission and refreshes the page, the mic permission is saved and audio can immediately start with no user interaction.

So, sorry, I think my issue is unrelated to this issue. Still though... perhaps the library warrants an explicit "no output" option.

@benji6
Copy link
Owner

benji6 commented Jan 25, 2025

Yes, I agree that you should be able to call getUserMedia before creating the audio context.

createVirtualAudioGraph has no side effects so that shouldn't be causing the issue (the code is simple https://github.com/benji6/virtual-audio-graph/blob/master/src/index.ts it instantiates a class which has no side effects in the constructor https://github.com/benji6/virtual-audio-graph/blob/master/src/VirtualAudioGraph.ts#L8-L11)

I thought it might be some kind of race condition or React calling hooks twice, but the code you shared looks OK to me.

Happy to rubber duck further if you run into issues, hopefully you have something working now :)

@vincerubinetti
Copy link

vincerubinetti commented Jan 26, 2025

Thanks for the sanity check on my bugs, and sorry for hijacking this issue.

I do agree with @kn0ll though that the library should allow no output. Was there a reason you have it throw an error on no output, other than trying to protect users against connecting the nodes to nothing when they probably didn't mean to? I think perhaps it should be a warning or something. Or alternatively, there could be another special key (like "output"), e.g. "no-output".

It seems like we can use mediaStreamDestination as a workaround, but that's not really what it's intended for?

Here's the basic audio graph of my app, which I think could be quite common, and has several places where you don't want any output:

flowchart TD
    M[microphone MediaStream]
    A[spectrum analyzer]
    R[recorder AudioWorklet #lpar;save audio to internal app state#rpar;]
    P[#quot;playthrough#quot; to speakers?]
    T[already-recorded BufferSources #lpar;get from internal app state#rpar;]
    V[volume gain]
    O[output #lpar;speakers#rpar;]
    M --> A
    M --> R
    M --> P
    P --> V
    T --> V
    V --> O
Loading

@benji6
Copy link
Owner

benji6 commented Jan 26, 2025

@vincerubinetti I think you're right that there is a valid need for this (e.g. with analyser nodes). Let me have a think about how best to implement this, I expect you are right that another special key is the best way forwards. Should be able to push a new version in the next few days

benji6 added a commit that referenced this issue Jan 29, 2025
@benji6
Copy link
Owner

benji6 commented Jan 29, 2025

I've added a new release to address this https://github.com/benji6/virtual-audio-graph/releases/tag/v1.3.0.

In fffa20a I've added a new export to the library NO_OUTPUT that you can use when you don't want to connect a node to anything. I decided to do this with a Symbol under the hood so this isn't a breaking change. I've also added an OUTPUT export which you can use instead of the "output" string but it is backwards compatible and both work.

I've also updated the docs https://virtual-audio-graph.netlify.app/

Let me know if you have any feedback.

@vincerubinetti
Copy link

Awesome! Thanks for the quick change. I'm assuming the "OUTPUT export" is also a symbol, I think that's a better way to do it. I'll try this out tonight.

@vincerubinetti
Copy link

Just tested it out, and it seems to work! I feel like you could close this issue, and if I or @kn0ll run into some edge case issue we can just re-comment or open a new issue.

You're the GOAT.

@benji6 benji6 closed this as completed Jan 29, 2025
@benji6
Copy link
Owner

benji6 commented Jan 29, 2025

The OUTPUT export is still the string "output" for backwards compatibility. Reading your comment it occurs to me that I could make it a symbol and still retain support for the string "output". I don't think it makes much difference in practice.

There's a TODO for updating it to be a symbol in the code for the next major version bump https://github.com/benji6/virtual-audio-graph/blob/master/src/constants.ts#L3

Thanks for all your helpful feedback!

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

No branches or pull requests

3 participants