Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(275)

Issue 1966043006: Revert of MediaStream audio: Refactor 3 separate "glue" implementations into one. (Closed)

Created:
4 years, 7 months ago by tommycli
Modified:
4 years, 7 months ago
CC:
chromium-reviews, mlamouri+watch-content_chromium.org, imcheng+watch_chromium.org, posciak+watch_chromium.org, avayvod+watch_chromium.org, jam, mcasas+watch_chromium.org, feature-media-reviews_chromium.org, darin-cc_chromium.org, jasonroberts+watch_google.com, mkwst+moarreviews-renderer_chromium.org, xjz+watch_chromium.org, isheriff+watch_chromium.org, miu+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of MediaStream audio: Refactor 3 separate "glue" implementations into one. (patchset #10 id:200001 of https://codereview.chromium.org/1834323002/ ) Reason for revert: Unfortunate to revert such a mega patch, but I think this broke: https://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20TSan%20Tests?numbuilds=200 First breaking build: https://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20TSan%20Tests/builds/20227 Original issue's description: > MediaStream audio: Refactor 3 separate "glue" implementations into one. > > This is a refactoring of the MediaStreamAudioSource/Track object graph > such that life-cycle and audio flow control are unified into a single > architecture. Previously, each of three implementations solved this > same problem in three different ways; and this has made it difficult to > maintain the code in src/content/renderer/media across all product > features, WebRTC or otherwise. > > Diagram of post-refactoring class relationships: https://docs.google.com/drawings/d/1yTsXvRMIyMlXjIEeQOVqXPxMPWgekB5ePWUVSI5-zwo/edit?usp=sharing > > The new architeture is as follows: > > 1. MediaStreamAudioSource becomes a base class implementation for > creating MediaStreamAudioTracks and delivering audio to them from the > source (i.e., an AudioInputDevice, a PeerConnection remote source, or a > WebAudio source). All of its methods are now assumed to be run on the > main thread, while audio flow may optionally occur on a separate > thread. > > 2. MediaStreamAudioTrack becomes a base class implementation for > connecting/disconnecting MediaStreamAudioSinks, and delivering audio to > them from the source. > > 3. Both MediaStreamAudioSource and MediaStreamAudioTrack are owned by > their blink counterparts (WebMediaStreamSource and WebMediaStreamTrack), > and their destruction may safely occur any time the blink implementation > requires it. > > Following the new architeture, the refactoring is: > > 1. WebRtcAudioCapturer becomes a sub-class of MediaStreamAudioSource and > is renamed to ProcessedLocalAudioSource. This new class owns and > manages the WebRTC-specific audio processing on the source data. Also: > a) Significant implementation from PeerConnectionDependencyFactory was > consolidated/moved to this class. b) The "EnablePeerConnectionMode" > functionality, which re-started the audio input with a different buffer > size, has been removed to simplify the implementation. Now, buffer size > is determined BEFORE the first time the audio input device is started. > c) Currently, all local audio sources (i.e., via AudioInputDevice) are > routed through this pipeline, but a soon-upcoming change will split the > WebRTC-specific cases from the ones that should not go through audio > processing. > > 2. MediaStreamRemoteAudioTrack becomes a sub-class of > MediaStreamAudioSource and is renamed to > PeerConnectionRemoteAudioSource. This new class owns and manages the > flow of audio data from a PeerConnection into the MediaStream > framework. > > 3. WebAudioCapturer becomes a sub-class of MediaStreamAudioSource and is > renamed to WebAudioMediaStreamSource. As a > blink::WebAudioDestinationConsumer, it manages the flow of audio from a > WebAudio destination node into the MediaStream framework. > > BUG=577881, 577874 > > Committed: https://crrev.com/e640bad76cad1155335d4068ecd46d250327bb9e > Cr-Commit-Position: refs/heads/master@{#392845} TBR=olka@chromium.org,tommi@chromium.org,perkj@chromium.org,nick@chromium.org,miu@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=577881, 577874 Committed: https://crrev.com/57740de7e4fa30c039f39f3a76d373b7833f6493 Cr-Commit-Position: refs/heads/master@{#392960}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+3847 lines, -3404 lines) Patch
M content/content_renderer.gypi View 7 chunks +19 lines, -15 lines 0 comments Download
M content/content_tests.gypi View 2 chunks +3 lines, -4 lines 0 comments Download
M content/public/renderer/media_stream_utils.cc View 2 chunks +25 lines, -9 lines 0 comments Download
M content/renderer/media/audio_track_recorder_unittest.cc View 2 chunks +10 lines, -8 lines 0 comments Download
D content/renderer/media/external_media_stream_audio_source.h View 1 chunk +0 lines, -59 lines 0 comments Download
D content/renderer/media/external_media_stream_audio_source.cc View 1 chunk +0 lines, -81 lines 0 comments Download
D content/renderer/media/media_stream_audio_deliverer.h View 1 chunk +0 lines, -156 lines 0 comments Download
M content/renderer/media/media_stream_audio_processor.cc View 1 chunk +0 lines, -3 lines 0 comments Download
M content/renderer/media/media_stream_audio_processor_options.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M content/renderer/media/media_stream_audio_processor_unittest.cc View 1 chunk +0 lines, -1 line 0 comments Download
A content/renderer/media/media_stream_audio_sink_owner.h View 1 chunk +47 lines, -0 lines 0 comments Download
A content/renderer/media/media_stream_audio_sink_owner.cc View 1 chunk +49 lines, -0 lines 0 comments Download
M content/renderer/media/media_stream_audio_source.h View 1 chunk +56 lines, -107 lines 0 comments Download
M content/renderer/media/media_stream_audio_source.cc View 2 chunks +52 lines, -105 lines 0 comments Download
M content/renderer/media/media_stream_audio_track.h View 2 chunks +30 lines, -69 lines 0 comments Download
M content/renderer/media/media_stream_audio_track.cc View 2 chunks +16 lines, -90 lines 0 comments Download
A content/renderer/media/media_stream_audio_track_sink.h View 1 chunk +61 lines, -0 lines 0 comments Download
D content/renderer/media/media_stream_audio_unittest.cc View 1 chunk +0 lines, -450 lines 0 comments Download
M content/renderer/media/media_stream_center.h View 2 chunks +4 lines, -2 lines 0 comments Download
M content/renderer/media/media_stream_center.cc View 4 chunks +29 lines, -37 lines 0 comments Download
M content/renderer/media/media_stream_renderer_factory_impl.cc View 2 chunks +5 lines, -2 lines 0 comments Download
M content/renderer/media/media_stream_source.h View 2 chunks +20 lines, -24 lines 0 comments Download
M content/renderer/media/media_stream_source.cc View 1 chunk +3 lines, -30 lines 0 comments Download
D content/renderer/media/mock_audio_device_factory.h View 1 chunk +0 lines, -82 lines 0 comments Download
D content/renderer/media/mock_audio_device_factory.cc View 1 chunk +0 lines, -28 lines 0 comments Download
M content/renderer/media/mock_media_stream_dispatcher.cc View 2 chunks +0 lines, -4 lines 0 comments Download
M content/renderer/media/mock_media_stream_registry.cc View 3 chunks +13 lines, -33 lines 0 comments Download
M content/renderer/media/remote_media_stream_impl.cc View 2 chunks +6 lines, -5 lines 0 comments Download
M content/renderer/media/rtc_peer_connection_handler.cc View 3 chunks +9 lines, -12 lines 0 comments Download
M content/renderer/media/rtc_peer_connection_handler_unittest.cc View 9 chunks +14 lines, -54 lines 0 comments Download
M content/renderer/media/speech_recognition_audio_sink_unittest.cc View 7 chunks +33 lines, -49 lines 0 comments Download
A content/renderer/media/tagged_list.h View 1 chunk +95 lines, -0 lines 0 comments Download
M content/renderer/media/user_media_client_impl.h View 3 chunks +4 lines, -8 lines 0 comments Download
M content/renderer/media/user_media_client_impl.cc View 5 chunks +17 lines, -25 lines 0 comments Download
M content/renderer/media/user_media_client_impl_unittest.cc View 5 chunks +6 lines, -29 lines 0 comments Download
A content/renderer/media/webaudio_capturer_source.h View 1 chunk +101 lines, -0 lines 0 comments Download
A content/renderer/media/webaudio_capturer_source.cc View 1 chunk +136 lines, -0 lines 0 comments Download
D content/renderer/media/webaudio_media_stream_source.h View 1 chunk +0 lines, -82 lines 0 comments Download
D content/renderer/media/webaudio_media_stream_source.cc View 1 chunk +0 lines, -111 lines 0 comments Download
A content/renderer/media/webrtc/media_stream_remote_audio_track.h View 1 chunk +89 lines, -0 lines 0 comments Download
A content/renderer/media/webrtc/media_stream_remote_audio_track.cc View 1 chunk +237 lines, -0 lines 0 comments Download
M content/renderer/media/webrtc/mock_peer_connection_dependency_factory.h View 2 chunks +12 lines, -5 lines 0 comments Download
M content/renderer/media/webrtc/mock_peer_connection_dependency_factory.cc View 4 chunks +21 lines, -6 lines 0 comments Download
M content/renderer/media/webrtc/peer_connection_dependency_factory.h View 5 chunks +41 lines, -8 lines 0 comments Download
M content/renderer/media/webrtc/peer_connection_dependency_factory.cc View 5 chunks +189 lines, -0 lines 0 comments Download
D content/renderer/media/webrtc/peer_connection_remote_audio_source.h View 1 chunk +0 lines, -102 lines 0 comments Download
D content/renderer/media/webrtc/peer_connection_remote_audio_source.cc View 1 chunk +0 lines, -153 lines 0 comments Download
D content/renderer/media/webrtc/processed_local_audio_source.h View 1 chunk +0 lines, -148 lines 0 comments Download
D content/renderer/media/webrtc/processed_local_audio_source.cc View 1 chunk +0 lines, -389 lines 0 comments Download
D content/renderer/media/webrtc/processed_local_audio_source_unittest.cc View 1 chunk +0 lines, -227 lines 0 comments Download
D content/renderer/media/webrtc/webrtc_audio_sink.h View 1 chunk +0 lines, -183 lines 0 comments Download
D content/renderer/media/webrtc/webrtc_audio_sink.cc View 1 chunk +0 lines, -194 lines 0 comments Download
A content/renderer/media/webrtc/webrtc_audio_sink_adapter.h View 1 chunk +51 lines, -0 lines 0 comments Download
A content/renderer/media/webrtc/webrtc_audio_sink_adapter.cc View 1 chunk +51 lines, -0 lines 0 comments Download
A content/renderer/media/webrtc/webrtc_local_audio_track_adapter.h View 1 chunk +107 lines, -0 lines 0 comments Download
A content/renderer/media/webrtc/webrtc_local_audio_track_adapter.cc View 1 chunk +161 lines, -0 lines 0 comments Download
A content/renderer/media/webrtc/webrtc_local_audio_track_adapter_unittest.cc View 1 chunk +102 lines, -0 lines 0 comments Download
M content/renderer/media/webrtc/webrtc_media_stream_adapter.h View 5 chunks +6 lines, -10 lines 0 comments Download
M content/renderer/media/webrtc/webrtc_media_stream_adapter.cc View 3 chunks +31 lines, -70 lines 0 comments Download
M content/renderer/media/webrtc/webrtc_media_stream_adapter_unittest.cc View 3 chunks +12 lines, -28 lines 0 comments Download
A content/renderer/media/webrtc_audio_capturer.h View 1 chunk +207 lines, -0 lines 0 comments Download
A content/renderer/media/webrtc_audio_capturer.cc View 1 chunk +566 lines, -0 lines 0 comments Download
A content/renderer/media/webrtc_audio_capturer_unittest.cc View 1 chunk +152 lines, -0 lines 0 comments Download
M content/renderer/media/webrtc_audio_device_impl.h View 4 chunks +7 lines, -8 lines 0 comments Download
M content/renderer/media/webrtc_audio_device_impl.cc View 4 chunks +7 lines, -7 lines 0 comments Download
M content/renderer/media/webrtc_audio_renderer.h View 1 chunk +0 lines, -1 line 0 comments Download
M content/renderer/media/webrtc_audio_renderer.cc View 3 chunks +7 lines, -9 lines 0 comments Download
M content/renderer/media/webrtc_audio_renderer_unittest.cc View 2 chunks +1 line, -4 lines 0 comments Download
M content/renderer/media/webrtc_local_audio_source_provider.h View 1 chunk +4 lines, -8 lines 0 comments Download
M content/renderer/media/webrtc_local_audio_source_provider_unittest.cc View 5 chunks +13 lines, -8 lines 0 comments Download
A content/renderer/media/webrtc_local_audio_track.h View 1 chunk +106 lines, -0 lines 0 comments Download
A content/renderer/media/webrtc_local_audio_track.cc View 1 chunk +169 lines, -0 lines 0 comments Download
A content/renderer/media/webrtc_local_audio_track_unittest.cc View 1 chunk +579 lines, -0 lines 0 comments Download
M content/renderer/pepper/pepper_media_stream_video_track_host.h View 5 chunks +18 lines, -9 lines 0 comments Download
M content/renderer/pepper/pepper_media_stream_video_track_host.cc View 6 chunks +35 lines, -50 lines 0 comments Download

Messages

Total messages: 6 (2 generated)
tommycli
Created Revert of MediaStream audio: Refactor 3 separate "glue" implementations into one.
4 years, 7 months ago (2016-05-11 17:00:05 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1966043006/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1966043006/1
4 years, 7 months ago (2016-05-11 17:00:31 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 7 months ago (2016-05-11 17:03:27 UTC) #4
commit-bot: I haz the power
4 years, 7 months ago (2016-05-11 17:05:22 UTC) #6
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/57740de7e4fa30c039f39f3a76d373b7833f6493
Cr-Commit-Position: refs/heads/master@{#392960}

Powered by Google App Engine
This is Rietveld 408576698