New issue
Advanced search Search tips

Issue 854229 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 854228



Sign in to add a comment

[WebAudio refactoring] AudioDestinationNode

Project Member Reported by hongchan@chromium.org, Jun 19 2018

Issue description

Current design:
AudioDestinationNode (realtime-specific render loop)
 - DefaultAudioDestinationNode (no render loop)
 - OffilneAudioDestinationNode (offline-specific render loop)

Action item:
 - Rename "Default" to "Online" or "Realtime".
 - Move the realtime render loop to the realtime destination node class.
 

Comment 1 by rtoy@chromium.org, Jun 19 2018

Can you do this yesterday, please?  

I always get confused because the names never match my expectations after all these years.
Project Member

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

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

commit 835940caaf12844cfb2a3ca3cdfef1d7afd11dec
Author: Hongchan Choi <hongchan@chromium.org>
Date: Thu Jun 21 18:00:39 2018

Removing unused members in DefaultAudioDestinationNode

This CL removes unused 'input'-related members from
DefaultAudioDestinationNode.

Bug: 854229
Change-Id: I8495ac736d4cb1f33c61fec243510c42a88e3fc2
Reviewed-on: https://chromium-review.googlesource.com/1110035
Commit-Queue: Hongchan Choi <hongchan@chromium.org>
Reviewed-by: Raymond Toy <rtoy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569319}
[modify] https://crrev.com/835940caaf12844cfb2a3ca3cdfef1d7afd11dec/third_party/blink/renderer/modules/webaudio/default_audio_destination_node.cc
[modify] https://crrev.com/835940caaf12844cfb2a3ca3cdfef1d7afd11dec/third_party/blink/renderer/modules/webaudio/default_audio_destination_node.h

Project Member

Comment 4 by bugdroid1@chromium.org, Jun 21 2018

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

commit 73c157741c24fd9071bebf64f54e7221312aecd4
Author: Hongchan Choi <hongchan@chromium.org>
Date: Thu Jun 21 23:54:20 2018

Tidy up redundant call chain of FramesPerBuffer

FramesPerBuffer() were scattered around many classes for no specific
reason. This CL cleans them up and contains it to
DefaultAudioDestinationNode.

TBR=andrewmacp@soundtrap.com

Bug: 854229
Change-Id: I9caa0e53aadf23fac9fda19f3b0b85f5b20c4483
Reviewed-on: https://chromium-review.googlesource.com/1110482
Commit-Queue: Hongchan Choi <hongchan@chromium.org>
Reviewed-by: Raymond Toy <rtoy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569459}
[modify] https://crrev.com/73c157741c24fd9071bebf64f54e7221312aecd4/third_party/blink/renderer/modules/webaudio/audio_context.cc
[modify] https://crrev.com/73c157741c24fd9071bebf64f54e7221312aecd4/third_party/blink/renderer/modules/webaudio/audio_destination_node.h
[modify] https://crrev.com/73c157741c24fd9071bebf64f54e7221312aecd4/third_party/blink/renderer/modules/webaudio/base_audio_context.h
[modify] https://crrev.com/73c157741c24fd9071bebf64f54e7221312aecd4/third_party/blink/renderer/modules/webaudio/default_audio_destination_node.cc
[modify] https://crrev.com/73c157741c24fd9071bebf64f54e7221312aecd4/third_party/blink/renderer/modules/webaudio/default_audio_destination_node.h
[modify] https://crrev.com/73c157741c24fd9071bebf64f54e7221312aecd4/third_party/blink/renderer/modules/webaudio/offline_audio_destination_node.h

Project Member

Comment 5 by bugdroid1@chromium.org, Jun 25 2018

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

commit f022b8fb8db35e20d59702a3ba5ed3a9dc13c516
Author: Hongchan Choi <hongchan@chromium.org>
Date: Mon Jun 25 17:22:40 2018

Tidy up redundant call chain of CallbackBufferSize

CallbackBufferSize() were scattered around many classes for no specific
reason. This CL cleans them up and contains it to
DefaultAudioDestinationNode.

Bug: 854229
Change-Id: I9116cf8ecd9398108fcdcac89cbbfb8c42de957e
Reviewed-on: https://chromium-review.googlesource.com/1112072
Commit-Queue: Hongchan Choi <hongchan@chromium.org>
Reviewed-by: Raymond Toy <rtoy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570081}
[modify] https://crrev.com/f022b8fb8db35e20d59702a3ba5ed3a9dc13c516/third_party/blink/renderer/modules/webaudio/audio_destination_node.h
[modify] https://crrev.com/f022b8fb8db35e20d59702a3ba5ed3a9dc13c516/third_party/blink/renderer/modules/webaudio/audio_node.h
[modify] https://crrev.com/f022b8fb8db35e20d59702a3ba5ed3a9dc13c516/third_party/blink/renderer/modules/webaudio/base_audio_context.h
[modify] https://crrev.com/f022b8fb8db35e20d59702a3ba5ed3a9dc13c516/third_party/blink/renderer/modules/webaudio/default_audio_destination_node.cc
[modify] https://crrev.com/f022b8fb8db35e20d59702a3ba5ed3a9dc13c516/third_party/blink/renderer/modules/webaudio/default_audio_destination_node.h
[modify] https://crrev.com/f022b8fb8db35e20d59702a3ba5ed3a9dc13c516/third_party/blink/renderer/modules/webaudio/offline_audio_destination_node.cc
[modify] https://crrev.com/f022b8fb8db35e20d59702a3ba5ed3a9dc13c516/third_party/blink/renderer/modules/webaudio/offline_audio_destination_node.h
[modify] https://crrev.com/f022b8fb8db35e20d59702a3ba5ed3a9dc13c516/third_party/blink/renderer/modules/webaudio/script_processor_node.cc
[modify] https://crrev.com/f022b8fb8db35e20d59702a3ba5ed3a9dc13c516/third_party/blink/renderer/modules/webaudio/script_processor_node.h

Project Member

Comment 6 by bugdroid1@chromium.org, Jun 25 2018

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

commit 856dfa819d6dd5ccc0191ee4753a5a3a178aa07c
Author: Hongchan Choi <hongchan@chromium.org>
Date: Mon Jun 25 20:48:19 2018

Move AudioIOCallback class to DefaultAudioDestinationNode

AudioIOCallback is specifically designed for real-time audio IO and
is not appropriate for AudioDestinationNode. This CL moves the class
to DefaultAudioDestinationNode.

Bug: 854229
Change-Id: Icb0df6414b944ea1dde3e957d2f36c314b079e0e
Reviewed-on: https://chromium-review.googlesource.com/1113955
Reviewed-by: Raymond Toy <rtoy@chromium.org>
Commit-Queue: Hongchan Choi <hongchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570167}
[modify] https://crrev.com/856dfa819d6dd5ccc0191ee4753a5a3a178aa07c/third_party/blink/renderer/modules/webaudio/audio_destination_node.cc
[modify] https://crrev.com/856dfa819d6dd5ccc0191ee4753a5a3a178aa07c/third_party/blink/renderer/modules/webaudio/audio_destination_node.h
[modify] https://crrev.com/856dfa819d6dd5ccc0191ee4753a5a3a178aa07c/third_party/blink/renderer/modules/webaudio/base_audio_context.h
[modify] https://crrev.com/856dfa819d6dd5ccc0191ee4753a5a3a178aa07c/third_party/blink/renderer/modules/webaudio/convolver_node_test.cc
[modify] https://crrev.com/856dfa819d6dd5ccc0191ee4753a5a3a178aa07c/third_party/blink/renderer/modules/webaudio/default_audio_destination_node.cc
[modify] https://crrev.com/856dfa819d6dd5ccc0191ee4753a5a3a178aa07c/third_party/blink/renderer/modules/webaudio/default_audio_destination_node.h

Project Member

Comment 7 by bugdroid1@chromium.org, Jul 11

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

commit a4247ad3b2fda13e8f4d57b68b639bea7d78df02
Author: Hongchan Choi <hongchan@chromium.org>
Date: Wed Jul 11 23:30:27 2018

Reorg DefaultAudioDestinationNode

No renaming or functional change. Just moving and grouping things
around. Plus, some clarification/simplification of comments.

Bug: 854229
Change-Id: I1a4fd3cdaf7fd41fd6c30fc199f281b46b061dc2
Reviewed-on: https://chromium-review.googlesource.com/1133715
Commit-Queue: Hongchan Choi <hongchan@chromium.org>
Reviewed-by: Raymond Toy <rtoy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574403}
[modify] https://crrev.com/a4247ad3b2fda13e8f4d57b68b639bea7d78df02/third_party/blink/renderer/modules/webaudio/default_audio_destination_node.cc
[modify] https://crrev.com/a4247ad3b2fda13e8f4d57b68b639bea7d78df02/third_party/blink/renderer/modules/webaudio/default_audio_destination_node.h

Project Member

Comment 8 by bugdroid1@chromium.org, Sep 17

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

commit 55de0d943e5c1d69acd07f8e58054ef60a44954e
Author: Hongchan Choi <hongchan@chromium.org>
Date: Mon Sep 17 19:43:05 2018

Refactoring DefaultAudioDestinationNode

- Clarify comments.
- Change the order of members for the better grouping.
- Add extra braces to |if| statement to clarify the scope.
- Add/Move DCHECK()s where it's appropriate.

Bug: 854229
Test: All the layout/WPT tests pass.
Change-Id: If3c95d35cfcfa3d8711ce238e4eddb7d0c5d9891
Reviewed-on: https://chromium-review.googlesource.com/1135527
Commit-Queue: Hongchan Choi <hongchan@chromium.org>
Reviewed-by: Raymond Toy <rtoy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591786}
[modify] https://crrev.com/55de0d943e5c1d69acd07f8e58054ef60a44954e/third_party/blink/renderer/modules/webaudio/default_audio_destination_node.cc
[modify] https://crrev.com/55de0d943e5c1d69acd07f8e58054ef60a44954e/third_party/blink/renderer/modules/webaudio/default_audio_destination_node.h

Sign in to add a comment