New issue
Advanced search Search tips

Issue 775567 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 469639



Sign in to add a comment

Consider refactoring thread reset process in BaseAudioContext

Project Member Reported by hongchan@chromium.org, Oct 17 2017

Issue description

audioWorklet.addModule() call and the construction of BaseAudioContext are orthogonal,
and there is an issue of thread switching in the middle of audio rendering.

Relevant CL: https://chromium-review.googlesource.com/c/chromium/src/+/722126

Currently the contexts are always resetting the destination (thread within) when
audioWorklet.addModule() is called. This is one-time operation, so it won't be
resetting the destination once the worklet messaging proxy is assigned to the
context, but it is still wasteful/redundant.

Consider refactoring this structure.

 
Status: Started (was: Assigned)
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1df805e3dc25af0ee0590467fa896c78aba34526

commit 1df805e3dc25af0ee0590467fa896c78aba34526
Author: Hongchan Choi <hongchan@chromium.org>
Date: Fri Nov 03 04:13:42 2017

Refactor thread reset process in DefaultAudioDestinationNode

This CL refactors the process of restarting the destination to fix the
crash when running "Hello AudioWorklet!" demo on the debug build.

Previously, when AudioWorklet.addModule() is called the context is
attached to AudioWorkletGlobalScope, and this eventually resets the
rendering thread. This process triggers "restarting destination".

Restarting destination even before the context starts rendering can
happen, and then the context starts the rendering later again. This
causes the destination to start twice, thus DCHECK() fails on the
second start call.

This patch rearranges the DCHECK location and does the gate keeping
on the destination state. In short, the multiple start/stops on the
destination will be ignored.

Demo: https://googlechromelabs.github.io/web-audio-samples/audio-worklet/basic/hello-audio-worklet.html

Bug:  775567 
Test: The demo page doesn't crash anymore on debug build with runtime flag.
Change-Id: I7d3a8df5ae7bb320031d281b7b5e8c5b1baeaa36
Reviewed-on: https://chromium-review.googlesource.com/749885
Commit-Queue: Hongchan Choi <hongchan@chromium.org>
Reviewed-by: Raymond Toy <rtoy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513676}
[modify] https://crrev.com/1df805e3dc25af0ee0590467fa896c78aba34526/third_party/WebKit/Source/modules/webaudio/AudioDestinationNode.h
[modify] https://crrev.com/1df805e3dc25af0ee0590467fa896c78aba34526/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp
[modify] https://crrev.com/1df805e3dc25af0ee0590467fa896c78aba34526/third_party/WebKit/Source/modules/webaudio/DefaultAudioDestinationNode.cpp
[modify] https://crrev.com/1df805e3dc25af0ee0590467fa896c78aba34526/third_party/WebKit/Source/modules/webaudio/DefaultAudioDestinationNode.h
[modify] https://crrev.com/1df805e3dc25af0ee0590467fa896c78aba34526/third_party/WebKit/Source/modules/webaudio/OfflineAudioDestinationNode.cpp
[modify] https://crrev.com/1df805e3dc25af0ee0590467fa896c78aba34526/third_party/WebKit/Source/modules/webaudio/OfflineAudioDestinationNode.h

Status: Fixed (was: Started)

Sign in to add a comment