GoogleChromeLabs / audioworklet-polyfill

🔊 Polyfill AudioWorklet using the legacy ScriptProcessor API.
https://googlechromelabs.github.io/audioworklet-polyfill/
Apache License 2.0
196 stars 20 forks source link

'currentTime' and 'sampleRate' are not set correctly before and between 'process' calls #14

Closed JohnWeisz closed 4 years ago

JohnWeisz commented 6 years ago

This is a follow-up of https://github.com/GoogleChromeLabs/audioworklet-polyfill/issues/7 which was claimed to have been fixed in https://github.com/GoogleChromeLabs/audioworklet-polyfill/commit/eb0840cc7745954d307235efa32b5e7782aa824e. Unfortunately, the problem with the current approach is that sampleRate is not defined until the first process call, so any instantiation-time use of it will yield an incorrect outcome (e.g. you can't use it in the constructor).

The same is true for currentTime, and although it's less of a problem here, it's still tied to process calls directly, which is incorrect. For example, for a case where the main-thread AudioWorkletNode posts messages to the AudioWorkletProcessor, which is then supposed to respond with a message based on currentTime but without process being called (such as when disconnected), the outcome will be incorrect.


Instead, the Realm must be configured correctly with getters that return directly from the working AudioContext, something like the following (which was working fine for my use-case, but I'm unsure of what side-effects it has):

realm.js

export function Realm (scope, parentElement) {
  const frame = document.createElement('iframe');
  frame.style.cssText = 'position:absolute;left:0;top:-999px;width:1px;height:1px;';
  parentElement.appendChild(frame);
  const win = frame.contentWindow;
  const doc = win.document;
  let vars = 'var window,$hook';
+ let accessors = "";
  for (const i in win) {
    if (!(i in scope) && i !== 'eval') {
      vars += ',';
      vars += i;
    }
  }
  for (const i in scope) {
-   vars += ',';
-   vars += i;
-   vars += '=self.';
-   vars += i;
+   accessors += `Object.defineProperty(this,'${i}',{`;
+   accessors += `    get: function(){return self.${i};},`;
+   accessors += `    set: function(v){self.${i}=v;}`;
+   accessors += `});`;
  }
  const script = doc.createElement('script');
  script.appendChild(doc.createTextNode(
    `function $hook(self,console) {"use strict";
-       ${vars};return function() {return eval(arguments[0])}}`
+       ${vars};${accessors}return function() {return eval(arguments[0])}}`
  ));
  doc.body.appendChild(script);
  this.exec = win.$hook(scope, console);
}

polyfill.js

    addModule (url, options) {
      return fetch(url).then(r => {
        if (!r.ok) throw Error(r.status);
        return r.text();
      }).then(code => {
        const context = {
          sampleRate: 0,
          currentTime: 0,
          AudioWorkletProcessor () {
            this.port = nextPort;
          },
          registerProcessor: (name, Processor) => {
            const processors = getProcessorsForContext(this.$$context);
            processors[name] = {
              realm,
              context,
              Processor,
              properties: Processor.parameterDescriptors || []
            };
          }
        };

+       Object.defineProperty(context, "sampleRate", {
+         get: () => this.$$context.sampleRate
+       });
+
+       Object.defineProperty(context, "currentTime", {
+         get: () => this.$$context.currentTime
+       });

        context.self = context;
        const realm = new Realm(context, document.documentElement);
        realm.exec(((options && options.transpile) || String)(code));
        return null;
      });
    }

This will throw errors when trying to assign to sampleRate or currentTime (which I believe is correct), but let other assignments through.

developit commented 6 years ago

Your explanation makes sense. The this / self issue can also be corrected more simply:

-  this.exec = win.$hook(scope, console);
+  this.exec = win.$hook.call(scope, scope, console);