Add async AudioRendererSink::GetOutputDeviceInfo and remove auth timeout for such calls. |
||||||||||||
Issue descriptionPer olka@ there are still a lot of auth timeouts with the new audio process, it also shows significant memory usage when launched at startup versus on demand. To help resolve this we should make an async version of the GODI API that at least <video>/<audio> can use. It's not quite possible for WebRTC, WebAudio without some significant refactoring. Put simply, this call to GODI should use a callback instead of being synchronous and delay init if device info is not yet available; if info is available the callback should be immediately executed: https://cs.chromium.org/chromium/src/media/renderers/audio_renderer_impl.cc?l=381 This won't fix cases where session_id should be used to select the device, since those invoke GetOutputDeviceInfo() at creation time: https://cs.chromium.org/chromium/src/content/renderer/media/audio/audio_renderer_mixer_manager.cc?l=162 While the GODI call happens on the media thread, during Pipeline::Shutdown() we join with the media thread which will cause a renderer thread hang if we don't change this call to be asynchronous: https://cs.chromium.org/chromium/src/media/base/pipeline_impl.cc?l=1058 An alternative solution to this issue would be to remove that join from PipelineImpl and just have a GetOutputDeviceInfo API which takes the timeout value. I'm actually not sure the media/render thread still needs to be joined... We might be able to just post the DataSource/Demuxer to the media thread after calling posting Stop() -- that's exactly how all of our unit tests work... I'll experiment with both approaches (possibly even doing both to avoid WaitableEvents).
,
Nov 15
Thanks Dale for looking into it!
,
Nov 16
The NextAction date has arrived: 2018-11-16
,
Nov 16
Probably won't get to this until Tuesday. Dan's working on: https://chromium-review.googlesource.com/c/chromium/src/+/1336659 https://chromium-review.googlesource.com/c/chromium/src/+/1338473
,
Nov 20
This is a bit more complicated than I was hoping. I forgot that GetOutputDeviceInfo doesn't actually initiate the auth request, that we share AudioRendererMixer instances across multiple threads, and that: https://cs.chromium.org/chromium/src/media/blink/webaudiosourceprovider_impl.cc?l=199 causes media to always take the blocking path. I think this is all fixable though, here's my proposal: - Add a new async only constructor parameter to AudioOutputDevice. This will remove the timeout from authorizations when created this way. - Remove GetOutputDeviceInfo() call and NullAudioSink fallback in WebAudioSourceProviderImpl during Initialize(). - Change WebAudioSourceProviderImpl to intercept and switch to NullAudioSink if GetOutputDeviceInfoAsync returns device_status != OUTPUT_DEVICE_STATUS_OK. - Switch AudioRendererImpl to use new GetOutputDeviceInfoAsync method during Initialize(). An alternative approach to the first step might be to refactor AudioOutputDevice into sync and async implementations, but this might take longer than 72. Will keep it in mind. Any objections to the above? I think it preserves all of our existing behaviors and statistics. Without the timeout the only risk should be more never-started playbacks, but I think the vast majority of the current cases are incorrect timeouts probably.
,
Nov 20
Actually I guess we already pass the source information, so we could just say that all AudioOutputDevice instances created for kSourceMediaElement will reject non-async calls and disable the timeout. Though that is complicated because it looks like AudioRendererMixerManager::GetMixer also calls GetOutputDeviceInfo... I don't think it should be doing that, ARMI always turns the nullptr returned into a OnRendererError() anyways. So instead we should just have AudioRendererMixerInput::Start() initiate an async request and issue an error if that fails. The last complication (so far) is SwitchOutputDevice, which also calls the sync GODI method, but SOD is an async method, so probably we just need to change ARMI::SwitchOutputDevice() to use the async version. Will dig into this more tomorrow.
,
Nov 20
The NextAction date has arrived: 2018-11-20
,
Nov 21
An update on the above AudioRendererMixerManager needs the device info in order to actually configure the mixer, so we'll need to rework the ARMM API to be asynchronous too to resolve this. I'm trying to hack a prototype together, but progress is slow. Will dig in some more tomorrow.
,
Nov 21
:( I suspected it might be not as easy as it seemed. Doesn't the change become too risky for M72?
,
Nov 21
,
Nov 21
Okay prototype here: https://chromium-review.googlesource.com/c/chromium/src/+/1347581 -- I'll split that into a few CLs: Thinking about the best way to split that out into patches now. First will definitely be just adding the async method itself. Will do that now.
,
Nov 22
Feel free to CQ these if they lgty Olga: https://chromium-review.googlesource.com/c/chromium/src/+/1347588 https://chromium-review.googlesource.com/c/chromium/src/+/1347146 This one can be reviewed, but should wait until other subsequent parts finished: https://chromium-review.googlesource.com/c/chromium/src/+/1347730 These can be used for discussion, but tests and SwitchOutputDevice isn't fixed yet: https://chromium-review.googlesource.com/c/chromium/src/+/1347795
,
Nov 22
Err, last one can be reviewed, but tests aren't all fixed yet.
,
Nov 22
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2bbd27e236f06313bfec25137d145496fa560343 commit 2bbd27e236f06313bfec25137d145496fa560343 Author: Dale Curtis <dalecurtis@chromium.org> Date: Thu Nov 22 07:17:15 2018 Remove synchronous blocking from PipelineImpl::Stop(). We should be able to avoid this with careful use of a trampoline through the media thread after PipelineImpl::Stop() completes. BUG= 905506 TEST=passes cq. Change-Id: I6a9c1a0e578c48a66af85588879346d26009d2ab Reviewed-on: https://chromium-review.googlesource.com/c/1336659 Reviewed-by: Fredrik Hubinette <hubbe@chromium.org> Reviewed-by: Dan Sanders <sandersd@chromium.org> Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Cr-Commit-Position: refs/heads/master@{#610327} [modify] https://crrev.com/2bbd27e236f06313bfec25137d145496fa560343/media/base/data_source.h [modify] https://crrev.com/2bbd27e236f06313bfec25137d145496fa560343/media/base/pipeline_impl.cc [modify] https://crrev.com/2bbd27e236f06313bfec25137d145496fa560343/media/blink/multibuffer_data_source.cc [modify] https://crrev.com/2bbd27e236f06313bfec25137d145496fa560343/media/blink/webmediaplayer_impl.cc [modify] https://crrev.com/2bbd27e236f06313bfec25137d145496fa560343/media/blink/webmediaplayer_impl.h [modify] https://crrev.com/2bbd27e236f06313bfec25137d145496fa560343/media/blink/webmediaplayer_impl_unittest.cc
,
Nov 22
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/da78e32ad75fe3cb97f921ba1f38db8c789c73be commit da78e32ad75fe3cb97f921ba1f38db8c789c73be Author: Anders Ruud <andruud@chromium.org> Date: Thu Nov 22 10:24:24 2018 Revert "Remove synchronous blocking from PipelineImpl::Stop()." This reverts commit 2bbd27e236f06313bfec25137d145496fa560343. Reason for revert: Suspected cause for media-related crashes: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.13%20Tests/6886 Original change's description: > Remove synchronous blocking from PipelineImpl::Stop(). > > We should be able to avoid this with careful use of a trampoline > through the media thread after PipelineImpl::Stop() completes. > > BUG= 905506 > TEST=passes cq. > > Change-Id: I6a9c1a0e578c48a66af85588879346d26009d2ab > Reviewed-on: https://chromium-review.googlesource.com/c/1336659 > Reviewed-by: Fredrik Hubinette <hubbe@chromium.org> > Reviewed-by: Dan Sanders <sandersd@chromium.org> > Commit-Queue: Dale Curtis <dalecurtis@chromium.org> > Cr-Commit-Position: refs/heads/master@{#610327} TBR=dalecurtis@chromium.org,xhwang@chromium.org,hubbe@chromium.org,sandersd@chromium.org Change-Id: Icc496d243065d76268ac22c87a03cf959d1e4948 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 905506 Reviewed-on: https://chromium-review.googlesource.com/c/1347365 Reviewed-by: Anders Ruud <andruud@chromium.org> Commit-Queue: Anders Ruud <andruud@chromium.org> Cr-Commit-Position: refs/heads/master@{#610363} [modify] https://crrev.com/da78e32ad75fe3cb97f921ba1f38db8c789c73be/media/base/data_source.h [modify] https://crrev.com/da78e32ad75fe3cb97f921ba1f38db8c789c73be/media/base/pipeline_impl.cc [modify] https://crrev.com/da78e32ad75fe3cb97f921ba1f38db8c789c73be/media/blink/multibuffer_data_source.cc [modify] https://crrev.com/da78e32ad75fe3cb97f921ba1f38db8c789c73be/media/blink/webmediaplayer_impl.cc [modify] https://crrev.com/da78e32ad75fe3cb97f921ba1f38db8c789c73be/media/blink/webmediaplayer_impl.h [modify] https://crrev.com/da78e32ad75fe3cb97f921ba1f38db8c789c73be/media/blink/webmediaplayer_impl_unittest.cc
,
Nov 26
,
Nov 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/63dd5d456b12a62a19793b361baea8b732853458 commit 63dd5d456b12a62a19793b361baea8b732853458 Author: Dale Curtis <dalecurtis@chromium.org> Date: Tue Nov 27 20:56:31 2018 Remove synchronous blocking from PipelineImpl::Stop() - take 2. We should be able to avoid this with careful use of a trampoline through the media thread after PipelineImpl::Stop() completes. This is an updated version of the last CL: https://chromium-review.googlesource.com/c/chromium/src/+/1336659 The difference is we now also preserve the RendererFactorySelector until after renderer shutdown. This passes all the failing tests I could find locally. BUG= 521176 , 905506 TEST=passes cq. R=sandersd Change-Id: I86c4814a91ee6c41f20f6bb8a05859ac44171d7e Reviewed-on: https://chromium-review.googlesource.com/c/1351791 Reviewed-by: Dan Sanders <sandersd@chromium.org> Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Cr-Commit-Position: refs/heads/master@{#611305} [modify] https://crrev.com/63dd5d456b12a62a19793b361baea8b732853458/media/base/data_source.h [modify] https://crrev.com/63dd5d456b12a62a19793b361baea8b732853458/media/base/pipeline_impl.cc [modify] https://crrev.com/63dd5d456b12a62a19793b361baea8b732853458/media/blink/multibuffer_data_source.cc [modify] https://crrev.com/63dd5d456b12a62a19793b361baea8b732853458/media/blink/webmediaplayer_impl.cc [modify] https://crrev.com/63dd5d456b12a62a19793b361baea8b732853458/media/blink/webmediaplayer_impl.h [modify] https://crrev.com/63dd5d456b12a62a19793b361baea8b732853458/media/blink/webmediaplayer_impl_unittest.cc
,
Nov 28
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/964cc812e77d2e3299f20248d9c95ab2166d8bba commit 964cc812e77d2e3299f20248d9c95ab2166d8bba Author: Tim Schumann <tschumann@chromium.org> Date: Wed Nov 28 08:47:03 2018 Revert "Remove synchronous blocking from PipelineImpl::Stop() - take 2." This reverts commit 63dd5d456b12a62a19793b361baea8b732853458. Reason for revert: Causes flakes in the following tests: virtual/video-surface-layer/media/track/track-cue-rendering-position-auto.html virtual/video-surface-layer/media/controls/volumechange-stopimmediatepropagation.html Bug: 909586 Original change's description: > Remove synchronous blocking from PipelineImpl::Stop() - take 2. > > We should be able to avoid this with careful use of a trampoline > through the media thread after PipelineImpl::Stop() completes. > > This is an updated version of the last CL: > https://chromium-review.googlesource.com/c/chromium/src/+/1336659 > > The difference is we now also preserve the RendererFactorySelector > until after renderer shutdown. This passes all the failing tests > I could find locally. > > BUG= 521176 , 905506 > TEST=passes cq. > R=sandersd > > Change-Id: I86c4814a91ee6c41f20f6bb8a05859ac44171d7e > Reviewed-on: https://chromium-review.googlesource.com/c/1351791 > Reviewed-by: Dan Sanders <sandersd@chromium.org> > Commit-Queue: Dale Curtis <dalecurtis@chromium.org> > Cr-Commit-Position: refs/heads/master@{#611305} TBR=dalecurtis@chromium.org,sandersd@chromium.org Change-Id: I8d70751211f938756bf5b98e86b298f4fe921617 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 521176 , 905506 Reviewed-on: https://chromium-review.googlesource.com/c/1352326 Reviewed-by: Tim Schumann <tschumann@chromium.org> Commit-Queue: Tim Schumann <tschumann@chromium.org> Cr-Commit-Position: refs/heads/master@{#611602} [modify] https://crrev.com/964cc812e77d2e3299f20248d9c95ab2166d8bba/media/base/data_source.h [modify] https://crrev.com/964cc812e77d2e3299f20248d9c95ab2166d8bba/media/base/pipeline_impl.cc [modify] https://crrev.com/964cc812e77d2e3299f20248d9c95ab2166d8bba/media/blink/multibuffer_data_source.cc [modify] https://crrev.com/964cc812e77d2e3299f20248d9c95ab2166d8bba/media/blink/webmediaplayer_impl.cc [modify] https://crrev.com/964cc812e77d2e3299f20248d9c95ab2166d8bba/media/blink/webmediaplayer_impl.h [modify] https://crrev.com/964cc812e77d2e3299f20248d9c95ab2166d8bba/media/blink/webmediaplayer_impl_unittest.cc
,
Nov 29
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d6514607ce6194e4065b923c288e33a02008e1c8 commit d6514607ce6194e4065b923c288e33a02008e1c8 Author: Dale Curtis <dalecurtis@chromium.org> Date: Thu Nov 29 20:00:09 2018 Add AudioRendererSink::GetOutputDeviceInfoAsync() API. This is part 1/4 CLs to move the <audio>/<video> elements off of a synchronous API that can lead to renderer hangs and premature audio renderer errors. BUG= 905506 TEST=updated tests, compiles. Change-Id: I9b26ed5ed8faea2f691b3a415dbdc79e18f5125a Reviewed-on: https://chromium-review.googlesource.com/c/1347588 Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Reviewed-by: Olga Sharonova <olka@chromium.org> Cr-Commit-Position: refs/heads/master@{#612330} [modify] https://crrev.com/d6514607ce6194e4065b923c288e33a02008e1c8/media/audio/audio_output_device.cc [modify] https://crrev.com/d6514607ce6194e4065b923c288e33a02008e1c8/media/audio/audio_output_device.h [modify] https://crrev.com/d6514607ce6194e4065b923c288e33a02008e1c8/media/audio/audio_output_device_unittest.cc [modify] https://crrev.com/d6514607ce6194e4065b923c288e33a02008e1c8/media/audio/audio_output_stream_sink.cc [modify] https://crrev.com/d6514607ce6194e4065b923c288e33a02008e1c8/media/audio/audio_output_stream_sink.h [modify] https://crrev.com/d6514607ce6194e4065b923c288e33a02008e1c8/media/audio/clockless_audio_sink.cc [modify] https://crrev.com/d6514607ce6194e4065b923c288e33a02008e1c8/media/audio/clockless_audio_sink.h [modify] https://crrev.com/d6514607ce6194e4065b923c288e33a02008e1c8/media/audio/null_audio_sink.cc [modify] https://crrev.com/d6514607ce6194e4065b923c288e33a02008e1c8/media/audio/null_audio_sink.h [modify] https://crrev.com/d6514607ce6194e4065b923c288e33a02008e1c8/media/base/audio_renderer_mixer_input.cc [modify] https://crrev.com/d6514607ce6194e4065b923c288e33a02008e1c8/media/base/audio_renderer_mixer_input.h [modify] https://crrev.com/d6514607ce6194e4065b923c288e33a02008e1c8/media/base/audio_renderer_sink.h [modify] https://crrev.com/d6514607ce6194e4065b923c288e33a02008e1c8/media/base/fake_audio_renderer_sink.cc [modify] https://crrev.com/d6514607ce6194e4065b923c288e33a02008e1c8/media/base/fake_audio_renderer_sink.h [modify] https://crrev.com/d6514607ce6194e4065b923c288e33a02008e1c8/media/base/mock_audio_renderer_sink.cc [modify] https://crrev.com/d6514607ce6194e4065b923c288e33a02008e1c8/media/base/mock_audio_renderer_sink.h [modify] https://crrev.com/d6514607ce6194e4065b923c288e33a02008e1c8/media/blink/webaudiosourceprovider_impl.cc [modify] https://crrev.com/d6514607ce6194e4065b923c288e33a02008e1c8/media/blink/webaudiosourceprovider_impl.h
,
Nov 29
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c4db771a5863cf17bba822c221c0c34d6e0c300b commit c4db771a5863cf17bba822c221c0c34d6e0c300b Author: Dale Curtis <dalecurtis@chromium.org> Date: Thu Nov 29 22:13:18 2018 Make authorization timeout provided to AudioOutputDevice configurable. This is part 2/4 CLs to move the <audio>/<video> elements off of a synchronous API that can lead to renderer hangs and premature audio renderer errors. Specifically this will allow us to create AudioRendererMixerInputs with an authorization timeout of zero when the asynchronous API is the only one in use. BUG= 905506 TEST=compiles. R=olka Change-Id: I298550db1e7bac9b73033566c17cff8f1d2252b3 Reviewed-on: https://chromium-review.googlesource.com/c/1347146 Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Reviewed-by: Olga Sharonova <olka@chromium.org> Cr-Commit-Position: refs/heads/master@{#612400} [modify] https://crrev.com/c4db771a5863cf17bba822c221c0c34d6e0c300b/content/renderer/media/audio/audio_device_factory.cc [modify] https://crrev.com/c4db771a5863cf17bba822c221c0c34d6e0c300b/content/renderer/media/audio/audio_device_factory.h [modify] https://crrev.com/c4db771a5863cf17bba822c221c0c34d6e0c300b/content/renderer/media/audio/mock_audio_device_factory.h [modify] https://crrev.com/c4db771a5863cf17bba822c221c0c34d6e0c300b/content/renderer/media/renderer_webaudiodevice_impl_unittest.cc [modify] https://crrev.com/c4db771a5863cf17bba822c221c0c34d6e0c300b/content/renderer/media/webrtc/webrtc_audio_renderer_unittest.cc
,
Nov 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/72a4baa716f40e87b73017f86b3d336a296498d3 commit 72a4baa716f40e87b73017f86b3d336a296498d3 Author: Dale Curtis <dalecurtis@chromium.org> Date: Fri Nov 30 00:17:03 2018 Stop using AudioRendererSinkCache for AudioRendererMixerManager. This is part 3/4 CLs to move the <audio>/<video> elements off of a synchronous API that can lead to renderer hangs and premature audio renderer errors. This moves the AudioRendererSinkCache from AudioRendererMixerManger to a base::NoDestructor instance in AudioDeviceFactory and stops ARMM from using the cache at all. The users remaining are WebAudio and WebRTC, we can reevaluate metrics on keeping it after. Due to base::(SingleThread|Sequenced)TaskRunner::Get() being deprecated for content/ I've had to switch to use base::PostTask to generate a cleanup task runner. Unfortunately this doesn't guarantee that it runs on the render thread, but since we're now using a static instance, we can just always use base::Unretained and drop the WeakPtr factory that was handling the cancellation of these tasks. There's no point in the cache for ARMM since we're going to require a sink to get a mixer. That sink will always be used to get output device info first too and then reused for the mixer. BUG= 905506 TEST=updated tests, compiles. R=olka Change-Id: I03408753f974e4c6fb9c89270508e26689162002 Reviewed-on: https://chromium-review.googlesource.com/c/1347730 Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Reviewed-by: Olga Sharonova <olka@chromium.org> Cr-Commit-Position: refs/heads/master@{#612460} [modify] https://crrev.com/72a4baa716f40e87b73017f86b3d336a296498d3/content/renderer/media/audio/audio_device_factory.cc [modify] https://crrev.com/72a4baa716f40e87b73017f86b3d336a296498d3/content/renderer/media/audio/audio_renderer_mixer_manager.cc [modify] https://crrev.com/72a4baa716f40e87b73017f86b3d336a296498d3/content/renderer/media/audio/audio_renderer_mixer_manager.h [modify] https://crrev.com/72a4baa716f40e87b73017f86b3d336a296498d3/content/renderer/media/audio/audio_renderer_mixer_manager_unittest.cc [modify] https://crrev.com/72a4baa716f40e87b73017f86b3d336a296498d3/content/renderer/media/audio/audio_renderer_sink_cache.h [modify] https://crrev.com/72a4baa716f40e87b73017f86b3d336a296498d3/content/renderer/media/audio/audio_renderer_sink_cache_impl.cc [modify] https://crrev.com/72a4baa716f40e87b73017f86b3d336a296498d3/content/renderer/media/audio/audio_renderer_sink_cache_impl.h [modify] https://crrev.com/72a4baa716f40e87b73017f86b3d336a296498d3/content/renderer/media/audio/audio_renderer_sink_cache_unittest.cc
,
Nov 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/41607b54686f80bc672c294161ca0da1cf49f89f commit 41607b54686f80bc672c294161ca0da1cf49f89f Author: Dale Curtis <dalecurtis@chromium.org> Date: Fri Nov 30 02:28:04 2018 Convert <audio> pipeline to use async device info requests. This is part 4/4 CLs to move the <audio>/<video> elements off of a synchronous API that can lead to renderer hangs and premature audio renderer errors. This changes the AudioRendererMixerPool API to require an AudioRendererSink and OutputDeviceInfo when providing a mixer. AudioRendererMixerInputs are subsequently changed to use the new API. Likewise AudioRendererImpl also now uses the asynchronous API. To simplify the async process, AudioRendererMixerInputs will only setup correctly when OutputDeviceInfo has been requested ahead of time, since that's the pattern that AudioRendererImpl will use. This also moves the NullAudioSink setup from WebAudioSourceProvider over to the AudioRendererImpl. This causes WebAudio to be disconnected from the element, but if audio isn't work anyways, it shouldn't matter. BUG= 905506 TEST=updated tests, compiles, runs. R=olka Change-Id: I4edf89bb1e20cc91191a6eb97a0e38b6aeba68f8 Reviewed-on: https://chromium-review.googlesource.com/c/1347795 Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Reviewed-by: Jesse Doherty <jwd@chromium.org> Reviewed-by: Olga Sharonova <olka@chromium.org> Reviewed-by: Chrome Cunningham <chcunningham@chromium.org> Cr-Commit-Position: refs/heads/master@{#612526} [modify] https://crrev.com/41607b54686f80bc672c294161ca0da1cf49f89f/content/renderer/media/audio/audio_device_factory.cc [modify] https://crrev.com/41607b54686f80bc672c294161ca0da1cf49f89f/content/renderer/media/audio/audio_device_factory.h [modify] https://crrev.com/41607b54686f80bc672c294161ca0da1cf49f89f/content/renderer/media/audio/audio_renderer_mixer_manager.cc [modify] https://crrev.com/41607b54686f80bc672c294161ca0da1cf49f89f/content/renderer/media/audio/audio_renderer_mixer_manager.h [modify] https://crrev.com/41607b54686f80bc672c294161ca0da1cf49f89f/content/renderer/media/audio/audio_renderer_mixer_manager_unittest.cc [modify] https://crrev.com/41607b54686f80bc672c294161ca0da1cf49f89f/media/audio/BUILD.gn [modify] https://crrev.com/41607b54686f80bc672c294161ca0da1cf49f89f/media/base/audio_renderer_mixer.cc [modify] https://crrev.com/41607b54686f80bc672c294161ca0da1cf49f89f/media/base/audio_renderer_mixer.h [modify] https://crrev.com/41607b54686f80bc672c294161ca0da1cf49f89f/media/base/audio_renderer_mixer_input.cc [modify] https://crrev.com/41607b54686f80bc672c294161ca0da1cf49f89f/media/base/audio_renderer_mixer_input.h [modify] https://crrev.com/41607b54686f80bc672c294161ca0da1cf49f89f/media/base/audio_renderer_mixer_input_unittest.cc [modify] https://crrev.com/41607b54686f80bc672c294161ca0da1cf49f89f/media/base/audio_renderer_mixer_pool.h [modify] https://crrev.com/41607b54686f80bc672c294161ca0da1cf49f89f/media/base/audio_renderer_mixer_unittest.cc [modify] https://crrev.com/41607b54686f80bc672c294161ca0da1cf49f89f/media/blink/webaudiosourceprovider_impl.cc [modify] https://crrev.com/41607b54686f80bc672c294161ca0da1cf49f89f/media/blink/webaudiosourceprovider_impl.h [modify] https://crrev.com/41607b54686f80bc672c294161ca0da1cf49f89f/media/blink/webaudiosourceprovider_impl_unittest.cc [modify] https://crrev.com/41607b54686f80bc672c294161ca0da1cf49f89f/media/renderers/BUILD.gn [modify] https://crrev.com/41607b54686f80bc672c294161ca0da1cf49f89f/media/renderers/audio_renderer_impl.cc [modify] https://crrev.com/41607b54686f80bc672c294161ca0da1cf49f89f/media/renderers/audio_renderer_impl.h [modify] https://crrev.com/41607b54686f80bc672c294161ca0da1cf49f89f/tools/metrics/histograms/histograms.xml
,
Dec 3
mr-72 for cl 3/4 and 4/4 which got unlucky with the branch cut. 1/4 and 2/4 made it in okay.
,
Dec 3
This bug requires manual review: Reverts referenced in bugdroid comments after merge request. Please contact the milestone owner if you have questions. Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 4
,
Dec 4
Will also need the fix merged from issue 910951.
,
Dec 4
Data is still coming in, but there is a small change in authorization timed out counts; possibly we went from ~0.06% failure to ~0.03%. The remaining timeouts I guess are all coming from WebAudio/WebRTC/etc. Cache utilization is way up, so I guess WebAudio/WebRTC/etc are also better about reusing the sink. We are seeing about 25% more device authorizations, it's unclear if that's due to <audio> using more sinks or just a natural expectation of bifurcating the sinks based on those w/ and w/o time outs (thus preventing sharing with WebAudio/WebRTC/etc). We can consider a cache for the discarded authorized sinks. https://uma.googleplex.com/p/chrome/timeline_v2/?sid=1d5e23547ac3371bc9c76d0265396be9
,
Dec 4
Default group of audio service experiment [1] (to remove audio process interference) Media.Audio.Render.OutputDeviceStatus: Ok bucket dropped, and Not Authorized way up. And cache utilization seems to drop? https://uma.googleplex.com/p/chrome/timeline_v2/?sid=59b9d485a50c3b2f5acc16759d89e211
,
Dec 4
Hmm, that same graph in my link shows it returns to normal levels in 73.0.3629.0.
,
Dec 4
Cache utilization drop is probably expected.
,
Dec 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3b3f55bb2996cde48fbb1d8fbecf849fb348e4b5 commit 3b3f55bb2996cde48fbb1d8fbecf849fb348e4b5 Author: Dale Curtis <dalecurtis@chromium.org> Date: Tue Dec 04 22:40:20 2018 Cleanup comments and interfaces for AudioRendererMixer+Input. Some cleanups I noticed while working on adding the async callback; does the following: - Fixes some spelling errors. - Removes the need to save a callback to subscribe for errors. - Removes a stale comment associated with the error callbcks. - Removes some unused methods from the ARM interface. - Always posts the OutputDeviceInfoCB per documented contract. - Switches to "using", "RepeatingCallback", etc where appropriate. - Switches to base::flat_set/map where appropriate. BUG= 905506 TEST=existing tests all pass. Change-Id: Ib670c04ed59b98b07c18cc3f3afac44d802a2a68 Reviewed-on: https://chromium-review.googlesource.com/c/1357540 Reviewed-by: Olga Sharonova <olka@chromium.org> Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Cr-Commit-Position: refs/heads/master@{#613731} [modify] https://crrev.com/3b3f55bb2996cde48fbb1d8fbecf849fb348e4b5/content/renderer/media/audio/audio_renderer_mixer_manager.cc [modify] https://crrev.com/3b3f55bb2996cde48fbb1d8fbecf849fb348e4b5/content/renderer/media/audio/audio_renderer_mixer_manager.h [modify] https://crrev.com/3b3f55bb2996cde48fbb1d8fbecf849fb348e4b5/content/renderer/media/audio/audio_renderer_mixer_manager_unittest.cc [modify] https://crrev.com/3b3f55bb2996cde48fbb1d8fbecf849fb348e4b5/media/base/audio_renderer_mixer.cc [modify] https://crrev.com/3b3f55bb2996cde48fbb1d8fbecf849fb348e4b5/media/base/audio_renderer_mixer.h [modify] https://crrev.com/3b3f55bb2996cde48fbb1d8fbecf849fb348e4b5/media/base/audio_renderer_mixer_input.cc [modify] https://crrev.com/3b3f55bb2996cde48fbb1d8fbecf849fb348e4b5/media/base/audio_renderer_mixer_input.h
,
Dec 4
Seems like there are still CLs that are coming in. Can you please confirm if these are ready to be reviewed? How safe are these fixes and are they behind a flag?
,
Dec 4
c#31 is just cleanups that don't need to be merged. They should be safe, half the changes are already in the M72 branch, the other half just got unlucky with the cut.
,
Dec 4
Thanks for more context. Approving merge to M72, branch:3626
,
Dec 4
Thanks, I've just landed the merges. So this issue is now fixed.
,
Dec 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d83c0020c5355f4db3e530ff5d5bc359e051eaa9 commit d83c0020c5355f4db3e530ff5d5bc359e051eaa9 Author: Dale Curtis <dalecurtis@chromium.org> Date: Tue Dec 04 23:31:13 2018 Merge 72: "Stop using AudioRendererSinkCache for AudioRendererMixerManager." This is part 3/4 CLs to move the <audio>/<video> elements off of a synchronous API that can lead to renderer hangs and premature audio renderer errors. This moves the AudioRendererSinkCache from AudioRendererMixerManger to a base::NoDestructor instance in AudioDeviceFactory and stops ARMM from using the cache at all. The users remaining are WebAudio and WebRTC, we can reevaluate metrics on keeping it after. Due to base::(SingleThread|Sequenced)TaskRunner::Get() being deprecated for content/ I've had to switch to use base::PostTask to generate a cleanup task runner. Unfortunately this doesn't guarantee that it runs on the render thread, but since we're now using a static instance, we can just always use base::Unretained and drop the WeakPtr factory that was handling the cancellation of these tasks. There's no point in the cache for ARMM since we're going to require a sink to get a mixer. That sink will always be used to get output device info first too and then reused for the mixer. BUG= 905506 TEST=updated tests, compiles. R=olka Change-Id: I03408753f974e4c6fb9c89270508e26689162002 Reviewed-on: https://chromium-review.googlesource.com/c/1347730 Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Reviewed-by: Olga Sharonova <olka@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#612460}(cherry picked from commit 72a4baa716f40e87b73017f86b3d336a296498d3) Reviewed-on: https://chromium-review.googlesource.com/c/1362294 Reviewed-by: Dale Curtis <dalecurtis@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#48} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} [modify] https://crrev.com/d83c0020c5355f4db3e530ff5d5bc359e051eaa9/content/renderer/media/audio/audio_device_factory.cc [modify] https://crrev.com/d83c0020c5355f4db3e530ff5d5bc359e051eaa9/content/renderer/media/audio/audio_renderer_mixer_manager.cc [modify] https://crrev.com/d83c0020c5355f4db3e530ff5d5bc359e051eaa9/content/renderer/media/audio/audio_renderer_mixer_manager.h [modify] https://crrev.com/d83c0020c5355f4db3e530ff5d5bc359e051eaa9/content/renderer/media/audio/audio_renderer_mixer_manager_unittest.cc [modify] https://crrev.com/d83c0020c5355f4db3e530ff5d5bc359e051eaa9/content/renderer/media/audio/audio_renderer_sink_cache.h [modify] https://crrev.com/d83c0020c5355f4db3e530ff5d5bc359e051eaa9/content/renderer/media/audio/audio_renderer_sink_cache_impl.cc [modify] https://crrev.com/d83c0020c5355f4db3e530ff5d5bc359e051eaa9/content/renderer/media/audio/audio_renderer_sink_cache_impl.h [modify] https://crrev.com/d83c0020c5355f4db3e530ff5d5bc359e051eaa9/content/renderer/media/audio/audio_renderer_sink_cache_unittest.cc
,
Dec 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6003b71a52080e3f5e2c540b537b1528b10908e0 commit 6003b71a52080e3f5e2c540b537b1528b10908e0 Author: Dale Curtis <dalecurtis@chromium.org> Date: Tue Dec 04 23:31:46 2018 Merge M72: "Convert <audio> pipeline to use async device info requests." This is part 4/4 CLs to move the <audio>/<video> elements off of a synchronous API that can lead to renderer hangs and premature audio renderer errors. This changes the AudioRendererMixerPool API to require an AudioRendererSink and OutputDeviceInfo when providing a mixer. AudioRendererMixerInputs are subsequently changed to use the new API. Likewise AudioRendererImpl also now uses the asynchronous API. To simplify the async process, AudioRendererMixerInputs will only setup correctly when OutputDeviceInfo has been requested ahead of time, since that's the pattern that AudioRendererImpl will use. This also moves the NullAudioSink setup from WebAudioSourceProvider over to the AudioRendererImpl. This causes WebAudio to be disconnected from the element, but if audio isn't work anyways, it shouldn't matter. BUG= 905506 TEST=updated tests, compiles, runs. R=olka Change-Id: I4edf89bb1e20cc91191a6eb97a0e38b6aeba68f8 Reviewed-on: https://chromium-review.googlesource.com/c/1347795 Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Reviewed-by: Jesse Doherty <jwd@chromium.org> Reviewed-by: Olga Sharonova <olka@chromium.org> Reviewed-by: Chrome Cunningham <chcunningham@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#612526}(cherry picked from commit 41607b54686f80bc672c294161ca0da1cf49f89f) Reviewed-on: https://chromium-review.googlesource.com/c/1362295 Reviewed-by: Dale Curtis <dalecurtis@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#49} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} [modify] https://crrev.com/6003b71a52080e3f5e2c540b537b1528b10908e0/content/renderer/media/audio/audio_device_factory.cc [modify] https://crrev.com/6003b71a52080e3f5e2c540b537b1528b10908e0/content/renderer/media/audio/audio_device_factory.h [modify] https://crrev.com/6003b71a52080e3f5e2c540b537b1528b10908e0/content/renderer/media/audio/audio_renderer_mixer_manager.cc [modify] https://crrev.com/6003b71a52080e3f5e2c540b537b1528b10908e0/content/renderer/media/audio/audio_renderer_mixer_manager.h [modify] https://crrev.com/6003b71a52080e3f5e2c540b537b1528b10908e0/content/renderer/media/audio/audio_renderer_mixer_manager_unittest.cc [modify] https://crrev.com/6003b71a52080e3f5e2c540b537b1528b10908e0/media/audio/BUILD.gn [modify] https://crrev.com/6003b71a52080e3f5e2c540b537b1528b10908e0/media/base/audio_renderer_mixer.cc [modify] https://crrev.com/6003b71a52080e3f5e2c540b537b1528b10908e0/media/base/audio_renderer_mixer.h [modify] https://crrev.com/6003b71a52080e3f5e2c540b537b1528b10908e0/media/base/audio_renderer_mixer_input.cc [modify] https://crrev.com/6003b71a52080e3f5e2c540b537b1528b10908e0/media/base/audio_renderer_mixer_input.h [modify] https://crrev.com/6003b71a52080e3f5e2c540b537b1528b10908e0/media/base/audio_renderer_mixer_input_unittest.cc [modify] https://crrev.com/6003b71a52080e3f5e2c540b537b1528b10908e0/media/base/audio_renderer_mixer_pool.h [modify] https://crrev.com/6003b71a52080e3f5e2c540b537b1528b10908e0/media/base/audio_renderer_mixer_unittest.cc [modify] https://crrev.com/6003b71a52080e3f5e2c540b537b1528b10908e0/media/blink/webaudiosourceprovider_impl.cc [modify] https://crrev.com/6003b71a52080e3f5e2c540b537b1528b10908e0/media/blink/webaudiosourceprovider_impl.h [modify] https://crrev.com/6003b71a52080e3f5e2c540b537b1528b10908e0/media/blink/webaudiosourceprovider_impl_unittest.cc [modify] https://crrev.com/6003b71a52080e3f5e2c540b537b1528b10908e0/media/renderers/BUILD.gn [modify] https://crrev.com/6003b71a52080e3f5e2c540b537b1528b10908e0/media/renderers/audio_renderer_impl.cc [modify] https://crrev.com/6003b71a52080e3f5e2c540b537b1528b10908e0/media/renderers/audio_renderer_impl.h [modify] https://crrev.com/6003b71a52080e3f5e2c540b537b1528b10908e0/tools/metrics/histograms/histograms.xml
,
Dec 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/38e8f7e485a1614b52f39b754b6539004caa803f commit 38e8f7e485a1614b52f39b754b6539004caa803f Author: Dale Curtis <dalecurtis@chromium.org> Date: Tue Dec 04 23:32:05 2018 Merge M72: "Release sink when AudioSourceProvider::SetClient() is called." There's currently no way to resume these sinks from html/js so there's no point in restoring the sink state after disconnection. Specifically you can't call MediaElementAudioSourceNode.disconnect() and expect the underlying media element to be able to ever work again. See this demo: http://storage.googleapis.com/dalecurtis/webaudio-test.html Once AudioContext.createMediaElementSource(element) has been called on an element, that element can never be used standalone again. It must always be connected to the AudioContext to be used. In fact it's actually causing unnecessary work when the elements are destructed since removing the client respins the sink and mixer, etc. BUG= 905506 ,910951 TEST=passes cq. R=hongchan Change-Id: I5ff7fc532545075d62859a30f96d17c83bff9d21 Reviewed-on: https://chromium-review.googlesource.com/c/1359092 Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Reviewed-by: Hongchan Choi <hongchan@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#613346}(cherry picked from commit 391671c4475861deaee0c91601326d03e262598b) Reviewed-on: https://chromium-review.googlesource.com/c/1362296 Reviewed-by: Dale Curtis <dalecurtis@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#50} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} [modify] https://crrev.com/38e8f7e485a1614b52f39b754b6539004caa803f/media/blink/webaudiosourceprovider_impl.cc [modify] https://crrev.com/38e8f7e485a1614b52f39b754b6539004caa803f/media/blink/webaudiosourceprovider_impl_unittest.cc
,
Dec 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cdd5484426ec936e1fe68c2eaf3578c6b8647e0d commit cdd5484426ec936e1fe68c2eaf3578c6b8647e0d Author: Dale Curtis <dalecurtis@chromium.org> Date: Tue Dec 04 23:32:34 2018 Merge M72: "Cleanup comments and interfaces for AudioRendererMixer+Input." Some cleanups I noticed while working on adding the async callback; does the following: - Fixes some spelling errors. - Removes the need to save a callback to subscribe for errors. - Removes a stale comment associated with the error callbcks. - Removes some unused methods from the ARM interface. - Always posts the OutputDeviceInfoCB per documented contract. - Switches to "using", "RepeatingCallback", etc where appropriate. - Switches to base::flat_set/map where appropriate. BUG= 905506 TEST=existing tests all pass. Change-Id: Ib670c04ed59b98b07c18cc3f3afac44d802a2a68 Reviewed-on: https://chromium-review.googlesource.com/c/1357540 Reviewed-by: Olga Sharonova <olka@chromium.org> Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#613731}(cherry picked from commit 3b3f55bb2996cde48fbb1d8fbecf849fb348e4b5) Reviewed-on: https://chromium-review.googlesource.com/c/1362216 Reviewed-by: Dale Curtis <dalecurtis@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#51} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} [modify] https://crrev.com/cdd5484426ec936e1fe68c2eaf3578c6b8647e0d/content/renderer/media/audio/audio_renderer_mixer_manager.cc [modify] https://crrev.com/cdd5484426ec936e1fe68c2eaf3578c6b8647e0d/content/renderer/media/audio/audio_renderer_mixer_manager.h [modify] https://crrev.com/cdd5484426ec936e1fe68c2eaf3578c6b8647e0d/content/renderer/media/audio/audio_renderer_mixer_manager_unittest.cc [modify] https://crrev.com/cdd5484426ec936e1fe68c2eaf3578c6b8647e0d/media/base/audio_renderer_mixer.cc [modify] https://crrev.com/cdd5484426ec936e1fe68c2eaf3578c6b8647e0d/media/base/audio_renderer_mixer.h [modify] https://crrev.com/cdd5484426ec936e1fe68c2eaf3578c6b8647e0d/media/base/audio_renderer_mixer_input.cc [modify] https://crrev.com/cdd5484426ec936e1fe68c2eaf3578c6b8647e0d/media/base/audio_renderer_mixer_input.h
,
Dec 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f273f8f008de42ca3a951f85661ded5615f33e7a commit f273f8f008de42ca3a951f85661ded5615f33e7a Author: Dale Curtis <dalecurtis@chromium.org> Date: Thu Dec 13 23:40:33 2018 Remove synchronous blocking from PipelineImpl::Stop() - take 3. We should be able to avoid this with careful use of a trampoline through the media thread after PipelineImpl::Stop() completes. This is an updated version of the last CL: https://chromium-review.googlesource.com/c/chromium/src/+/1351791 PS#1 is the original change, latest PS is the fixed change. The differences are as follows: - We now disconnect the WebSurfaceLayerBridge from WMPI and preserve it until after renderer shutdown -- otherwise layout tests don't always get the last painted frame. - When stopping the pipeline for HLS playback we now destruct the DataSource and Demuxer at time of OnError(), rather than waiting until the subsequent StartPipeline() call. This ensures the survive until renderer stop completes. BUG= 521176 , 905506 , 909586 , 909832 TEST=cq, layout tests, hls playback test. Change-Id: Ifa895a6abf2b2ca2d51c126c31f210e594b261b0 Reviewed-on: https://chromium-review.googlesource.com/c/1351531 Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Reviewed-by: Philip Rogers <pdr@chromium.org> Reviewed-by: Frank Liberato <liberato@chromium.org> Cr-Commit-Position: refs/heads/master@{#616493} [modify] https://crrev.com/f273f8f008de42ca3a951f85661ded5615f33e7a/content/renderer/media/stream/webmediaplayer_ms_unittest.cc [modify] https://crrev.com/f273f8f008de42ca3a951f85661ded5615f33e7a/media/base/data_source.h [modify] https://crrev.com/f273f8f008de42ca3a951f85661ded5615f33e7a/media/base/pipeline_impl.cc [modify] https://crrev.com/f273f8f008de42ca3a951f85661ded5615f33e7a/media/blink/multibuffer_data_source.cc [modify] https://crrev.com/f273f8f008de42ca3a951f85661ded5615f33e7a/media/blink/webmediaplayer_impl.cc [modify] https://crrev.com/f273f8f008de42ca3a951f85661ded5615f33e7a/media/blink/webmediaplayer_impl.h [modify] https://crrev.com/f273f8f008de42ca3a951f85661ded5615f33e7a/media/blink/webmediaplayer_impl_unittest.cc [modify] https://crrev.com/f273f8f008de42ca3a951f85661ded5615f33e7a/third_party/blink/public/platform/web_surface_layer_bridge.h [modify] https://crrev.com/f273f8f008de42ca3a951f85661ded5615f33e7a/third_party/blink/renderer/platform/graphics/surface_layer_bridge.cc [modify] https://crrev.com/f273f8f008de42ca3a951f85661ded5615f33e7a/third_party/blink/renderer/platform/graphics/surface_layer_bridge.h
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/38e8f7e485a1614b52f39b754b6539004caa803f Commit: 38e8f7e485a1614b52f39b754b6539004caa803f Author: dalecurtis@chromium.org Commiter: dalecurtis@chromium.org Date: 2018-12-04 23:32:05 +0000 UTC Merge M72: "Release sink when AudioSourceProvider::SetClient() is called." There's currently no way to resume these sinks from html/js so there's no point in restoring the sink state after disconnection. Specifically you can't call MediaElementAudioSourceNode.disconnect() and expect the underlying media element to be able to ever work again. See this demo: http://storage.googleapis.com/dalecurtis/webaudio-test.html Once AudioContext.createMediaElementSource(element) has been called on an element, that element can never be used standalone again. It must always be connected to the AudioContext to be used. In fact it's actually causing unnecessary work when the elements are destructed since removing the client respins the sink and mixer, etc. BUG= 905506 ,910951 TEST=passes cq. R=hongchan Change-Id: I5ff7fc532545075d62859a30f96d17c83bff9d21 Reviewed-on: https://chromium-review.googlesource.com/c/1359092 Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Reviewed-by: Hongchan Choi <hongchan@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#613346}(cherry picked from commit 391671c4475861deaee0c91601326d03e262598b) Reviewed-on: https://chromium-review.googlesource.com/c/1362296 Reviewed-by: Dale Curtis <dalecurtis@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#50} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6003b71a52080e3f5e2c540b537b1528b10908e0 Commit: 6003b71a52080e3f5e2c540b537b1528b10908e0 Author: dalecurtis@chromium.org Commiter: dalecurtis@chromium.org Date: 2018-12-04 23:31:46 +0000 UTC Merge M72: "Convert <audio> pipeline to use async device info requests." This is part 4/4 CLs to move the <audio>/<video> elements off of a synchronous API that can lead to renderer hangs and premature audio renderer errors. This changes the AudioRendererMixerPool API to require an AudioRendererSink and OutputDeviceInfo when providing a mixer. AudioRendererMixerInputs are subsequently changed to use the new API. Likewise AudioRendererImpl also now uses the asynchronous API. To simplify the async process, AudioRendererMixerInputs will only setup correctly when OutputDeviceInfo has been requested ahead of time, since that's the pattern that AudioRendererImpl will use. This also moves the NullAudioSink setup from WebAudioSourceProvider over to the AudioRendererImpl. This causes WebAudio to be disconnected from the element, but if audio isn't work anyways, it shouldn't matter. BUG= 905506 TEST=updated tests, compiles, runs. R=olka Change-Id: I4edf89bb1e20cc91191a6eb97a0e38b6aeba68f8 Reviewed-on: https://chromium-review.googlesource.com/c/1347795 Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Reviewed-by: Jesse Doherty <jwd@chromium.org> Reviewed-by: Olga Sharonova <olka@chromium.org> Reviewed-by: Chrome Cunningham <chcunningham@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#612526}(cherry picked from commit 41607b54686f80bc672c294161ca0da1cf49f89f) Reviewed-on: https://chromium-review.googlesource.com/c/1362295 Reviewed-by: Dale Curtis <dalecurtis@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#49} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cdd5484426ec936e1fe68c2eaf3578c6b8647e0d Commit: cdd5484426ec936e1fe68c2eaf3578c6b8647e0d Author: dalecurtis@chromium.org Commiter: dalecurtis@chromium.org Date: 2018-12-04 23:32:34 +0000 UTC Merge M72: "Cleanup comments and interfaces for AudioRendererMixer+Input." Some cleanups I noticed while working on adding the async callback; does the following: - Fixes some spelling errors. - Removes the need to save a callback to subscribe for errors. - Removes a stale comment associated with the error callbcks. - Removes some unused methods from the ARM interface. - Always posts the OutputDeviceInfoCB per documented contract. - Switches to "using", "RepeatingCallback", etc where appropriate. - Switches to base::flat_set/map where appropriate. BUG= 905506 TEST=existing tests all pass. Change-Id: Ib670c04ed59b98b07c18cc3f3afac44d802a2a68 Reviewed-on: https://chromium-review.googlesource.com/c/1357540 Reviewed-by: Olga Sharonova <olka@chromium.org> Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#613731}(cherry picked from commit 3b3f55bb2996cde48fbb1d8fbecf849fb348e4b5) Reviewed-on: https://chromium-review.googlesource.com/c/1362216 Reviewed-by: Dale Curtis <dalecurtis@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#51} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d83c0020c5355f4db3e530ff5d5bc359e051eaa9 Commit: d83c0020c5355f4db3e530ff5d5bc359e051eaa9 Author: dalecurtis@chromium.org Commiter: dalecurtis@chromium.org Date: 2018-12-04 23:31:13 +0000 UTC Merge 72: "Stop using AudioRendererSinkCache for AudioRendererMixerManager." This is part 3/4 CLs to move the <audio>/<video> elements off of a synchronous API that can lead to renderer hangs and premature audio renderer errors. This moves the AudioRendererSinkCache from AudioRendererMixerManger to a base::NoDestructor instance in AudioDeviceFactory and stops ARMM from using the cache at all. The users remaining are WebAudio and WebRTC, we can reevaluate metrics on keeping it after. Due to base::(SingleThread|Sequenced)TaskRunner::Get() being deprecated for content/ I've had to switch to use base::PostTask to generate a cleanup task runner. Unfortunately this doesn't guarantee that it runs on the render thread, but since we're now using a static instance, we can just always use base::Unretained and drop the WeakPtr factory that was handling the cancellation of these tasks. There's no point in the cache for ARMM since we're going to require a sink to get a mixer. That sink will always be used to get output device info first too and then reused for the mixer. BUG= 905506 TEST=updated tests, compiles. R=olka Change-Id: I03408753f974e4c6fb9c89270508e26689162002 Reviewed-on: https://chromium-review.googlesource.com/c/1347730 Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Reviewed-by: Olga Sharonova <olka@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#612460}(cherry picked from commit 72a4baa716f40e87b73017f86b3d336a296498d3) Reviewed-on: https://chromium-review.googlesource.com/c/1362294 Reviewed-by: Dale Curtis <dalecurtis@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#48} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by dalecurtis@google.com
, Nov 15