chrisguttandin / standardized-audio-context

A cross-browser wrapper for the Web Audio API which aims to closely follow the standard.
MIT License
679 stars 33 forks source link

Safari 14.5 Support #977

Closed thecynicalpaul closed 3 years ago

thecynicalpaul commented 3 years ago

Hi there! First and foremost would like to thank you for the awesome project. It has been incredibly helpful. As you may know, Apple released major update to Safari a couple of days ago, which enables their experimental AudioWorklets by default. The problem is that they simply... don't work. I am able to send and receive messages, but no logging is visible from inside the Processor modules, and it's virtually impossible to debug it. Attempting to write data into a Float32Array and then deliver its .buffer as a Transferable over worklet's port just returns an empty array.

Was wondering if there are any workaround/fixes or if it's possible to remove global scope defined by Safari before SAC loads so that its compatibility mode works (which it does if you disable the experimental feature, which we can't really tell our clients to do, especially on iOS)

TIA!

thecynicalpaul commented 3 years ago

Upon further inspection, it seems like the issue is in the wrapping process(i,o,p) class member. It doesn't get to execute super.process(...) and crashes beforehand, triggering onprocessorerror (which is not helpful at all).

thecynicalpaul commented 3 years ago

Upon further investigation, the problem is in bundling - bug fix wrappers, like patchedSourceWithoutImportSatements simply don't work in webpack. They look for class extension such as ... extends AudioWorkletProcessor { however, webpack has its own way of structuring things, especially if you are using Babel. This is the case for both Firefox and Safari, as they fail the isSupportingPostMessage check. Advice on how to work around this would be super helpful!

chrisguttandin commented 3 years ago

Hi @thecynicalpaul,

unfortunately I had no time yet to update the code for Safari v14.1. I made some tests while the new code in Safari was still behind a flag and found some edge case that would need to be fixed. But since it was considered to be experimental at this point I did not really bother.

I'm not sure if I understand your comment about webpack and Babel. Do you bundle your worklet source code? If so I would expect it to not contain any import statements anymore. Could share an example worklet that you're using?

thecynicalpaul commented 3 years ago

Hi @chrisguttandin Thanks for looking into this!

Yeah I bundle both worklet and main code, but they are in separate chunks obviously. So essentially what happens is when webpack bundles things, it replaces imports and class definitions with its own macros like __WEBPACK_MODULE_etc, and since there is a regex in the code that looks for ...extends AudioWorkletProcessor things like __buffer and the Transferable compatibility patch for Firefox (and now Safari apparently) doesn't get injected into the worklet class code. And as a result of that, when the wrapping process function gets called, it tries to access __b to store the Transferables in, and fails because it's not defined.

The code is fairly basic as far as worklets go, (also cannot share due to it being a commercial project), but I've figured out a temporary work-around. (using Typescript) Adding private __b = new WeakSet(); at the top of your own processor class prevents the wrapping process(i,o,p) from breaking, like so:

class MyProcessor extends AudioWorkletProcessor {
  private __b = new WeakSet();

  constructor(options){
    super(options);
    // do postMessage override code here, and maybe pass isSupportingPostMessage in the processorOptions
  }

  public process(...){ ... }
}

Perhaps a better alternative would be to have the AudioWorkletProcessor class custom defined so that it already comes with these, or something along the lines. TLDR: regex compatibility replacement doesn't work because webpack changes class definition code.

chrisguttandin commented 3 years ago

Could you maybe configure Babel for your worklet chunk to not transpile class definitions? I think it's safe to do so since all AudioWorklet implementations support class definitions.

thecynicalpaul commented 3 years ago

It's probably possible, but definitely far from ideal - as well as some es6 features aren't supported universally yet. My current workaround is to have a wrapper class like so:

export class ExtendedAudioWorkletProcessor extends AudioWorkletProcessor {
  private __b: WeakSet<any> | null = null;

  constructor({
    isSupportingPostMessage = true,
    ...nativeOptions
  }: ExtendedAudioWorkletNodeOptions = {}) {
    super(nativeOptions);

    if (!isSupportingPostMessage) { return; }
    this.extendPostMessageTransferrableSupport();
  }

  // ===========================================================================
  // Private members
  // ===========================================================================
  private extendPostMessageTransferrableSupport() {
    this.__b = new WeakSet();
    this.port.postMessage = ((
      postMessage: AudioWorkletProcessorPostMessage
    ) => (
      message: any,
      transferableList: Transferable[]
    ) => {
      const filteredTransferableList = (transferableList)
        ? transferableList.filter(transferable => !this.__b?.has(transferable))
        : transferableList;
      return postMessage.call(
        this.port,
        message,
        filteredTransferableList as any
      );
    })(this.port.postMessage) as AudioWorkletProcessorPostMessage;
  }
}

Where I have a carbon copy of testAudioWorkletProcessPostMessageSupport passing its result into the constructor. While I understand the aspirations under this library, I think it'd be really nice to have access to some of the helper factories as public APIs.

Also, just an FYI, creating an offline audio context with samplerate of anything below 46100 (or something along the lines) doesn't work in iOS Safari.

chrisguttandin commented 3 years ago

It's possible to configure Babel to not transpile a certain feature. You don't have to disable all es6 features. Any browser that is supported by standardized-audio-context also supports the module syntax. It's also a requirement for the ScriptProcessor fallback for browsers without AudioWorklet support.

Maybe I'm missing something but why isn't it ideal not to transpile something that the browser supports natively?

As mentioned in the README Safari has some limitations when it comes to the sampleRate of an OfflineAudioContext.

⚠️ Safari does not support creating an OfflineAudioContext with more than 10 channels or with a sampleRate below 44100 Hz.

But it looks like v14.5 supports anything until 3000 Hz.

thecynicalpaul commented 3 years ago

We are using Next.js which is running on top of webpack and does SSR, and apparently webpack on node doesn't deal well with some es6 features, that's why we have to go through this awkward route. We are actually transpiling standardized-audio-context as well because of that.

And yup. Although creating OfflineContext below 44100 on 14.5 on iOS fails. The reason I brought this up was because I noticed that the compatibility patch for transferables is using offline context with a rather small sample rate, so in case someone had unexpected results on 14.5 (since it now supports worklets) that would explain it.

chrisguttandin commented 3 years ago

Okay, I understand. Sorry for making that super-clever comment about selective transpilation with Babel. I didn't know that doing SSR with Next.js prevents that.

I pushed a fix now which injects the patched AudioWorkletProcessor with an immediately invoked function. Not sure why I didn't do that in the first place. That way the regex isn't necessary anymore. I'm pretty sure it's immune against any down-level transpilation as well. Please let me know if it doesn't work.

I also updated all internal OfflineAudioContexts to use a sampleRate of 44100 Hz.

Also Safari v14.5 is now generally supported.

Again please let me know if you're still experiencing any issues. And thanks again for making me aware of the shortcomings of the regex approach.

thecynicalpaul commented 3 years ago

Thanks for that! This library has been a life-saver, and really appreciate all the work you've done.

chrisguttandin commented 3 years ago

Thanks for your nice words. I'm happy that it proves to be useful.