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

Side by Side Diff: content/renderer/media/webrtc_audio_renderer.cc

Issue 2856943003: Revert of Remove dangling PlayingState pointers in WebRtcAudioRenderer. (Closed)
Patch Set: Created 3 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « content/renderer/media/webrtc_audio_renderer.h ('k') | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "content/renderer/media/webrtc_audio_renderer.h" 5 #include "content/renderer/media/webrtc_audio_renderer.h"
6 6
7 #include <utility> 7 #include <utility>
8 8
9 #include "base/bind.h" 9 #include "base/bind.h"
10 #include "base/bind_helpers.h" 10 #include "base/bind_helpers.h"
(...skipping 36 matching lines...) Expand 10 before | Expand all | Expand 10 after
47 47
48 // This is a simple wrapper class that's handed out to users of a shared 48 // This is a simple wrapper class that's handed out to users of a shared
49 // WebRtcAudioRenderer instance. This class maintains the per-user 'playing' 49 // WebRtcAudioRenderer instance. This class maintains the per-user 'playing'
50 // and 'started' states to avoid problems related to incorrect usage which 50 // and 'started' states to avoid problems related to incorrect usage which
51 // might violate the implementation assumptions inside WebRtcAudioRenderer 51 // might violate the implementation assumptions inside WebRtcAudioRenderer
52 // (see the play reference count). 52 // (see the play reference count).
53 class SharedAudioRenderer : public MediaStreamAudioRenderer { 53 class SharedAudioRenderer : public MediaStreamAudioRenderer {
54 public: 54 public:
55 // Callback definition for a callback that is called when when Play(), Pause() 55 // Callback definition for a callback that is called when when Play(), Pause()
56 // or SetVolume are called (whenever the internal |playing_state_| changes). 56 // or SetVolume are called (whenever the internal |playing_state_| changes).
57 using OnPlayStateChanged = 57 typedef base::Callback<void(const blink::WebMediaStream&,
58 base::Callback<void(const blink::WebMediaStream&, 58 WebRtcAudioRenderer::PlayingState*)>
59 WebRtcAudioRenderer::PlayingState*)>; 59 OnPlayStateChanged;
60
61 // Signals that the PlayingState* is about to become invalid, see comment in
62 // OnPlayStateRemoved.
63 using OnPlayStateRemoved =
64 base::OnceCallback<void(WebRtcAudioRenderer::PlayingState*)>;
65 60
66 SharedAudioRenderer(const scoped_refptr<MediaStreamAudioRenderer>& delegate, 61 SharedAudioRenderer(const scoped_refptr<MediaStreamAudioRenderer>& delegate,
67 const blink::WebMediaStream& media_stream, 62 const blink::WebMediaStream& media_stream,
68 const OnPlayStateChanged& on_play_state_changed, 63 const OnPlayStateChanged& on_play_state_changed)
69 OnPlayStateRemoved on_play_state_removed)
70 : delegate_(delegate), 64 : delegate_(delegate),
71 media_stream_(media_stream), 65 media_stream_(media_stream),
72 started_(false), 66 started_(false),
73 on_play_state_changed_(on_play_state_changed), 67 on_play_state_changed_(on_play_state_changed) {
74 on_play_state_removed_(std::move(on_play_state_removed)) {
75 DCHECK(!on_play_state_changed_.is_null()); 68 DCHECK(!on_play_state_changed_.is_null());
76 DCHECK(!media_stream_.isNull()); 69 DCHECK(!media_stream_.isNull());
77 } 70 }
78 71
79 protected: 72 protected:
80 ~SharedAudioRenderer() override { 73 ~SharedAudioRenderer() override {
81 DCHECK(thread_checker_.CalledOnValidThread()); 74 DCHECK(thread_checker_.CalledOnValidThread());
82 DVLOG(1) << __func__; 75 DVLOG(1) << __func__;
83 Stop(); 76 Stop();
84 std::move(on_play_state_removed_).Run(&playing_state_);
85 } 77 }
86 78
87 void Start() override { 79 void Start() override {
88 DCHECK(thread_checker_.CalledOnValidThread()); 80 DCHECK(thread_checker_.CalledOnValidThread());
89 if (started_) 81 if (started_)
90 return; 82 return;
91 started_ = true; 83 started_ = true;
92 delegate_->Start(); 84 delegate_->Start();
93 } 85 }
94 86
(...skipping 54 matching lines...) Expand 10 before | Expand all | Expand 10 after
149 return delegate_->IsLocalRenderer(); 141 return delegate_->IsLocalRenderer();
150 } 142 }
151 143
152 private: 144 private:
153 base::ThreadChecker thread_checker_; 145 base::ThreadChecker thread_checker_;
154 const scoped_refptr<MediaStreamAudioRenderer> delegate_; 146 const scoped_refptr<MediaStreamAudioRenderer> delegate_;
155 const blink::WebMediaStream media_stream_; 147 const blink::WebMediaStream media_stream_;
156 bool started_; 148 bool started_;
157 WebRtcAudioRenderer::PlayingState playing_state_; 149 WebRtcAudioRenderer::PlayingState playing_state_;
158 OnPlayStateChanged on_play_state_changed_; 150 OnPlayStateChanged on_play_state_changed_;
159 OnPlayStateRemoved on_play_state_removed_;
160 }; 151 };
161 152
162 } // namespace 153 } // namespace
163 154
164 WebRtcAudioRenderer::WebRtcAudioRenderer( 155 WebRtcAudioRenderer::WebRtcAudioRenderer(
165 const scoped_refptr<base::SingleThreadTaskRunner>& signaling_thread, 156 const scoped_refptr<base::SingleThreadTaskRunner>& signaling_thread,
166 const blink::WebMediaStream& media_stream, 157 const blink::WebMediaStream& media_stream,
167 int source_render_frame_id, 158 int source_render_frame_id,
168 int session_id, 159 int session_id,
169 const std::string& device_id, 160 const std::string& device_id,
(...skipping 52 matching lines...) Expand 10 before | Expand all | Expand 10 after
222 } 213 }
223 sink_->Start(); 214 sink_->Start();
224 sink_->Play(); // Not all the sinks play on start. 215 sink_->Play(); // Not all the sinks play on start.
225 216
226 return true; 217 return true;
227 } 218 }
228 219
229 scoped_refptr<MediaStreamAudioRenderer> 220 scoped_refptr<MediaStreamAudioRenderer>
230 WebRtcAudioRenderer::CreateSharedAudioRendererProxy( 221 WebRtcAudioRenderer::CreateSharedAudioRendererProxy(
231 const blink::WebMediaStream& media_stream) { 222 const blink::WebMediaStream& media_stream) {
232 SharedAudioRenderer::OnPlayStateChanged on_play_state_changed = 223 content::SharedAudioRenderer::OnPlayStateChanged on_play_state_changed =
233 base::Bind(&WebRtcAudioRenderer::OnPlayStateChanged, this); 224 base::Bind(&WebRtcAudioRenderer::OnPlayStateChanged, this);
234 SharedAudioRenderer::OnPlayStateRemoved on_play_state_removed = 225 return new SharedAudioRenderer(this, media_stream, on_play_state_changed);
235 base::BindOnce(&WebRtcAudioRenderer::OnPlayStateRemoved, this);
236 return new SharedAudioRenderer(this, media_stream, on_play_state_changed,
237 std::move(on_play_state_removed));
238 } 226 }
239 227
240 bool WebRtcAudioRenderer::IsStarted() const { 228 bool WebRtcAudioRenderer::IsStarted() const {
241 DCHECK(thread_checker_.CalledOnValidThread()); 229 DCHECK(thread_checker_.CalledOnValidThread());
242 return start_ref_count_ != 0; 230 return start_ref_count_ != 0;
243 } 231 }
244 232
245 bool WebRtcAudioRenderer::CurrentThreadIsRenderingThread() { 233 bool WebRtcAudioRenderer::CurrentThreadIsRenderingThread() {
246 return sink_->CurrentThreadIsRenderingThread(); 234 return sink_->CurrentThreadIsRenderingThread();
247 } 235 }
(...skipping 350 matching lines...) Expand 10 before | Expand all | Expand 10 after
598 if (!state->playing()) { 586 if (!state->playing()) {
599 if (RemovePlayingState(source, state)) 587 if (RemovePlayingState(source, state))
600 EnterPauseState(); 588 EnterPauseState();
601 } else if (AddPlayingState(source, state)) { 589 } else if (AddPlayingState(source, state)) {
602 EnterPlayState(); 590 EnterPlayState();
603 } 591 }
604 UpdateSourceVolume(source); 592 UpdateSourceVolume(source);
605 } 593 }
606 } 594 }
607 595
608 void WebRtcAudioRenderer::OnPlayStateRemoved(PlayingState* state) {
609 // It is possible we associated |state| to a source for a track that is no
610 // longer easily reachable. We iterate over |source_playing_states_| to
611 // ensure there are no dangling pointers to |state| there. See
612 // crbug.com/697256.
613 // TODO(maxmorin): Clean up cleanup code in this and related classes so that
614 // this hack isn't necessary.
615 DCHECK(thread_checker_.CalledOnValidThread());
616 for (auto it = source_playing_states_.begin();
617 it != source_playing_states_.end();) {
618 PlayingStates& states = it->second;
619 // We cannot use RemovePlayingState as it might invalidate |it|.
620 states.erase(std::remove(states.begin(), states.end(), state),
621 states.end());
622 if (states.empty())
623 it = source_playing_states_.erase(it);
624 else
625 ++it;
626 }
627 }
628
629 void WebRtcAudioRenderer::PrepareSink() { 596 void WebRtcAudioRenderer::PrepareSink() {
630 DCHECK(thread_checker_.CalledOnValidThread()); 597 DCHECK(thread_checker_.CalledOnValidThread());
631 media::AudioParameters new_sink_params; 598 media::AudioParameters new_sink_params;
632 { 599 {
633 base::AutoLock lock(lock_); 600 base::AutoLock lock(lock_);
634 new_sink_params = sink_params_; 601 new_sink_params = sink_params_;
635 } 602 }
636 603
637 const media::OutputDeviceInfo& device_info = sink_->GetOutputDeviceInfo(); 604 const media::OutputDeviceInfo& device_info = sink_->GetOutputDeviceInfo();
638 DCHECK_EQ(device_info.device_status(), media::OUTPUT_DEVICE_STATUS_OK); 605 DCHECK_EQ(device_info.device_status(), media::OUTPUT_DEVICE_STATUS_OK);
(...skipping 52 matching lines...) Expand 10 before | Expand all | Expand 10 after
691 sink_params_ = new_sink_params; 658 sink_params_ = new_sink_params;
692 } 659 }
693 660
694 // Specify the latency info to be passed to the browser side. 661 // Specify the latency info to be passed to the browser side.
695 new_sink_params.set_latency_tag(AudioDeviceFactory::GetSourceLatencyType( 662 new_sink_params.set_latency_tag(AudioDeviceFactory::GetSourceLatencyType(
696 AudioDeviceFactory::AudioDeviceFactory::kSourceWebRtc)); 663 AudioDeviceFactory::AudioDeviceFactory::kSourceWebRtc));
697 sink_->Initialize(new_sink_params, this); 664 sink_->Initialize(new_sink_params, this);
698 } 665 }
699 666
700 } // namespace content 667 } // namespace content
OLDNEW
« no previous file with comments | « content/renderer/media/webrtc_audio_renderer.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698