[Media] Clean up AudioRendererSink, fix ownership [chromium/src : main]
Jordan Bayles (Gerrit)
Attention is currently required from: Dale Curtis, Mark Foltz, Matthew Wolenetz.
Jordan Bayles would like Dale Curtis, Matthew Wolenetz and Mark Foltz to review this change.
[Media] Clean up AudioRendererSink, fix ownership
This patch changes AudioRendererSink to be owned by a std::unique_ptr
instead of a scoped_refptr, clarifying the ownership of this class and
its descendents as well as fixing a variety of bugs and potential bugs
caused by confusing ownership structures.
Of significant note is the changes to the AudioRendererSinkCache, which
now maintains ownership of sinks instead of a complex and confusing ref
counted sharing with callers + garbage collection.
Bug: 151051, 1381431
Change-Id: Icd9bf047810df2441c874263cd28ecb4ad2c839e
---
M components/cast_streaming/renderer/playback_command_forwarding_renderer_factory_unittest.cc
M content/renderer/media/media_factory.cc
M content/renderer/media/renderer_webaudiodevice_impl.cc
M content/renderer/media/renderer_webaudiodevice_impl.h
M content/renderer/media/renderer_webaudiodevice_impl_unittest.cc
M content/renderer/renderer_blink_platform_impl.cc
M content/renderer/renderer_blink_platform_impl.h
M media/audio/BUILD.gn
M media/audio/audio_output_device.cc
M media/audio/audio_output_device.h
M media/audio/audio_output_device_unittest.cc
D media/audio/audio_output_stream_sink.cc
D media/audio/audio_output_stream_sink.h
M media/audio/clockless_audio_sink.cc
M media/audio/clockless_audio_sink.h
M media/audio/null_audio_sink.cc
M media/audio/null_audio_sink.h
M media/base/audio_renderer_mixer.cc
M media/base/audio_renderer_mixer.h
M media/base/audio_renderer_mixer_input.cc
M media/base/audio_renderer_mixer_input.h
M media/base/audio_renderer_mixer_input_unittest.cc
M media/base/audio_renderer_mixer_pool.h
M media/base/audio_renderer_mixer_unittest.cc
M media/base/audio_renderer_sink.h
M media/base/fake_audio_renderer_sink.cc
M media/base/fake_audio_renderer_sink.h
M media/base/mock_audio_renderer_sink.cc
M media/base/mock_audio_renderer_sink.h
M media/base/silent_sink_suspender.cc
M media/base/silent_sink_suspender.h
M media/base/silent_sink_suspender_unittest.cc
M media/mojo/services/test_mojo_media_client.cc
M media/mojo/services/test_mojo_media_client.h
M media/renderers/audio_renderer_impl.cc
M media/renderers/audio_renderer_impl.h
M media/renderers/audio_renderer_impl_unittest.cc
M media/test/pipeline_integration_test_base.cc
M media/test/pipeline_integration_test_base.h
M services/audio/public/cpp/output_device_unittest.cc
M third_party/blink/public/platform/media/web_media_player_builder.h
M third_party/blink/public/platform/platform.h
M third_party/blink/public/platform/web_media_player.h
M third_party/blink/public/platform/webaudiosourceprovider_impl.h
M third_party/blink/public/web/modules/media/audio/audio_device_factory.h
M third_party/blink/renderer/core/exported/web_media_player_impl_unittest.cc
M third_party/blink/renderer/core/html/media/html_media_element.cc
M third_party/blink/renderer/core/html/media/html_media_element.h
M third_party/blink/renderer/modules/media/audio/audio_device_factory.cc
M third_party/blink/renderer/modules/media/audio/audio_renderer_mixer_manager.cc
M third_party/blink/renderer/modules/media/audio/audio_renderer_mixer_manager.h
M third_party/blink/renderer/modules/media/audio/audio_renderer_mixer_manager_test.cc
M third_party/blink/renderer/modules/media/audio/audio_renderer_sink_cache.cc
M third_party/blink/renderer/modules/media/audio/audio_renderer_sink_cache.h
M third_party/blink/renderer/modules/media/audio/audio_renderer_sink_cache_test.cc
M third_party/blink/renderer/modules/mediacapturefromelement/html_audio_element_capturer_source.cc
M third_party/blink/renderer/modules/mediacapturefromelement/html_audio_element_capturer_source.h
M third_party/blink/renderer/modules/mediacapturefromelement/html_audio_element_capturer_source_unittest.cc
M third_party/blink/renderer/modules/mediastream/track_audio_renderer.cc
M third_party/blink/renderer/modules/mediastream/track_audio_renderer.h
M third_party/blink/renderer/modules/mediastream/track_audio_renderer_test.cc
M third_party/blink/renderer/modules/peerconnection/webrtc_audio_renderer_test.cc
M third_party/blink/renderer/modules/webrtc/webrtc_audio_renderer.cc
M third_party/blink/renderer/modules/webrtc/webrtc_audio_renderer.h
M third_party/blink/renderer/platform/media/web_media_player_builder.cc
M third_party/blink/renderer/platform/media/web_media_player_impl.cc
M third_party/blink/renderer/platform/media/web_media_player_impl.h
M third_party/blink/renderer/platform/media/webaudiosourceprovider_impl.cc
M third_party/blink/renderer/platform/media/webaudiosourceprovider_impl_test.cc
69 files changed, 889 insertions(+), 1,352 deletions(-)
To view, visit change 4044524. To unsubscribe, or for help writing mail filters, visit settings.
Jordan Bayles (Gerrit)
Attention is currently required from: Dale Curtis, Kentaro Hara, Mark Foltz, Matthew Wolenetz, Ryan Keane.
Jordan Bayles would like Kentaro Hara and Ryan Keane to review this change.
Jordan Bayles (Gerrit)
Attention is currently required from: Dale Curtis, Mark Foltz, Matthew Wolenetz.
1 comment:
Patchset:
Doing this as a code health rotation task.
Please LMK if I need to split this up, all of the changes cascaded from making the change to media/base/aduio_renderer_sink.h.
PTAL.
To view, visit change 4044524. To unsubscribe, or for help writing mail filters, visit settings.
Jordan Bayles (Gerrit)
Attention is currently required from: Dale Curtis, Kentaro Hara, Mark Foltz, Matthew Wolenetz, Ryan Keane.
1 comment:
Patchset:
Also would appreciate any pointers on manual testing to ensure this doesn't introduce any regressions.
To view, visit change 4044524. To unsubscribe, or for help writing mail filters, visit settings.
Jordan Bayles (Gerrit)
Patch set 6:Commit-Queue +1
8 comments:
File media/audio/audio_output_device_unittest.cc:
Patch Set #5, Line 112:
OutputDeviceStatus device_status_;use default member initializer for 'device_status_' (https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-default-member-init.html)
(Lint observed on `android-clang-tidy-rel`, `linux-chromeos-clang-tidy-rel`, and `linux-clang-tidy-rel`)
Please fix.
File media/base/audio_renderer_mixer_input.cc:
Patch Set #5, Line 201:
new_sink->GetOutputDeviceInfoAsync(base::BindOnce('new_sink' used after it was moved (https://clang.llvm.org/extra/clang-tidy/checks/bugprone/use-after-move.html)
(Lint observed on `android-clang-tidy-rel`, `linux-chromeos-clang-tidy-rel`, and `linux-clang-tidy-rel`)
Please fix.
Patch Set #5, Line 201:
new_sink->GetOutputDeviceInfoAsync(base::BindOnce(A `move` operation occurred here, which caused "'new_sink' used after it was moved" at media/base/audio_renderer_mixer_input.cc:201 (https://clang.llvm.org/extra/clang-tidy/checks/bugprone/use-after-move.html)
(Lint observed on `android-clang-tidy-rel`, `linux-chromeos-clang-tidy-rel`, and `linux-clang-tidy-rel`)
Please fix.
File media/base/fake_audio_renderer_sink.cc:
Patch Set #5, Line 43:
: state_(State::kInitialized),member initializer for 'state_' is redundant (https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-default-member-init.html)
(Lint observed on `android-clang-tidy-rel`, `linux-chromeos-clang-tidy-rel`, and `linux-clang-tidy-rel`)
Please fix.
File third_party/blink/renderer/modules/media/audio/audio_renderer_sink_cache.cc:
Patch Set #5, Line 149:
MaybeAddToCache(source_frame_token, sink->GetOutputDeviceInfo().device_id(),'sink' used after it was moved (https://clang.llvm.org/extra/clang-tidy/checks/bugprone/use-after-move.html)
(Lint observed on `android-clang-tidy-rel`, `linux-chromeos-clang-tidy-rel`, and `linux-clang-tidy-rel`)
Please fix.
Patch Set #5, Line 150:
std::move(sink));A `move` operation occurred here, which caused "'sink' used after it was moved" at third_party/blink/renderer/modules/media/audio/audio_renderer_sink_cache.cc:149 (https://clang.llvm.org/extra/clang-tidy/checks/bugprone/use-after-move.html)
(Lint observed on `android-clang-tidy-rel`, `linux-chromeos-clang-tidy-rel`, and `linux-clang-tidy-rel`)
Please fix.
File third_party/blink/renderer/modules/media/audio/audio_renderer_sink_cache_test.cc:
Patch Set #5, Line 67:
cache_->DeleteSink(sink_ptr, /*force_deleted_used=*/true);argument name 'force_deleted_used' in comment does not match parameter name 'force_delete_used' (https://clang.llvm.org/extra/clang-tidy/checks/bugprone/argument-comment.html)
(Lint observed on `android-clang-tidy-rel`, `linux-chromeos-clang-tidy-rel`, and `linux-clang-tidy-rel`)
Please fix.
File third_party/blink/renderer/modules/mediacapturefromelement/html_audio_element_capturer_source.cc:
Patch Set #5, Line 36:
is_started_(false) {member initializer for 'is_started_' is redundant (https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-default-member-init.html)
(Lint observed on `android-clang-tidy-rel`, `linux-chromeos-clang-tidy-rel`, and `linux-clang-tidy-rel`)
Please fix.
To view, visit change 4044524. To unsubscribe, or for help writing mail filters, visit settings.
Olga Sharonova (Gerrit)
Attention is currently required from: Jordan Bayles.
4 comments:
Patchset:
I'll be happy to see a switch from ref counting to unique_ptr, but there is more to it than changing a pointer type. The current approach breaks AudioOutputDevice implementations.
File media/audio/audio_output_device.cc:
Patch Set #6, Line 82:
base::Unretained(this), params, std::ref(callback)));Throughout the whole file: Since AOD is changed to unique pointer, this cannot be posted unretained: AOD is destroyed on another thread and the destructor can race with IO thread operations.
TRACE_EVENT0("audio", "AudioOutputDevice::Stop");
io_task_runner_->PostTask(
FROM_HERE, base::BindOnce(&AudioOutputDevice::ShutDownOnIOThread,
base::Unretained(this)));This made Stop() async and thus broke the contract "no callbacks from the rendering sink will come after Stop() exits".
File third_party/blink/renderer/modules/webrtc/webrtc_audio_renderer.h:
Patch Set #6, Line 364:
media::AudioParameters audio_params_;Could you avoid this rename please? The there are vraious parameters involved in WebRTC audio rendering,
To view, visit change 4044524. To unsubscribe, or for help writing mail filters, visit settings.
Olga Sharonova (Gerrit)
Attention is currently required from: Jordan Bayles.
1 comment:
File third_party/blink/renderer/modules/media/audio/audio_renderer_sink_cache.h:
Patch Set #6, Line 66:
scoped_refptr<media::AudioRendererSink> GetSink(Would be nice to remove unused methods of the cache in a separate CL instead.
To view, visit change 4044524. To unsubscribe, or for help writing mail filters, visit settings.
Ryan Keane (Gerrit)
Attention is currently required from: Jordan Bayles.
Patch set 6:Code-Review +1
3 comments:
Patchset:
cast_streaming LGTM
File content/renderer/media/renderer_webaudiodevice_impl.cc:
Patch Set #6, Line 209:
StartWhy are you changing the initialization flow?
File media/renderers/audio_renderer_impl.cc:
if (sink_)
sink_->Stop();Why were we never hitting a nullref here before? Should this be a DCHECK?
To view, visit change 4044524. To unsubscribe, or for help writing mail filters, visit settings.
Jordan Bayles (Gerrit)
Attention is currently required from: Olga Sharonova, Ryan Keane.
Patch set 7:Commit-Queue +1
6 comments:
Patchset:
I agree I put too many refactors in this patch, I have pulled out refactors not directly relevant to the core change from scoped_refptr to std::unique_ptr and the patch is now 1/3 smaller.
I'll add attentions back when the CQ tests succeed.
File content/renderer/media/renderer_webaudiodevice_impl.cc:
Patch Set #6, Line 209:
StartWhy are you changing the initialization flow?
Good point. I think I should have split that up in a separate patch--initialize is no longer necessary but it's not related to the primary change in this patch.
File media/audio/audio_output_device.cc:
Patch Set #6, Line 82:
base::Unretained(this), params, std::ref(callback)));
Throughout the whole file: Since AOD is changed to unique pointer, this cannot be posted unretained: […]
Done
TRACE_EVENT0("audio", "AudioOutputDevice::Stop");
io_task_runner_->PostTask(
FROM_HERE, base::BindOnce(&AudioOutputDevice::ShutDownOnIOThread,
base::Unretained(this)));
This made Stop() async and thus broke the contract "no callbacks from the rendering sink will come a […]
Done
File media/renderers/audio_renderer_impl.cc:
if (sink_)
sink_->Stop();Why were we never hitting a nullref here before? Should this be a DCHECK?
Done
File third_party/blink/renderer/modules/webrtc/webrtc_audio_renderer.h:
Patch Set #6, Line 364:
media::AudioParameters audio_params_;
Could you avoid this rename please? The there are vraious parameters involved in WebRTC audio render […]
Fair enough. It's confusing in the impl file due to AudioSinkParameters.
To view, visit change 4044524. To unsubscribe, or for help writing mail filters, visit settings.
Jordan Bayles (Gerrit)
Attention is currently required from: Olga Sharonova, Ryan Keane.
1 comment:
File third_party/blink/renderer/modules/media/audio/audio_renderer_sink_cache.h:
Patch Set #6, Line 66:
scoped_refptr<media::AudioRendererSink> GetSink(Would be nice to remove unused methods of the cache in a separate CL instead.
It's not exactly removed, just had to refactor due to no longer used scoped_refptr. I've kept it public to minimize the diff.
To view, visit change 4044524. To unsubscribe, or for help writing mail filters, visit settings.
Olga Sharonova (Gerrit)
Attention is currently required from: Jordan Bayles, Ryan Keane.
4 comments:
File media/audio/audio_output_device.cc:
Patch Set #7, Line 52:
base::BindOnce(&AudioOutputDevice::InitializeOnIOThread, AsWeakPtr(),The weak ptr factory must be destroyed on a thread its weak pointers are accessed on, so we need to think of a way to safely destroy AudioOutputDevice. For example, one way would be to extract "AudioOutputDeviceDelegate" object which would be accessed and destroyed on IO thread, another way - to use a custom deleter; there are likely other options.
File media/audio/audio_output_stream_sink.h:
Patch Set #7, Line 1:
// Copyright 2014 The Chromium AuthorsMove this removal to another CL?
File third_party/blink/renderer/modules/media/audio/audio_renderer_sink_cache.h:
Patch Set #7, Line 66:
scoped_refptr<media::AudioRendererSink> GetSink(If I understand correctly, this method is not used anywhere. Could you remove it in a separate CL?
File third_party/blink/renderer/platform/media/web_media_player_impl.cc:
and waits for it to
// finish.|audio_source_provider_|
It does not wait, just posts and exits.
To view, visit change 4044524. To unsubscribe, or for help writing mail filters, visit settings.
Jordan Bayles (Gerrit)
Attention is currently required from: Olga Sharonova.
Patch set 8:Commit-Queue +1
6 comments:
Patchset:
I'll need to rebase when 4066988 lands.
Waiting for green tests to readd attention sets.
File content/renderer/media/renderer_webaudiodevice_impl_unittest.cc:
Patch Set #7, Line 81:
RendererWebAudioDeviceImplTest() {}use '= default' to define a trivial default constructor (https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-equals-default.html)
(Lint observed on `android-clang-tidy-rel`, `linux-chromeos-clang-tidy-rel`, and `linux-clang-tidy-rel`)
Please fix.
File media/audio/audio_output_device.cc:
Patch Set #7, Line 52:
base::BindOnce(&AudioOutputDevice::InitializeOnIOThread, AsWeakPtr(),
The weak ptr factory must be destroyed on a thread its weak pointers are accessed on, so we need to […]
Done
File media/audio/audio_output_stream_sink.h:
Patch Set #7, Line 1:
// Copyright 2014 The Chromium AuthorsMove this removal to another CL?
File third_party/blink/renderer/modules/media/audio/audio_renderer_sink_cache.h:
Patch Set #7, Line 66:
scoped_refptr<media::AudioRendererSink> GetSink(
If I understand correctly, this method is not used anywhere. […]
Happy to remove it later. It has to be refactored due to using scoped_refptr in this patch however.
File third_party/blink/renderer/platform/media/web_media_player_impl.cc:
and waits for it to
// finish.
|audio_source_provider_| […]
Done
To view, visit change 4044524. To unsubscribe, or for help writing mail filters, visit settings.
Olga Sharonova (Gerrit)
Attention is currently required from: Jordan Bayles.
2 comments:
File media/audio/audio_output_device.h:
Patch Set #8, Line 96:
: public AudioOutputIPCDelegate,Could you flip the naming around? Delegate is usually an internal class, the owner delegates the calls to the delegate on a given thread and takes care of destroying it on that thread.
Here you changed the whole meaning of AudioOutputDevice, which makes the change counterintuitive for everybody familiar with this code. Also, we have AudioInputDevice, so we want the naming to be consistent.
File third_party/blink/renderer/modules/media/audio/audio_renderer_sink_cache.h:
Patch Set #7, Line 66:
scoped_refptr<media::AudioRendererSink> GetSink(
Happy to remove it later. It has to be refactored due to using scoped_refptr in this patch however.
No, there is no point in leaving it around. My idea was to remove it prior, so that this CL would have a smaller scope.
Do you know that you can have CLs dependent one on another in chain (https://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/depot_tools_tutorial.html#_managing_dependent_cls)?
To view, visit change 4044524. To unsubscribe, or for help writing mail filters, visit settings.
Olga Sharonova (Gerrit)
Attention is currently required from: Jordan Bayles.
Jordan Bayles has uploaded this change for review.
[Media] Clean up AudioRendererSink, fix ownership
This patch changes AudioRendererSink to be owned by a std::unique_ptr
instead of a scoped_refptr, clarifying the ownership of this class and
its descendents as well as fixing a variety of bugs and potential bugs
caused by confusing ownership structures.
Of significant note is the changes to the AudioRendererSinkCache, which
now maintains ownership of sinks instead of a complex and confusing ref
counted sharing with callers + garbage collection.
Bug: 151051, 1381431
Change-Id: Icd9bf047810df2441c874263cd28ecb4ad2c839e
---
M chromecast/media/audio/cast_audio_device_factory.cc
M chromecast/media/audio/cast_audio_device_factory.h
M components/cast_streaming/renderer/playback_command_forwarding_renderer_factory_unittest.cc
M content/renderer/media/media_factory.cc
M content/renderer/media/renderer_webaudiodevice_impl.cc
M content/renderer/media/renderer_webaudiodevice_impl.h
M content/renderer/media/renderer_webaudiodevice_impl_unittest.cc
M content/renderer/renderer_blink_platform_impl.cc
M content/renderer/renderer_blink_platform_impl.h
M media/audio/BUILD.gn
M media/audio/audio_output_device.cc
M media/audio/audio_output_device.h
M media/audio/audio_output_device_unittest.cc
D media/audio/audio_output_stream_sink.cc
D media/audio/audio_output_stream_sink.h
M media/audio/clockless_audio_sink.h
M media/audio/null_audio_sink.h
M media/base/audio_renderer_mixer.cc
M media/base/audio_renderer_mixer.h
M media/base/audio_renderer_mixer_input.cc
M media/base/audio_renderer_mixer_input.h
M media/base/audio_renderer_mixer_input_unittest.cc
M media/base/audio_renderer_mixer_pool.h
M media/base/audio_renderer_mixer_unittest.cc
M media/base/audio_renderer_sink.h
M media/base/fake_audio_renderer_sink.h
M media/base/mock_audio_renderer_sink.h
M media/base/silent_sink_suspender.cc
M media/base/silent_sink_suspender.h
M media/base/silent_sink_suspender_unittest.cc
M media/mojo/services/test_mojo_media_client.cc
M media/mojo/services/test_mojo_media_client.h
M media/renderers/audio_renderer_impl.cc
M media/renderers/audio_renderer_impl.h
M media/renderers/audio_renderer_impl_unittest.cc
M media/renderers/renderer_impl_factory.cc
68 files changed, 961 insertions(+), 1,176 deletions(-)
Jordan Bayles (Gerrit)
Attention is currently required from: Olga Sharonova.
Patch set 9:Commit-Queue +1
3 comments:
Patchset:
I'll readd attention sets when I have finished making all the platforms happy. Had to do a significant Fuchsia only refactor unfortunately.
File media/audio/audio_output_device.h:
Patch Set #8, Line 96:
: public AudioOutputIPCDelegate,
Could you flip the naming around? Delegate is usually an internal class, the owner delegates the cal […]
Apologies for the code review update before I was totally ready, I didn't remove you from the attention set and only updated this patch to see what bot failures remain.
I agree the Delegate should be internal, I was planning on looking at other examples of this pattern since I haven't seen it much in my section of the codebase.
File third_party/blink/renderer/modules/media/audio/audio_renderer_sink_cache.h:
Patch Set #7, Line 66:
scoped_refptr<media::AudioRendererSink> GetSink(
No, there is no point in leaving it around. […]
I've completed that now. I have done dependent CLs but wasn't planning on that when I started this refactor. This started as a code health rotation cleanup patch and obviously blew up--I definitely could have been more strategic in the patch scope but was hoping for this to not take longer than a week to finish implementation.
To view, visit change 4044524. To unsubscribe, or for help writing mail filters, visit settings.
Olga Sharonova (Gerrit)
File third_party/blink/renderer/modules/media/audio/audio_renderer_sink_cache.h:
Patch Set #7, Line 66:
scoped_refptr<media::AudioRendererSink> GetSink(
I've completed that now. […]
I'm very grateful you are working on this: have been wanting to do it for so long, but I knew it would not be a quick fix 😄.
To view, visit change 4044524. To unsubscribe, or for help writing mail filters, visit settings.
Jordan Bayles (Gerrit)
Attention is currently required from: Jordan Bayles.
Jordan Bayles uploaded patch set #15 to this change.
[Media] Clean up AudioRendererSink, fix ownership
This patch changes AudioRendererSink to be owned by a std::unique_ptr
instead of a scoped_refptr, clarifying the ownership of this class and
its descendants as well as fixing a variety of bugs and potential bugs
caused by confusing ownership structures.
Bug: 151051, 1381431
Change-Id: Icd9bf047810df2441c874263cd28ecb4ad2c839e
---
M base/threading/thread_restrictions.h
M chromecast/media/audio/cast_audio_device_factory.cc
M chromecast/media/audio/cast_audio_device_factory.h
M chromecast/media/audio/cast_audio_output_device.cc
M components/cast_streaming/renderer/playback_command_forwarding_renderer_factory_unittest.cc
M content/renderer/media/media_factory.cc
M content/renderer/media/renderer_webaudiodevice_impl.cc
M content/renderer/media/renderer_webaudiodevice_impl.h
M content/renderer/media/renderer_webaudiodevice_impl_unittest.cc
M content/renderer/renderer_blink_platform_impl.cc
M content/renderer/renderer_blink_platform_impl.h
M fuchsia_web/webengine/renderer/web_engine_audio_device_factory.cc
M fuchsia_web/webengine/renderer/web_engine_audio_device_factory.h
M fuchsia_web/webengine/renderer/web_engine_audio_output_device.cc
M fuchsia_web/webengine/renderer/web_engine_audio_output_device.h
M fuchsia_web/webengine/renderer/web_engine_audio_output_device_test.cc
M media/audio/audio_output_device.cc
M media/audio/audio_output_device.h
M media/audio/audio_output_device_unittest.cc
M media/audio/clockless_audio_sink.h
M media/audio/null_audio_sink.h
M media/base/audio_renderer_mixer.cc
M media/base/audio_renderer_mixer.h
M media/base/audio_renderer_mixer_input.cc
M media/base/audio_renderer_mixer_input.h
M media/base/audio_renderer_mixer_input_unittest.cc
M media/base/audio_renderer_mixer_pool.h
M media/base/audio_renderer_mixer_unittest.cc
M media/base/audio_renderer_sink.h
M media/base/fake_audio_renderer_sink.h
M media/base/mock_audio_renderer_sink.h
M media/base/output_device_info.cc
M media/base/output_device_info.h
74 files changed, 1,082 insertions(+), 931 deletions(-)
To view, visit change 4044524. To unsubscribe, or for help writing mail filters, visit settings.
Jordan Bayles (Gerrit)
Attention is currently required from: Jordan Bayles.
Jordan Bayles uploaded patch set #14 to this change.
[Media] Clean up AudioRendererSink, fix ownership
This patch changes AudioRendererSink to be owned by a std::unique_ptr
instead of a scoped_refptr, clarifying the ownership of this class and
its descendants as well as fixing a variety of bugs and potential bugs
caused by confusing ownership structures.
Of significant note is the changes to the AudioRendererSinkCache, which
now maintains ownership of sinks instead of a complex and confusing ref
counted sharing with callers + garbage collection.
74 files changed, 1,086 insertions(+), 931 deletions(-)
To view, visit change 4044524. To unsubscribe, or for help writing mail filters, visit settings.
Jordan Bayles (Gerrit)
Attention is currently required from: Jordan Bayles.
Jordan Bayles uploaded patch set #17 to this change.
[Media] Clean up AudioRendererSink, fix ownership
This patch changes AudioRendererSink to be owned by a std::unique_ptr
instead of a scoped_refptr, clarifying the ownership of this class and
its descendents as well as fixing a variety of bugs and potential bugs
caused by confusing ownership structures.
Bug: 151051, 1381431
Change-Id: Icd9bf047810df2441c874263cd28ecb4ad2c839e
---
M base/threading/thread_restrictions.h
M chromecast/media/audio/cast_audio_device_factory.cc
M chromecast/media/audio/cast_audio_device_factory.h
M chromecast/media/audio/cast_audio_output_device.cc
M chromecast/media/audio/cast_audio_output_device.h
75 files changed, 1,145 insertions(+), 995 deletions(-)
To view, visit change 4044524. To unsubscribe, or for help writing mail filters, visit settings.
Matthew Wolenetz (Gerrit)
Attention is currently required from: Jordan Bayles.
1 comment:
Patchset:
Is this a work-in-progress or is it ready for review? It's been sitting in our collective inboxes since before the holidays, when the most recent patch sets had description "in progress refactor". My apologies if it's been awaiting my review in actuality.
To view, visit change 4044524. To unsubscribe, or for help writing mail filters, visit settings.
Jordan Bayles (Gerrit)
Jordan Bayles removed Dale Curtis, Kentaro Hara, Matthew Wolenetz, Olga Sharonova, Ryan Keane and Mark Foltz from this change.
[Media] Clean up AudioRendererSink, fix ownership
This patch changes AudioRendererSink to be owned by a std::unique_ptr
instead of a scoped_refptr, clarifying the ownership of this class and
its descendents as well as fixing a variety of bugs and potential bugs
caused by confusing ownership structures.
Bug: 151051, 1381431
Change-Id: Icd9bf047810df2441c874263cd28ecb4ad2c839e
---
M base/threading/thread_restrictions.h
M chromecast/media/audio/BUILD.gn
M chromecast/media/audio/cast_audio_device_factory.cc
M chromecast/media/audio/cast_audio_device_factory.h
M chromecast/media/audio/cast_audio_output_device.cc
M chromecast/media/audio/cast_audio_output_device.h
M chromecast/media/audio/mixer_service/mock_mixer_socket.h
M chromecast/media/audio/mixer_service/receiver/receiver_creation.cc
M media/base/pipeline_impl.cc
M media/base/silent_sink_suspender.cc
M media/base/silent_sink_suspender.h
M media/base/silent_sink_suspender_unittest.cc
M media/mojo/services/test_mojo_media_client.cc
M media/mojo/services/test_mojo_media_client.h
M media/renderers/audio_renderer_impl.cc
M media/renderers/audio_renderer_impl.h
M media/renderers/audio_renderer_impl_unittest.cc
M media/renderers/renderer_impl.cc
80 files changed, 1,200 insertions(+), 1,035 deletions(-)
Jordan Bayles (Gerrit)
1 comment:
Patchset:
I'll put back up for review when this is ready.
To view, visit change 4044524. To unsubscribe, or for help writing mail filters, visit settings.
Jordan Bayles (Gerrit)
Jordan Bayles abandoned this change.
To view, visit change 4044524. To unsubscribe, or for help writing mail filters, visit settings.
Jordan Bayles (Gerrit)
Attention is currently required from: Jordan Bayles.
Jordan Bayles uploaded patch set #3 to this change.
[Media] Clean up AudioRendererSink, fix ownership
This patch changes AudioRendererSink to be owned by a std::unique_ptr
instead of a scoped_refptr, clarifying the ownership of this class and
its descendents as well as fixing a variety of bugs and potential bugs
caused by confusing ownership structures.
Bug: 151051, 1381431
Change-Id: I8192e7fa4bd2ee8ada23e6cc413fa28a48b5a37e
---
M base/threading/thread_restrictions.h
M chromecast/media/audio/BUILD.gn
M chromecast/media/audio/cast_audio_device_factory.cc
M chromecast/media/audio/cast_audio_device_factory.h
M chromecast/media/audio/cast_audio_output_device.h
M chromecast/media/audio/mixer_service/receiver/receiver_creation.cc
M components/cast_streaming/renderer/control/playback_command_forwarding_renderer_factory_unittest.cc
M content/renderer/media/media_factory.cc
M content/renderer/media/renderer_webaudiodevice_impl.cc
M content/renderer/media/renderer_webaudiodevice_impl.h
M content/renderer/media/renderer_webaudiodevice_impl_unittest.cc
M content/renderer/renderer_blink_platform_impl.cc
M content/renderer/renderer_blink_platform_impl.h
M fuchsia_web/webengine/renderer/web_engine_audio_device_factory.cc
M fuchsia_web/webengine/renderer/web_engine_audio_device_factory.h
M fuchsia_web/webengine/renderer/web_engine_audio_output_device.cc
M fuchsia_web/webengine/renderer/web_engine_audio_output_device.h
M fuchsia_web/webengine/renderer/web_engine_audio_output_device_test.cc
M media/audio/audio_input_unittest.cc
79 files changed, 1,203 insertions(+), 973 deletions(-)
To view, visit change 4904163. To unsubscribe, or for help writing mail filters, visit settings.
Jordan Bayles (Gerrit)
Attention is currently required from: Jordan Bayles.
Jordan Bayles uploaded patch set #4 to this change.
79 files changed, 1,213 insertions(+), 977 deletions(-)
To view, visit change 4904163. To unsubscribe, or for help writing mail filters, visit settings.
Jordan Bayles (Gerrit)
Jordan Bayles abandoned this change![]( "Open in Gerrit")
Related details
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |