New issue
Advanced search Search tips

Issue 605726 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Remove connection from AudioParamTimeline to AudioContext

Project Member Reported by rtoy@chromium.org, Apr 21 2016

Issue description

Fix this todo item: https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/modules/webaudio/AudioParam.cpp&l=40

A peek indicates that the AudioParamTimeline needs the context to get the sample rate and current time/frame.  The sample rate is a constant once the context is created, so AudioParamTimeline can be updated to take that as an argument in the constructor (or something).

The current frame (m_currentSampleFrame) is maintained by AudioDestinationNode.  So it seems that AudioParamTimeline can just have a pointer to the AudioDestinationNode handler, we can remove the dependency on the AudioContext from AudioParamTimeline.  (The timeline objects are not part of Oilpan and neither the AudioDestinationHandler.)
 

Comment 1 by rtoy@chromium.org, Apr 21 2016

Cc: tkent@chromium.org
tkent:  If AudioParamHandler and AudioParamTimeline access the AudioDestinationHandler instead of the AudioContext, will that work so we can get rid of the TODO?

Comment 2 by tkent@chromium.org, Apr 22 2016

Yes if it won't make reference cycle.

Project Member

Comment 3 by bugdroid1@chromium.org, Apr 27 2016

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

commit f1afe5ba38f00c14f9982c8d0b3afd598865f411
Author: rtoy <rtoy@chromium.org>
Date: Wed Apr 27 20:53:50 2016

Remove connection from AudioParamHandler to AudioContext.

This removes the connection from AudioParamHandler and
AudioParamTimeline to AudioContext.  This is achieved by having
AudioParamHandler and AudioParamTimeline access the
AudioDestinationHandler instead of the AudioContext.
AudioDestinationHandler has the necessary sampleRate(),
currentSampleFrame(), and currentTime() methods that AudioParamHandler
and AudioParamTimeline need.

BUG= 605726 
TEST=none

Review-Url: https://codereview.chromium.org/1911133002
Cr-Commit-Position: refs/heads/master@{#390179}

[modify] https://crrev.com/f1afe5ba38f00c14f9982c8d0b3afd598865f411/third_party/WebKit/Source/modules/webaudio/AudioParam.cpp
[modify] https://crrev.com/f1afe5ba38f00c14f9982c8d0b3afd598865f411/third_party/WebKit/Source/modules/webaudio/AudioParam.h
[modify] https://crrev.com/f1afe5ba38f00c14f9982c8d0b3afd598865f411/third_party/WebKit/Source/modules/webaudio/AudioParamTimeline.cpp
[modify] https://crrev.com/f1afe5ba38f00c14f9982c8d0b3afd598865f411/third_party/WebKit/Source/modules/webaudio/AudioParamTimeline.h

Comment 4 by rtoy@chromium.org, May 2 2016

Owner: rtoy@chromium.org
Status: Fixed (was: Available)

Sign in to add a comment