New issue
Advanced search Search tips

Issue 636576 link

Starred by 1 user
Status: Fixed
Owner:
rtoy@chromium.org
Closed: Sep 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

AudioNode.channelInterpretation not set if not connected

Project Member Reported by rtoy@chromium.org, Aug 10 2016 

OS: All

What steps will reproduce the problem?
(1) Run the following in the dev console (or wherever)

c = new AudioContext()
n = c.createGain()
n.channelInterpretation = "discrete"; console.log(n.channelInterpretation)

(2) It's really important that the last line be on one line.
(3) Now do
n.connect(c.destination)
(4) Wait a bit
console.log(n.channelInterpretation);

What is the expected output?

The console should print "discrete", "discrete"

What do you see instead?

"speakers", "discrete"

So, the channelInterpretation never gets set as expected unless the node is connected to the destination.

This change was done a while ago to fix a race condition where the audio thread needed the value while the main thread was changing it.  The change is now deferred until the end (or beginning) of the audio thread rendering phase.  Thus, this never gets changed if the node is not connected to the destination.

Comment 1 by rtoy@chromium.org, Aug 10 2016 

channelCountMode has the same issue.  For the same reason.

Comment 2 by rtoy@chromium.org, Aug 11 2016

Labels: -Pri-3 Pri-2
Owner: rtoy@chromium.org
Status: Started (was: Available)
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 16 2016 

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

commit e3860bd297e465778059f3d845280634b4074e19
Author: rtoy <rtoy@chromium.org>
Date: Tue Aug 16 18:00:18 2016

Don't delay setting of channelInterpretation and channelCountMode

Don't defer the setting of the values.  Instead, use atomic access for
these internal variables, adding appropriate getters and setters for
these.  This also allows us to remove the code that implemented the
deferral.

Ran the tests from  bug 613332  to make sure we didn't regress the race
conditions.  Tests still pass without issues.

BUG= 636576 
TEST=See  bug 613332  for tests

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

[modify] https://crrev.com/e3860bd297e465778059f3d845280634b4074e19/third_party/WebKit/Source/modules/webaudio/AudioNode.cpp
[modify] https://crrev.com/e3860bd297e465778059f3d845280634b4074e19/third_party/WebKit/Source/modules/webaudio/AudioNode.h
[modify] https://crrev.com/e3860bd297e465778059f3d845280634b4074e19/third_party/WebKit/Source/modules/webaudio/ChannelMergerNode.cpp
[modify] https://crrev.com/e3860bd297e465778059f3d845280634b4074e19/third_party/WebKit/Source/modules/webaudio/ConvolverNode.cpp
[modify] https://crrev.com/e3860bd297e465778059f3d845280634b4074e19/third_party/WebKit/Source/modules/webaudio/DefaultAudioDestinationNode.cpp
[modify] https://crrev.com/e3860bd297e465778059f3d845280634b4074e19/third_party/WebKit/Source/modules/webaudio/DeferredTaskHandler.cpp
[modify] https://crrev.com/e3860bd297e465778059f3d845280634b4074e19/third_party/WebKit/Source/modules/webaudio/DeferredTaskHandler.h
[modify] https://crrev.com/e3860bd297e465778059f3d845280634b4074e19/third_party/WebKit/Source/modules/webaudio/OfflineAudioDestinationNode.cpp
[modify] https://crrev.com/e3860bd297e465778059f3d845280634b4074e19/third_party/WebKit/Source/modules/webaudio/PannerNode.cpp
[modify] https://crrev.com/e3860bd297e465778059f3d845280634b4074e19/third_party/WebKit/Source/modules/webaudio/ScriptProcessorNode.cpp
[modify] https://crrev.com/e3860bd297e465778059f3d845280634b4074e19/third_party/WebKit/Source/modules/webaudio/StereoPannerNode.cpp

Comment 4 by rtoy@chromium.org, Aug 22 2016

Status: Fixed (was: Started)
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 25 2016 

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

commit 50f926133cfbb5e2576619d1035760b027f27672
Author: rtoy <rtoy@chromium.org>
Date: Thu Aug 25 02:16:14 2016

Revert CL 2242573002

https://codereview.chromium.org/2242573002

We'll do the channel interpretation and mode in a different way.

TBR=hongchan@chromium.org
BUG=640575, 636576 
TEST=Test from 640575 passes

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

[modify] https://crrev.com/50f926133cfbb5e2576619d1035760b027f27672/third_party/WebKit/Source/modules/webaudio/AudioNode.cpp
[modify] https://crrev.com/50f926133cfbb5e2576619d1035760b027f27672/third_party/WebKit/Source/modules/webaudio/AudioNode.h
[modify] https://crrev.com/50f926133cfbb5e2576619d1035760b027f27672/third_party/WebKit/Source/modules/webaudio/AudioNodeInput.cpp
[modify] https://crrev.com/50f926133cfbb5e2576619d1035760b027f27672/third_party/WebKit/Source/modules/webaudio/BiquadProcessor.cpp
[modify] https://crrev.com/50f926133cfbb5e2576619d1035760b027f27672/third_party/WebKit/Source/modules/webaudio/ChannelMergerNode.cpp
[modify] https://crrev.com/50f926133cfbb5e2576619d1035760b027f27672/third_party/WebKit/Source/modules/webaudio/ConvolverNode.cpp
[modify] https://crrev.com/50f926133cfbb5e2576619d1035760b027f27672/third_party/WebKit/Source/modules/webaudio/DefaultAudioDestinationNode.cpp
[modify] https://crrev.com/50f926133cfbb5e2576619d1035760b027f27672/third_party/WebKit/Source/modules/webaudio/DeferredTaskHandler.cpp
[modify] https://crrev.com/50f926133cfbb5e2576619d1035760b027f27672/third_party/WebKit/Source/modules/webaudio/DeferredTaskHandler.h
[modify] https://crrev.com/50f926133cfbb5e2576619d1035760b027f27672/third_party/WebKit/Source/modules/webaudio/OfflineAudioDestinationNode.cpp
[modify] https://crrev.com/50f926133cfbb5e2576619d1035760b027f27672/third_party/WebKit/Source/modules/webaudio/PannerNode.cpp
[modify] https://crrev.com/50f926133cfbb5e2576619d1035760b027f27672/third_party/WebKit/Source/modules/webaudio/ScriptProcessorNode.cpp
[modify] https://crrev.com/50f926133cfbb5e2576619d1035760b027f27672/third_party/WebKit/Source/modules/webaudio/StereoPannerNode.cpp

Comment 6 by rtoy@chromium.org, Aug 25 2016

Status: Started (was: Fixed)
Reopening because the fix was reverted.
Project Member

Comment 7 by bugdroid1@chromium.org, Sep 1 2016 

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

commit c307211c34797418cd5ddddd31efb83f5a509d5e
Author: rtoy <rtoy@chromium.org>
Date: Thu Sep 01 22:05:52 2016

Return the correct channelCountMode and channelInterpretation

Instead of returning the current mode or interpretation, return the
value used in the setter.  Eventually, the real mode will get set to
the desired value.

The members m_channelCountMode and m_channelInterpretation are also
made private to force use of the setters and getters.  A setter is
added too so that the internal state is maintained.

BUG= 636576 
TEST=channel-mode-interp-basic.html

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

[add] https://crrev.com/c307211c34797418cd5ddddd31efb83f5a509d5e/third_party/WebKit/LayoutTests/webaudio/channel-mode-interp-basic.html
[modify] https://crrev.com/c307211c34797418cd5ddddd31efb83f5a509d5e/third_party/WebKit/Source/modules/webaudio/AudioNode.cpp
[modify] https://crrev.com/c307211c34797418cd5ddddd31efb83f5a509d5e/third_party/WebKit/Source/modules/webaudio/AudioNode.h
[modify] https://crrev.com/c307211c34797418cd5ddddd31efb83f5a509d5e/third_party/WebKit/Source/modules/webaudio/ChannelMergerNode.cpp
[modify] https://crrev.com/c307211c34797418cd5ddddd31efb83f5a509d5e/third_party/WebKit/Source/modules/webaudio/ConvolverNode.cpp
[modify] https://crrev.com/c307211c34797418cd5ddddd31efb83f5a509d5e/third_party/WebKit/Source/modules/webaudio/DefaultAudioDestinationNode.cpp
[modify] https://crrev.com/c307211c34797418cd5ddddd31efb83f5a509d5e/third_party/WebKit/Source/modules/webaudio/OfflineAudioDestinationNode.cpp
[modify] https://crrev.com/c307211c34797418cd5ddddd31efb83f5a509d5e/third_party/WebKit/Source/modules/webaudio/PannerNode.cpp
[modify] https://crrev.com/c307211c34797418cd5ddddd31efb83f5a509d5e/third_party/WebKit/Source/modules/webaudio/ScriptProcessorNode.cpp
[modify] https://crrev.com/c307211c34797418cd5ddddd31efb83f5a509d5e/third_party/WebKit/Source/modules/webaudio/StereoPannerNode.cpp

Comment 8 by rtoy@chromium.org, Sep 9 2016

Status: Fixed (was: Started)
►

Sign in to add a comment