New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 768610 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Move debug recording of microphone input to the AudioManager

Project Member Reported by maxmorin@chromium.org, Sep 25 2017

Issue description

This will make it work the same way as for output, and save us a bunch of plumbing through a bunch of layers. Will also simplify future refactoring significantly.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 4 2017

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

commit df7b7f8eb4de451cd063e019013a7e41c9723c9f
Author: maxmorin <maxmorin@chromium.org>
Date: Wed Oct 04 08:46:23 2017

Move microphone debug recording to AM.

Introduces the AudioInputStreamDataInterceptor, which transparently sends audio
data off for debug recording. AudioManagerBase wraps each created input stream
in an interceptor to enable debug recording. A side effect of this refactoring
is that WebContents input streams will not be recorded, but AFAIK noone ever
used recordings anyways. I also somehow had to fix a bunch of IWYU, not sure
how I got dragged into that :).

Since AudioInputRendererHost doesn't have to be accessed by the
RenderProcessHostImpl anymore, the member variable was removed; the lifetime of
the AudioInputRendererHost is now managed by the IPC::Channel as it is for other
message filters.

Bug:  768610 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I24a89435b283cd5df68373eb37d776a320d6ffd5
Reviewed-on: https://chromium-review.googlesource.com/680217
Commit-Queue: Max Morin <maxmorin@chromium.org>
Reviewed-by: Olga Sharonova <olka@chromium.org>
Reviewed-by: Tommi <tommi@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Henrik Grunell <grunell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506338}
[modify] https://crrev.com/df7b7f8eb4de451cd063e019013a7e41c9723c9f/chrome/browser/media/webrtc/audio_debug_recordings_handler.cc
[modify] https://crrev.com/df7b7f8eb4de451cd063e019013a7e41c9723c9f/content/browser/renderer_host/media/audio_input_renderer_host.cc
[modify] https://crrev.com/df7b7f8eb4de451cd063e019013a7e41c9723c9f/content/browser/renderer_host/media/audio_input_renderer_host.h
[modify] https://crrev.com/df7b7f8eb4de451cd063e019013a7e41c9723c9f/content/browser/renderer_host/media/audio_input_renderer_host_unittest.cc
[modify] https://crrev.com/df7b7f8eb4de451cd063e019013a7e41c9723c9f/content/browser/renderer_host/media/audio_input_sync_writer.h
[modify] https://crrev.com/df7b7f8eb4de451cd063e019013a7e41c9723c9f/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/df7b7f8eb4de451cd063e019013a7e41c9723c9f/content/browser/renderer_host/render_process_host_impl.h
[modify] https://crrev.com/df7b7f8eb4de451cd063e019013a7e41c9723c9f/content/browser/resources/media/dump_creator.js
[modify] https://crrev.com/df7b7f8eb4de451cd063e019013a7e41c9723c9f/content/browser/webrtc/webrtc_audio_debug_recordings_browsertest.cc
[modify] https://crrev.com/df7b7f8eb4de451cd063e019013a7e41c9723c9f/content/browser/webrtc/webrtc_internals.cc
[modify] https://crrev.com/df7b7f8eb4de451cd063e019013a7e41c9723c9f/media/audio/BUILD.gn
[modify] https://crrev.com/df7b7f8eb4de451cd063e019013a7e41c9723c9f/media/audio/audio_input_controller.cc
[modify] https://crrev.com/df7b7f8eb4de451cd063e019013a7e41c9723c9f/media/audio/audio_input_controller.h
[modify] https://crrev.com/df7b7f8eb4de451cd063e019013a7e41c9723c9f/media/audio/audio_input_controller_unittest.cc
[add] https://crrev.com/df7b7f8eb4de451cd063e019013a7e41c9723c9f/media/audio/audio_input_stream_data_interceptor.cc
[add] https://crrev.com/df7b7f8eb4de451cd063e019013a7e41c9723c9f/media/audio/audio_input_stream_data_interceptor.h
[add] https://crrev.com/df7b7f8eb4de451cd063e019013a7e41c9723c9f/media/audio/audio_input_stream_data_interceptor_unittest.cc
[modify] https://crrev.com/df7b7f8eb4de451cd063e019013a7e41c9723c9f/media/audio/audio_manager.cc
[modify] https://crrev.com/df7b7f8eb4de451cd063e019013a7e41c9723c9f/media/audio/audio_manager.h
[modify] https://crrev.com/df7b7f8eb4de451cd063e019013a7e41c9723c9f/media/audio/audio_manager_base.cc
[modify] https://crrev.com/df7b7f8eb4de451cd063e019013a7e41c9723c9f/media/audio/audio_manager_base.h
[modify] https://crrev.com/df7b7f8eb4de451cd063e019013a7e41c9723c9f/media/audio/audio_manager_unittest.cc
[modify] https://crrev.com/df7b7f8eb4de451cd063e019013a7e41c9723c9f/media/audio/mock_audio_manager.cc
[modify] https://crrev.com/df7b7f8eb4de451cd063e019013a7e41c9723c9f/media/audio/mock_audio_manager.h
[modify] https://crrev.com/df7b7f8eb4de451cd063e019013a7e41c9723c9f/media/audio/test_audio_input_controller_factory.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Oct 4 2017

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

commit 721855dafb1b2df9748a7d49c5ab0a3305b0f3c0
Author: Corentin Wallez <cwallez@chromium.org>
Date: Wed Oct 04 16:45:38 2017

Revert "Move microphone debug recording to AM."

This reverts commit df7b7f8eb4de451cd063e019013a7e41c9723c9f.

Reason for revert: WinAudioInputTest.WASAPIAudioInputStreamMiscCallingSequences started failing on Windows after this patch.

Original change's description:
> Move microphone debug recording to AM.
> 
> Introduces the AudioInputStreamDataInterceptor, which transparently sends audio
> data off for debug recording. AudioManagerBase wraps each created input stream
> in an interceptor to enable debug recording. A side effect of this refactoring
> is that WebContents input streams will not be recorded, but AFAIK noone ever
> used recordings anyways. I also somehow had to fix a bunch of IWYU, not sure
> how I got dragged into that :).
> 
> Since AudioInputRendererHost doesn't have to be accessed by the
> RenderProcessHostImpl anymore, the member variable was removed; the lifetime of
> the AudioInputRendererHost is now managed by the IPC::Channel as it is for other
> message filters.
> 
> Bug:  768610 
> Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
> Change-Id: I24a89435b283cd5df68373eb37d776a320d6ffd5
> Reviewed-on: https://chromium-review.googlesource.com/680217
> Commit-Queue: Max Morin <maxmorin@chromium.org>
> Reviewed-by: Olga Sharonova <olka@chromium.org>
> Reviewed-by: Tommi <tommi@chromium.org>
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Reviewed-by: Henrik Grunell <grunell@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#506338}

TBR=avi@chromium.org,tommi@chromium.org,grunell@chromium.org,olka@chromium.org,maxmorin@chromium.org

Change-Id: Id99c595e6ce4cba32e4a43ef93bee2ec329c749e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  768610 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/700079
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506412}
[modify] https://crrev.com/721855dafb1b2df9748a7d49c5ab0a3305b0f3c0/chrome/browser/media/webrtc/audio_debug_recordings_handler.cc
[modify] https://crrev.com/721855dafb1b2df9748a7d49c5ab0a3305b0f3c0/content/browser/renderer_host/media/audio_input_renderer_host.cc
[modify] https://crrev.com/721855dafb1b2df9748a7d49c5ab0a3305b0f3c0/content/browser/renderer_host/media/audio_input_renderer_host.h
[modify] https://crrev.com/721855dafb1b2df9748a7d49c5ab0a3305b0f3c0/content/browser/renderer_host/media/audio_input_renderer_host_unittest.cc
[modify] https://crrev.com/721855dafb1b2df9748a7d49c5ab0a3305b0f3c0/content/browser/renderer_host/media/audio_input_sync_writer.h
[modify] https://crrev.com/721855dafb1b2df9748a7d49c5ab0a3305b0f3c0/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/721855dafb1b2df9748a7d49c5ab0a3305b0f3c0/content/browser/renderer_host/render_process_host_impl.h
[modify] https://crrev.com/721855dafb1b2df9748a7d49c5ab0a3305b0f3c0/content/browser/resources/media/dump_creator.js
[modify] https://crrev.com/721855dafb1b2df9748a7d49c5ab0a3305b0f3c0/content/browser/webrtc/webrtc_audio_debug_recordings_browsertest.cc
[modify] https://crrev.com/721855dafb1b2df9748a7d49c5ab0a3305b0f3c0/content/browser/webrtc/webrtc_internals.cc
[modify] https://crrev.com/721855dafb1b2df9748a7d49c5ab0a3305b0f3c0/media/audio/BUILD.gn
[modify] https://crrev.com/721855dafb1b2df9748a7d49c5ab0a3305b0f3c0/media/audio/audio_input_controller.cc
[modify] https://crrev.com/721855dafb1b2df9748a7d49c5ab0a3305b0f3c0/media/audio/audio_input_controller.h
[modify] https://crrev.com/721855dafb1b2df9748a7d49c5ab0a3305b0f3c0/media/audio/audio_input_controller_unittest.cc
[delete] https://crrev.com/c23238753932312c791912e90d8e3c00f38aacd3/media/audio/audio_input_stream_data_interceptor.cc
[delete] https://crrev.com/c23238753932312c791912e90d8e3c00f38aacd3/media/audio/audio_input_stream_data_interceptor.h
[delete] https://crrev.com/c23238753932312c791912e90d8e3c00f38aacd3/media/audio/audio_input_stream_data_interceptor_unittest.cc
[modify] https://crrev.com/721855dafb1b2df9748a7d49c5ab0a3305b0f3c0/media/audio/audio_manager.cc
[modify] https://crrev.com/721855dafb1b2df9748a7d49c5ab0a3305b0f3c0/media/audio/audio_manager.h
[modify] https://crrev.com/721855dafb1b2df9748a7d49c5ab0a3305b0f3c0/media/audio/audio_manager_base.cc
[modify] https://crrev.com/721855dafb1b2df9748a7d49c5ab0a3305b0f3c0/media/audio/audio_manager_base.h
[modify] https://crrev.com/721855dafb1b2df9748a7d49c5ab0a3305b0f3c0/media/audio/audio_manager_unittest.cc
[modify] https://crrev.com/721855dafb1b2df9748a7d49c5ab0a3305b0f3c0/media/audio/mock_audio_manager.cc
[modify] https://crrev.com/721855dafb1b2df9748a7d49c5ab0a3305b0f3c0/media/audio/mock_audio_manager.h
[modify] https://crrev.com/721855dafb1b2df9748a7d49c5ab0a3305b0f3c0/media/audio/test_audio_input_controller_factory.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 5 2017

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

commit 9f78506b3d20875d0ebc836b0f96529bad101622
Author: Max Morin <maxmorin@chromium.org>
Date: Thu Oct 05 11:39:57 2017

Remove downcast in WinAudioInputTest.

The downcast is invalid after
https://chromium-review.googlesource.com/c/chromium/src/+/700079
has been relanded.

Drive-by: fix an instance of an int16_t[] being stored in a
unique_ptr<int16_t>.

Bug:  768610 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: If74ac21ba59370eeae719d16f337d14e40a91ccc
Reviewed-on: https://chromium-review.googlesource.com/702260
Reviewed-by: Tommi <tommi@chromium.org>
Commit-Queue: Max Morin <maxmorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506702}
[modify] https://crrev.com/9f78506b3d20875d0ebc836b0f96529bad101622/media/audio/win/audio_low_latency_input_win_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 16 2017

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

commit f1ec09cb51012e0a3147e97ee223dc007940c581
Author: Max Morin <maxmorin@chromium.org>
Date: Mon Oct 16 09:02:37 2017

Reland "Move microphone debug recording to AM."

This reverts commit 721855dafb1b2df9748a7d49c5ab0a3305b0f3c0.

Test was fixed in https://chromium-review.googlesource.com/c/chromium/src/+/702260, so I'm relanding this without changes now.

Original change's description:
> Revert "Move microphone debug recording to AM."
> 
> This reverts commit df7b7f8eb4de451cd063e019013a7e41c9723c9f.
> 
> Reason for revert: WinAudioInputTest.WASAPIAudioInputStreamMiscCallingSequences started failing on Windows after this patch.
> 
> Original change's description:
> > Move microphone debug recording to AM.
> > 
> > Introduces the AudioInputStreamDataInterceptor, which transparently sends audio
> > data off for debug recording. AudioManagerBase wraps each created input stream
> > in an interceptor to enable debug recording. A side effect of this refactoring
> > is that WebContents input streams will not be recorded, but AFAIK noone ever
> > used recordings anyways. I also somehow had to fix a bunch of IWYU, not sure
> > how I got dragged into that :).
> > 
> > Since AudioInputRendererHost doesn't have to be accessed by the
> > RenderProcessHostImpl anymore, the member variable was removed; the lifetime of
> > the AudioInputRendererHost is now managed by the IPC::Channel as it is for other
> > message filters.
> > 
> > Bug:  768610 
> > Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
> > Change-Id: I24a89435b283cd5df68373eb37d776a320d6ffd5
> > Reviewed-on: https://chromium-review.googlesource.com/680217
> > Commit-Queue: Max Morin <maxmorin@chromium.org>
> > Reviewed-by: Olga Sharonova <olka@chromium.org>
> > Reviewed-by: Tommi <tommi@chromium.org>
> > Reviewed-by: Avi Drissman <avi@chromium.org>
> > Reviewed-by: Henrik Grunell <grunell@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#506338}
> 
> TBR=avi@chromium.org,tommi@chromium.org,grunell@chromium.org,olka@chromium.org,maxmorin@chromium.org
> 
> Change-Id: Id99c595e6ce4cba32e4a43ef93bee2ec329c749e
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  768610 
> Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
> Reviewed-on: https://chromium-review.googlesource.com/700079
> Reviewed-by: Corentin Wallez <cwallez@chromium.org>
> Commit-Queue: Corentin Wallez <cwallez@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#506412}

TBR=avi@chromium.org,tommi@chromium.org,grunell@chromium.org,cwallez@chromium.org,olka@chromium.org,maxmorin@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  768610 
Change-Id: I19fb2d465dd91e747f6b8267279f27b3fb286dcd
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/720756
Reviewed-by: Max Morin <maxmorin@chromium.org>
Commit-Queue: Max Morin <maxmorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508999}
[modify] https://crrev.com/f1ec09cb51012e0a3147e97ee223dc007940c581/chrome/browser/media/webrtc/audio_debug_recordings_handler.cc
[modify] https://crrev.com/f1ec09cb51012e0a3147e97ee223dc007940c581/content/browser/renderer_host/media/audio_input_renderer_host.cc
[modify] https://crrev.com/f1ec09cb51012e0a3147e97ee223dc007940c581/content/browser/renderer_host/media/audio_input_renderer_host.h
[modify] https://crrev.com/f1ec09cb51012e0a3147e97ee223dc007940c581/content/browser/renderer_host/media/audio_input_renderer_host_unittest.cc
[modify] https://crrev.com/f1ec09cb51012e0a3147e97ee223dc007940c581/content/browser/renderer_host/media/audio_input_sync_writer.h
[modify] https://crrev.com/f1ec09cb51012e0a3147e97ee223dc007940c581/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/f1ec09cb51012e0a3147e97ee223dc007940c581/content/browser/renderer_host/render_process_host_impl.h
[modify] https://crrev.com/f1ec09cb51012e0a3147e97ee223dc007940c581/content/browser/resources/media/dump_creator.js
[modify] https://crrev.com/f1ec09cb51012e0a3147e97ee223dc007940c581/content/browser/webrtc/webrtc_audio_debug_recordings_browsertest.cc
[modify] https://crrev.com/f1ec09cb51012e0a3147e97ee223dc007940c581/content/browser/webrtc/webrtc_internals.cc
[modify] https://crrev.com/f1ec09cb51012e0a3147e97ee223dc007940c581/media/audio/BUILD.gn
[modify] https://crrev.com/f1ec09cb51012e0a3147e97ee223dc007940c581/media/audio/audio_input_controller.cc
[modify] https://crrev.com/f1ec09cb51012e0a3147e97ee223dc007940c581/media/audio/audio_input_controller.h
[modify] https://crrev.com/f1ec09cb51012e0a3147e97ee223dc007940c581/media/audio/audio_input_controller_unittest.cc
[add] https://crrev.com/f1ec09cb51012e0a3147e97ee223dc007940c581/media/audio/audio_input_stream_data_interceptor.cc
[add] https://crrev.com/f1ec09cb51012e0a3147e97ee223dc007940c581/media/audio/audio_input_stream_data_interceptor.h
[add] https://crrev.com/f1ec09cb51012e0a3147e97ee223dc007940c581/media/audio/audio_input_stream_data_interceptor_unittest.cc
[modify] https://crrev.com/f1ec09cb51012e0a3147e97ee223dc007940c581/media/audio/audio_manager.cc
[modify] https://crrev.com/f1ec09cb51012e0a3147e97ee223dc007940c581/media/audio/audio_manager.h
[modify] https://crrev.com/f1ec09cb51012e0a3147e97ee223dc007940c581/media/audio/audio_manager_base.cc
[modify] https://crrev.com/f1ec09cb51012e0a3147e97ee223dc007940c581/media/audio/audio_manager_base.h
[modify] https://crrev.com/f1ec09cb51012e0a3147e97ee223dc007940c581/media/audio/audio_manager_unittest.cc
[modify] https://crrev.com/f1ec09cb51012e0a3147e97ee223dc007940c581/media/audio/mock_audio_manager.cc
[modify] https://crrev.com/f1ec09cb51012e0a3147e97ee223dc007940c581/media/audio/mock_audio_manager.h
[modify] https://crrev.com/f1ec09cb51012e0a3147e97ee223dc007940c581/media/audio/test_audio_input_controller_factory.cc

Status: Fixed (was: Started)

Sign in to add a comment