Avoid memory allocation in AudioWorkletProcessor.process()

Based on the spec [1], the current implementation of
AudioWorkletProcessor needs to create a new data container
(i.e. WebIDL sequence<>) for input, output, and param arrays.

With the new spec change [2], this CL changes the overall design
of the audio processing callback:

1. Moves the processing call from AudioWorkletGlobalScope to
   AudioWorkletProcessor object.
2. AudioWorkletProcessor now keeps the data container within
   the object and allocate memory when it is needed.

The preliminary benchmark shows the sizable improvement in the
audio stream quality. The glitch score
(= buffer underrun/total callback) is improved by ~9x in the
low-tier machine. [3]

This is an API change [4], but the real world impact would be
negligible because there's no functionality change.

[1]: https://webaudio.github.io/web-audio-api/#dom-audioworkletprocessor-process-inputs-outputs-parameters-inputs
[2]: WebAudio/web-audio-api#1933 (comment)
[3]: https://bugs.chromium.org/p/chromium/issues/detail?id=1086665#c2
[4]: https://chromestatus.com/feature/5647541725036544


Bug: 1071085, 1086665
Change-Id: I3e664754973d4d86649d38c1807c6b9d7830fb96
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2218702
Reviewed-by: Raymond Toy <[email protected]>
Reviewed-by: Yuki Shiino <[email protected]>
Commit-Queue: Hongchan Choi <[email protected]>
Cr-Commit-Position: refs/heads/master@{#779052}

Author: hoch

third_party/blink/renderer/modules/webaudio/audio_worklet_global_scope.cc

@@ -12,24 +12,13 @@
 #include "third_party/blink/renderer/bindings/core/v8/idl_types.h"
 #include "third_party/blink/renderer/bindings/core/v8/native_value_traits_impl.h"
 #include "third_party/blink/renderer/bindings/core/v8/serialization/serialized_script_value.h"
-#include "third_party/blink/renderer/bindings/core/v8/to_v8_for_core.h"
-#include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_core.h"
 #include "third_party/blink/renderer/bindings/core/v8/worker_or_worklet_script_controller.h"
-#include "third_party/blink/renderer/bindings/modules/v8/v8_audio_param_descriptor.h"
 #include "third_party/blink/renderer/bindings/modules/v8/v8_audio_worklet_processor.h"
 #include "third_party/blink/renderer/bindings/modules/v8/v8_blink_audio_worklet_process_callback.h"
 #include "third_party/blink/renderer/bindings/modules/v8/v8_blink_audio_worklet_processor_constructor.h"
-#include "third_party/blink/renderer/core/messaging/message_port.h"
-#include "third_party/blink/renderer/core/typed_arrays/dom_typed_array.h"
-#include "third_party/blink/renderer/core/workers/global_scope_creation_params.h"
 #include "third_party/blink/renderer/core/workers/worker_thread.h"
-#include "third_party/blink/renderer/modules/webaudio/audio_buffer.h"
-#include "third_party/blink/renderer/modules/webaudio/audio_worklet_processor.h"
 #include "third_party/blink/renderer/modules/webaudio/audio_worklet_processor_definition.h"
-#include "third_party/blink/renderer/modules/webaudio/audio_worklet_processor_error_state.h"
 #include "third_party/blink/renderer/modules/webaudio/cross_thread_audio_worklet_processor_info.h"
-#include "third_party/blink/renderer/platform/audio/audio_bus.h"
-#include "third_party/blink/renderer/platform/audio/audio_utilities.h"
 #include "third_party/blink/renderer/platform/bindings/callback_method_retriever.h"
 
 namespace blink {
@@ -217,180 +206,6 @@ AudioWorkletProcessor* AudioWorkletGlobalScope::CreateProcessor(
   return processor;
 }
 
-bool AudioWorkletGlobalScope::Process(
-    AudioWorkletProcessor* processor,
-    Vector<scoped_refptr<AudioBus>>* input_buses,
-    Vector<AudioBus*>* output_buses,
-    HashMap<String, std::unique_ptr<AudioFloatArray>>* param_value_map) {
-  CHECK_GE(input_buses->size(), 0u);
-  CHECK_GE(output_buses->size(), 0u);
-
-  ScriptState* script_state = ScriptController()->GetScriptState();
-  ScriptState::Scope scope(script_state);
-
-  v8::Isolate* isolate = script_state->GetIsolate();
-  v8::Local<v8::Context> current_context = script_state->GetContext();
-  AudioWorkletProcessorDefinition* definition =
-      FindDefinition(processor->Name());
-  DCHECK(definition);
-
-  v8::TryCatch try_catch(isolate);
-  try_catch.SetVerbose(true);
-
-  // Prepare arguments of JS callback (inputs, outputs and param_values) with
-  // directly using V8 API because the overhead of
-  // ToV8(HeapVector<HeapVector<DOMFloat32Array>>) is not negligible and there
-  // is no need to externalize the array buffers.
-
-  // 1st arg of JS callback: inputs
-  v8::Local<v8::Array> inputs = v8::Array::New(isolate, input_buses->size());
-  uint32_t input_bus_index = 0;
-  for (auto input_bus : *input_buses) {
-    // If |input_bus| is null, it means the input port is not connected, and
-    // the corresponding array for that input port should have zero channel.
-    unsigned number_of_channels = input_bus ? input_bus->NumberOfChannels() : 0;
-    size_t bus_length = input_bus ? input_bus->length() : 0;
-
-    v8::Local<v8::Array> channels = v8::Array::New(isolate, number_of_channels);
-    bool success;
-    if (!inputs
-             ->CreateDataProperty(current_context, input_bus_index++, channels)
-             .To(&success)) {
-      return false;
-    }
-    for (uint32_t channel_index = 0; channel_index < number_of_channels;
-         ++channel_index) {
-      v8::Local<v8::ArrayBuffer> array_buffer =
-          v8::ArrayBuffer::New(isolate, bus_length * sizeof(float));
-      v8::Local<v8::Float32Array> float32_array =
-          v8::Float32Array::New(array_buffer, 0, bus_length);
-      if (!channels
-               ->CreateDataProperty(current_context, channel_index,
-                                    float32_array)
-               .To(&success)) {
-        return false;
-      }
-      const v8::ArrayBuffer::Contents& contents = array_buffer->GetContents();
-      if (input_bus) {
-        memcpy(contents.Data(), input_bus->Channel(channel_index)->Data(),
-               bus_length * sizeof(float));
-      }
-    }
-  }
-
-  // 2nd arg of JS callback: outputs
-  v8::Local<v8::Array> outputs = v8::Array::New(isolate, output_buses->size());
-  uint32_t output_bus_counter = 0;
-
-  // |output_array_buffers| stores underlying array buffers so that we can copy
-  // them back to |output_buses|.
-  Vector<Vector<v8::Local<v8::ArrayBuffer>>> output_array_buffers;
-  output_array_buffers.ReserveInitialCapacity(output_buses->size());
-
-  for (auto* const output_bus : *output_buses) {
-    output_array_buffers.UncheckedAppend(Vector<v8::Local<v8::ArrayBuffer>>());
-    output_array_buffers.back().ReserveInitialCapacity(
-        output_bus->NumberOfChannels());
-    v8::Local<v8::Array> channels =
-        v8::Array::New(isolate, output_bus->NumberOfChannels());
-    bool success;
-    if (!outputs
-             ->CreateDataProperty(current_context, output_bus_counter++,
-                                  channels)
-             .To(&success)) {
-      return false;
-    }
-    for (uint32_t channel_index = 0;
-         channel_index < output_bus->NumberOfChannels(); ++channel_index) {
-      v8::Local<v8::ArrayBuffer> array_buffer =
-          v8::ArrayBuffer::New(isolate, output_bus->length() * sizeof(float));
-      v8::Local<v8::Float32Array> float32_array =
-          v8::Float32Array::New(array_buffer, 0, output_bus->length());
-      if (!channels
-               ->CreateDataProperty(current_context, channel_index,
-                                    float32_array)
-               .To(&success)) {
-        return false;
-      }
-      output_array_buffers.back().UncheckedAppend(array_buffer);
-    }
-  }
-
-  // 3rd arg of JS callback: param_values
-  v8::Local<v8::Object> param_values = v8::Object::New(isolate);
-  for (const auto& param : *param_value_map) {
-    const String& param_name = param.key;
-    const AudioFloatArray* param_array = param.value.get();
-
-    // If the AudioParam is constant, then the param array should have length 1.
-    // Manually check to see if the parameter is truly constant.
-    unsigned array_size = 1;
-
-    for (unsigned k = 1; k < param_array->size(); ++k) {
-      if (param_array->Data()[k] != param_array->Data()[0]) {
-        array_size = param_array->size();
-        break;
-      }
-    }
-
-    v8::Local<v8::ArrayBuffer> array_buffer =
-        v8::ArrayBuffer::New(isolate, array_size * sizeof(float));
-    v8::Local<v8::Float32Array> float32_array =
-        v8::Float32Array::New(array_buffer, 0, array_size);
-    bool success;
-    if (!param_values
-             ->CreateDataProperty(current_context,
-                                  V8String(isolate, param_name.IsolatedCopy()),
-                                  float32_array)
-             .To(&success)) {
-      return false;
-    }
-    const v8::ArrayBuffer::Contents& contents = array_buffer->GetContents();
-    memcpy(contents.Data(), param_array->Data(), array_size * sizeof(float));
-  }
-
-  // Perform JS function process() in AudioWorkletProcessor instance. The actual
-  // V8 operation happens here to make the AudioWorkletProcessor class a thin
-  // wrapper of v8::Object instance.
-  ScriptValue result;
-  if (!definition->ProcessFunction()
-           ->Invoke(processor, ScriptValue(isolate, inputs),
-                    ScriptValue(isolate, outputs),
-                    ScriptValue(isolate, param_values))
-           .To(&result)) {
-    // process() method call failed for some reason or an exception was thrown
-    // by the user supplied code. Disable the processor to exclude it from the
-    // subsequent rendering task.
-    processor->SetErrorState(AudioWorkletProcessorErrorState::kProcessError);
-    return false;
-  }
-
-  // Copy |sequence<sequence<Float32Array>>| back to the original
-  // |Vector<AudioBus*>|. While iterating, we also check if the size of backing
-  // array buffer is changed. When the size does not match, silence the buffer.
-  for (uint32_t output_bus_index = 0; output_bus_index < output_buses->size();
-       ++output_bus_index) {
-    AudioBus* output_bus = (*output_buses)[output_bus_index];
-    for (uint32_t channel_index = 0;
-         channel_index < output_bus->NumberOfChannels(); ++channel_index) {
-      const v8::ArrayBuffer::Contents& contents =
-          output_array_buffers[output_bus_index][channel_index]->GetContents();
-      const size_t size = output_bus->length() * sizeof(float);
-      if (contents.ByteLength() == size) {
-        memcpy(output_bus->Channel(channel_index)->MutableData(),
-               contents.Data(), size);
-      } else {
-        memset(output_bus->Channel(channel_index)->MutableData(), 0, size);
-      }
-    }
-  }
-
-  // Return the value from the user-supplied |process()| function. It is
-  // used to maintain the lifetime of the node and the processor.
-  DCHECK(!try_catch.HasCaught());
-  return result.V8Value()->IsTrue();
-}
-
 AudioWorkletProcessorDefinition* AudioWorkletGlobalScope::FindDefinition(
     const String& name) {
   return processor_definition_map_.at(name);

third_party/blink/renderer/modules/webaudio/audio_worklet_global_scope.h

@@ -15,7 +15,6 @@
 
 namespace blink {
 
-class AudioBus;
 class AudioWorkletProcessor;
 class AudioWorkletProcessorDefinition;
 class CrossThreadAudioWorkletProcessorInfo;
@@ -77,14 +76,6 @@ class MODULES_EXPORT AudioWorkletGlobalScope final : public WorkletGlobalScope {
       MessagePortChannel,
       scoped_refptr<SerializedScriptValue> node_options);
 
-  // Invokes the JS audio processing function from an instance of
-  // AudioWorkletProcessor, along with given AudioBuffer from the audio graph.
-  bool Process(
-      AudioWorkletProcessor*,
-      Vector<scoped_refptr<AudioBus>>* input_buses,
-      Vector<AudioBus*>* output_buses,
-      HashMap<String, std::unique_ptr<AudioFloatArray>>* param_value_map);
-
   AudioWorkletProcessorDefinition* FindDefinition(const String& name);
 
   unsigned NumberOfRegisteredDefinitions();

third_party/blink/renderer/modules/webaudio/audio_worklet_global_scope_test.cc

@@ -296,7 +296,7 @@ class AudioWorkletGlobalScopeTest : public PageTestBase {
     EXPECT_TRUE(processor);
 
     Vector<scoped_refptr<AudioBus>> input_buses;
-    Vector<AudioBus*> output_buses;
+    Vector<scoped_refptr<AudioBus>> output_buses;
     HashMap<String, std::unique_ptr<AudioFloatArray>> param_data_map;
     scoped_refptr<AudioBus> input_bus =
         AudioBus::Create(1, kRenderQuantumFrames);

third_party/blink/renderer/modules/webaudio/audio_worklet_node.cc

@@ -95,7 +95,7 @@ void AudioWorkletHandler::Process(uint32_t frames_to_process) {
   // the rendering in the AudioWorkletGlobalScope.
   if (processor_ && !processor_->hasErrorOccured()) {
     Vector<scoped_refptr<AudioBus>> input_buses;
-    Vector<AudioBus*> output_buses;
+    Vector<scoped_refptr<AudioBus>> output_buses;
     for (unsigned i = 0; i < NumberOfInputs(); ++i) {
       // If the input is not connected, inform the processor of that
       // fact by setting the bus to null.
@@ -104,8 +104,7 @@ void AudioWorkletHandler::Process(uint32_t frames_to_process) {
       input_buses.push_back(bus);
     }
     for (unsigned i = 0; i < NumberOfOutputs(); ++i)
-      output_buses.push_back(Output(i).Bus());
-
+      output_buses.push_back(WrapRefCounted(Output(i).Bus()));
     for (const auto& param_name : param_value_map_.Keys()) {
       auto* const param_handler = param_handler_map_.at(param_name);
       AudioFloatArray* param_values = param_value_map_.at(param_name);