New issue
Advanced search Search tips

Issue 845913 link

Starred by 5 users
Status: Fixed
Owner:
rtoy@chromium.org
Closed: Jan 14
Components:
EstimatedDays: ----
NextAction: 2018-10-01
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 842225



Sign in to add a comment

Move Media* nodes to AudioContext from BaseAudioContext

Project Member Reported by rtoy@chromium.org, May 23 2018 
The factory methods for creating the Media* nodes are currently defined on the BaseAudioContext.  This is incorrect.  They should only be defined on an AudioContext.

This is a legacy issue from the time before there was a BaseAudioContext.  Also, we didn't have any way of running tests on trybots because they may not have audio HW, so we used an OfflineAudioContext.

This latter issue is fixed (https://chromium-review.googlesource.com/c/chromium/src/+/1067100) so we can use an AudioContext now.

Comment 1 by rtoy@chromium.org, May 23 2018

Blocking: 842225
Project Member

Comment 2 by bugdroid1@chromium.org, May 24 2018 

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

commit 895fec41cf70f1d9de706b7d4edf26b2d6b4029e
Author: Raymond Toy <rtoy@chromium.org>
Date: Thu May 24 18:09:15 2018

Use AudioContext in MediaElementAudioSource tests

The trybots can now run AudioContext with a fake audio device if no
audio HW is available.  Change the MediaElementAudioSource tests to
use an AudioContext instead of an OfflineAudioContext.

This is the first step in moving the factory methods from
BaseAudioContext to AudioContext.

Bug:  845913 
Change-Id: Id4fe3d7d1b943db25cf023940c541dfab2a01aed
Reviewed-on: https://chromium-review.googlesource.com/1070217
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561551}
[modify] https://crrev.com/895fec41cf70f1d9de706b7d4edf26b2d6b4029e/third_party/WebKit/LayoutTests/http/tests/security/media-element-audio-source-node-cross-origin-allowed.html
[modify] https://crrev.com/895fec41cf70f1d9de706b7d4edf26b2d6b4029e/third_party/WebKit/LayoutTests/http/tests/security/media-element-audio-source-node-cross-origin-with-credentials.html
[modify] https://crrev.com/895fec41cf70f1d9de706b7d4edf26b2d6b4029e/third_party/WebKit/LayoutTests/http/tests/security/media-element-audio-source-node-cross-origin.html
[modify] https://crrev.com/895fec41cf70f1d9de706b7d4edf26b2d6b4029e/third_party/WebKit/LayoutTests/http/tests/security/media-element-audio-source-node-same-origin.html
[modify] https://crrev.com/895fec41cf70f1d9de706b7d4edf26b2d6b4029e/third_party/WebKit/LayoutTests/http/tests/security/resources/webaudio/media-element-audio-source-node-test.js
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 18 2018 

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

commit 80a33cc0764de8a49b17047634398462fb9c3e40
Author: Raymond Toy <rtoy@chromium.org>
Date: Mon Jun 18 20:06:24 2018

Deprecate WebAudio Media nodes on an OfflineAudioContext

Print a deprecation message when WebAudio Media nodes
(MediaElementAudioSourceNode, MediaStreamAudioDestinationNode,
MediaStreamAudioSourceNode) are created on an OfflineAudioContext.
Doing so is not allowed in the WebAudio spec.

Intent: https://groups.google.com/a/chromium.org/d/msg/blink-dev/H29uXnsIN54/BKXDqxoJAQAJ
Chrome feature entry: https://www.chromestatus.com/feature/5258622686724096

Bug:  845913 
Change-Id: I5247d67456afa11582099f4d52d65a76f788639d
Reviewed-on: https://chromium-review.googlesource.com/1091656
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568124}
[modify] https://crrev.com/80a33cc0764de8a49b17047634398462fb9c3e40/third_party/WebKit/LayoutTests/webaudio/AudioParam/audioparam-nominal-range-expected.txt
[modify] https://crrev.com/80a33cc0764de8a49b17047634398462fb9c3e40/third_party/WebKit/LayoutTests/webaudio/AudioParam/audioparam-nominal-range.html
[modify] https://crrev.com/80a33cc0764de8a49b17047634398462fb9c3e40/third_party/WebKit/LayoutTests/webaudio/internals/mediaelementaudiosourcenode-gc-expected.txt
[modify] https://crrev.com/80a33cc0764de8a49b17047634398462fb9c3e40/third_party/WebKit/LayoutTests/webaudio/internals/mediaelementaudiosourcenode-gc.html
[modify] https://crrev.com/80a33cc0764de8a49b17047634398462fb9c3e40/third_party/blink/public/platform/web_feature.mojom
[modify] https://crrev.com/80a33cc0764de8a49b17047634398462fb9c3e40/third_party/blink/renderer/core/frame/deprecation.cc
[modify] https://crrev.com/80a33cc0764de8a49b17047634398462fb9c3e40/third_party/blink/renderer/modules/webaudio/media_element_audio_source_node.cc
[modify] https://crrev.com/80a33cc0764de8a49b17047634398462fb9c3e40/third_party/blink/renderer/modules/webaudio/media_stream_audio_destination_node.cc
[modify] https://crrev.com/80a33cc0764de8a49b17047634398462fb9c3e40/third_party/blink/renderer/modules/webaudio/media_stream_audio_source_node.cc
[modify] https://crrev.com/80a33cc0764de8a49b17047634398462fb9c3e40/tools/metrics/histograms/enums.xml

Comment 4 by rtoy@chromium.org, Jun 22 2018

NextAction: 2018-08-13

Comment 5 by monor...@bugs.chromium.org, Aug 13 

The NextAction date has arrived: 2018-08-13

Comment 6 by rtoy@chromium.org, Aug 28

NextAction: 2018-10-01
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 31 

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

commit 5ca87ba8a23c482b3dfd55ea17ccab73c8a02a70
Author: Raymond Toy <rtoy@chromium.org>
Date: Fri Aug 31 00:56:51 2018

Change milestone to M71 instead M70 for media element of OAC

Just update the messages to say M71 for when the media element nodes
will be removed from an OfflineAudioContext, as required by the
WebAudio spec.  We're still gathering some UMA stats on this.

Intent: https://groups.google.com/a/chromium.org/d/msg/blink-dev/H29uXnsIN54/BKXDqxoJAQAJ
Chrome feature entry: https://www.chromestatus.com/feature/5258622686724096

Bug:  845913 
Change-Id: Id6b5803533a96bca509d4b5b2b8c4dd2d8ecd7a3
Reviewed-on: https://chromium-review.googlesource.com/1194737
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587883}
[modify] https://crrev.com/5ca87ba8a23c482b3dfd55ea17ccab73c8a02a70/third_party/blink/renderer/core/frame/deprecation.cc

Comment 8 by rtoy@chromium.org, Sep 4

Labels: Merge-Request-70
Requesting merge of CL in c#7 that just updates the deprecation message from M70 to M71.
Project Member

Comment 9 by sheriffbot@chromium.org, Sep 5

Labels: -Merge-Request-70 Hotlist-Merge-Approved Merge-Approved-70
Your change meets the bar and is auto-approved for M70. Please go ahead and merge the CL to branch 3538 manually. Please contact milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 10 by rtoy@chromium.org, Sep 5 

The merge is in https://chromium-review.googlesource.com/c/chromium/src/+/1208150. Not sure why this bug wasn't updated with that information.
Project Member

Comment 11 by bugdroid1@chromium.org, Sep 5

Labels: -merge-approved-70 merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c997eeefffdfff099c057c054098f01350da7b38

commit c997eeefffdfff099c057c054098f01350da7b38
Author: Raymond Toy <rtoy@chromium.org>
Date: Wed Sep 05 18:31:30 2018

Change milestone to M71 instead M70 for media element of OAC

Just update the messages to say M71 for when the media element nodes
will be removed from an OfflineAudioContext, as required by the
WebAudio spec.  We're still gathering some UMA stats on this.

Intent: https://groups.google.com/a/chromium.org/d/msg/blink-dev/H29uXnsIN54/BKXDqxoJAQAJ
Chrome feature entry: https://www.chromestatus.com/feature/5258622686724096

Bug:  845913 
Change-Id: Id6b5803533a96bca509d4b5b2b8c4dd2d8ecd7a3
Reviewed-on: https://chromium-review.googlesource.com/1194737
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#587883}(cherry picked from commit 5ca87ba8a23c482b3dfd55ea17ccab73c8a02a70)
Reviewed-on: https://chromium-review.googlesource.com/1208150
Reviewed-by: Raymond Toy <rtoy@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#60}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/c997eeefffdfff099c057c054098f01350da7b38/third_party/blink/renderer/core/frame/deprecation.cc

Comment 12 by monor...@bugs.chromium.org, Oct 1 

The NextAction date has arrived: 2018-10-01
Project Member

Comment 13 by bugdroid1@chromium.org, Oct 5 

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

commit 79cb0630cf6ad293e913124fe0f51872a587005b
Author: Raymond Toy <rtoy@chromium.org>
Date: Fri Oct 05 21:50:00 2018

Move Media nodes to AudioContext

The Media nodes should be on the AudioContext, not BaseAudioContext.
Make it so.  This also implies that the constructors for these nodes
should take an AudioContext, not BaseAudioContext.

Intent: https://groups.google.com/a/chromium.org/d/msg/blink-dev/H29uXnsIN54/BKXDqxoJAQAJ
Chrome feature entry: https://www.chromestatus.com/feature/5258622686724096

Bug:  845913 ,  842225 ,  697123 
Change-Id: Ic2a82a2343fa4a169a26733375c646fec61769ed
Reviewed-on: https://chromium-review.googlesource.com/c/1069692
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597322}
[modify] https://crrev.com/79cb0630cf6ad293e913124fe0f51872a587005b/content/test/data/media/mediarecorder_test.html
[modify] https://crrev.com/79cb0630cf6ad293e913124fe0f51872a587005b/third_party/WebKit/LayoutTests/external/wpt/webaudio/idlharness.https-expected.txt
[modify] https://crrev.com/79cb0630cf6ad293e913124fe0f51872a587005b/third_party/WebKit/LayoutTests/external/wpt/webaudio/idlharness.https.window-expected.txt
[modify] https://crrev.com/79cb0630cf6ad293e913124fe0f51872a587005b/third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/79cb0630cf6ad293e913124fe0f51872a587005b/third_party/WebKit/LayoutTests/webaudio/AudioParam/audioparam-nominal-range-expected.txt
[modify] https://crrev.com/79cb0630cf6ad293e913124fe0f51872a587005b/third_party/WebKit/LayoutTests/webaudio/AudioParam/audioparam-nominal-range.html
[modify] https://crrev.com/79cb0630cf6ad293e913124fe0f51872a587005b/third_party/WebKit/LayoutTests/webaudio/MediaStreamAudioDestination/ctor-mediastreamaudiodestination.html
[modify] https://crrev.com/79cb0630cf6ad293e913124fe0f51872a587005b/third_party/WebKit/LayoutTests/webaudio/MediaStreamAudioSource/ctor-mediastreamaudiosource.html
[modify] https://crrev.com/79cb0630cf6ad293e913124fe0f51872a587005b/third_party/WebKit/LayoutTests/webaudio/dom-exceptions-expected.txt
[modify] https://crrev.com/79cb0630cf6ad293e913124fe0f51872a587005b/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/79cb0630cf6ad293e913124fe0f51872a587005b/third_party/blink/renderer/modules/webaudio/audio_context.cc
[modify] https://crrev.com/79cb0630cf6ad293e913124fe0f51872a587005b/third_party/blink/renderer/modules/webaudio/audio_context.h
[modify] https://crrev.com/79cb0630cf6ad293e913124fe0f51872a587005b/third_party/blink/renderer/modules/webaudio/audio_context.idl
[modify] https://crrev.com/79cb0630cf6ad293e913124fe0f51872a587005b/third_party/blink/renderer/modules/webaudio/base_audio_context.cc
[modify] https://crrev.com/79cb0630cf6ad293e913124fe0f51872a587005b/third_party/blink/renderer/modules/webaudio/base_audio_context.h
[modify] https://crrev.com/79cb0630cf6ad293e913124fe0f51872a587005b/third_party/blink/renderer/modules/webaudio/base_audio_context.idl
[modify] https://crrev.com/79cb0630cf6ad293e913124fe0f51872a587005b/third_party/blink/renderer/modules/webaudio/media_element_audio_source_node.cc
[modify] https://crrev.com/79cb0630cf6ad293e913124fe0f51872a587005b/third_party/blink/renderer/modules/webaudio/media_element_audio_source_node.h
[modify] https://crrev.com/79cb0630cf6ad293e913124fe0f51872a587005b/third_party/blink/renderer/modules/webaudio/media_element_audio_source_node.idl
[modify] https://crrev.com/79cb0630cf6ad293e913124fe0f51872a587005b/third_party/blink/renderer/modules/webaudio/media_stream_audio_destination_node.cc
[modify] https://crrev.com/79cb0630cf6ad293e913124fe0f51872a587005b/third_party/blink/renderer/modules/webaudio/media_stream_audio_destination_node.h
[modify] https://crrev.com/79cb0630cf6ad293e913124fe0f51872a587005b/third_party/blink/renderer/modules/webaudio/media_stream_audio_destination_node.idl
[modify] https://crrev.com/79cb0630cf6ad293e913124fe0f51872a587005b/third_party/blink/renderer/modules/webaudio/media_stream_audio_source_node.cc
[modify] https://crrev.com/79cb0630cf6ad293e913124fe0f51872a587005b/third_party/blink/renderer/modules/webaudio/media_stream_audio_source_node.h
[modify] https://crrev.com/79cb0630cf6ad293e913124fe0f51872a587005b/third_party/blink/renderer/modules/webaudio/media_stream_audio_source_node.idl

Comment 14 by rtoy@chromium.org, Jan 14

Status: Fixed (was: Started)
All the relevant CLs have landed and a quick test shows that I can't create MediaElement node on an offline audio context.
►

Sign in to add a comment