diff --git a/pc/BUILD.gn b/pc/BUILD.gn index ba0586755e..59459ec0af 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -1242,6 +1242,7 @@ rtc_library("video_track") { "video_track.h", ] deps = [ + ":rtc_pc_base", "../api:media_stream_interface", "../api:scoped_refptr", "../api:sequence_checker", diff --git a/pc/audio_track.h b/pc/audio_track.h index 8a705cf8fb..8e05f546cc 100644 --- a/pc/audio_track.h +++ b/pc/audio_track.h @@ -20,6 +20,10 @@ namespace webrtc { +// TODO(tommi): Instead of inheriting from `MediaStreamTrack<>`, implement the +// properties directly in this class. `MediaStreamTrack` doesn't guard against +// conflicting access, so we'd need to override those methods anyway in this +// class in order to make sure things are correctly checked. class AudioTrack : public MediaStreamTrack, public ObserverInterface { protected: diff --git a/pc/media_stream_track_proxy.h b/pc/media_stream_track_proxy.h index 02784233fd..004bcdbfb8 100644 --- a/pc/media_stream_track_proxy.h +++ b/pc/media_stream_track_proxy.h @@ -29,12 +29,12 @@ BYPASS_PROXY_CONSTMETHOD0(std::string, kind) BYPASS_PROXY_CONSTMETHOD0(std::string, id) PROXY_CONSTMETHOD0(TrackState, state) PROXY_CONSTMETHOD0(bool, enabled) +PROXY_METHOD1(bool, set_enabled, bool) BYPASS_PROXY_CONSTMETHOD0(AudioSourceInterface*, GetSource) PROXY_METHOD1(void, AddSink, AudioTrackSinkInterface*) PROXY_METHOD1(void, RemoveSink, AudioTrackSinkInterface*) PROXY_METHOD1(bool, GetSignalLevel, int*) PROXY_METHOD0(rtc::scoped_refptr, GetAudioProcessor) -PROXY_METHOD1(bool, set_enabled, bool) PROXY_METHOD1(void, RegisterObserver, ObserverInterface*) PROXY_METHOD1(void, UnregisterObserver, ObserverInterface*) END_PROXY_MAP(AudioTrack) @@ -43,7 +43,9 @@ BEGIN_PROXY_MAP(VideoTrack) PROXY_PRIMARY_THREAD_DESTRUCTOR() BYPASS_PROXY_CONSTMETHOD0(std::string, kind) BYPASS_PROXY_CONSTMETHOD0(std::string, id) -PROXY_SECONDARY_CONSTMETHOD0(TrackState, state) +// TODO(bugs.webrtc.org/13540): This should probably be PROXY_CONSTMETHOD0, but +// leaving as is due to downstream incompatibility. +BYPASS_PROXY_CONSTMETHOD0(TrackState, state) PROXY_SECONDARY_CONSTMETHOD0(bool, enabled) PROXY_SECONDARY_METHOD1(bool, set_enabled, bool) PROXY_SECONDARY_CONSTMETHOD0(ContentHint, content_hint) diff --git a/pc/video_rtp_receiver.cc b/pc/video_rtp_receiver.cc index c5ead9e773..d1f8fb0d96 100644 --- a/pc/video_rtp_receiver.cc +++ b/pc/video_rtp_receiver.cc @@ -41,11 +41,7 @@ VideoRtpReceiver::VideoRtpReceiver( track_(VideoTrackProxyWithInternal::Create( rtc::Thread::Current(), worker_thread, - VideoTrack::Create(receiver_id, - CreateVideoTrackSourceProxy(rtc::Thread::Current(), - worker_thread, - source_), - worker_thread))), + VideoTrack::Create(receiver_id, source_, worker_thread))), attachment_id_(GenerateUniqueId()) { RTC_DCHECK(worker_thread_); SetStreams(streams); diff --git a/pc/video_track.cc b/pc/video_track.cc index ad552ea1c9..f0da930508 100644 --- a/pc/video_track.cc +++ b/pc/video_track.cc @@ -22,12 +22,14 @@ namespace webrtc { -VideoTrack::VideoTrack(const std::string& label, - VideoTrackSourceInterface* video_source, - rtc::Thread* worker_thread) +VideoTrack::VideoTrack( + const std::string& label, + rtc::scoped_refptr< + VideoTrackSourceProxyWithInternal> source, + rtc::Thread* worker_thread) : MediaStreamTrack(label), worker_thread_(worker_thread), - video_source_(video_source), + video_source_(std::move(source)), content_hint_(ContentHint::kNone) { RTC_DCHECK_RUN_ON(&signaling_thread_); // Detach the thread checker for VideoSourceBaseGuarded since we'll make calls @@ -54,18 +56,18 @@ void VideoTrack::AddOrUpdateSink(rtc::VideoSinkInterface* sink, VideoSourceBaseGuarded::AddOrUpdateSink(sink, wants); rtc::VideoSinkWants modified_wants = wants; modified_wants.black_frames = !enabled(); - video_source_->AddOrUpdateSink(sink, modified_wants); + video_source_->internal()->AddOrUpdateSink(sink, modified_wants); } void VideoTrack::RemoveSink(rtc::VideoSinkInterface* sink) { RTC_DCHECK_RUN_ON(worker_thread_); VideoSourceBaseGuarded::RemoveSink(sink); - video_source_->RemoveSink(sink); + video_source_->internal()->RemoveSink(sink); } void VideoTrack::RequestRefreshFrame() { RTC_DCHECK_RUN_ON(worker_thread_); - video_source_->RequestRefreshFrame(); + video_source_->internal()->RequestRefreshFrame(); } VideoTrackSourceInterface* VideoTrack::GetSource() const { @@ -88,10 +90,11 @@ void VideoTrack::set_content_hint(ContentHint hint) { bool VideoTrack::set_enabled(bool enable) { RTC_DCHECK_RUN_ON(worker_thread_); + auto* source = video_source_->internal(); for (auto& sink_pair : sink_pairs()) { rtc::VideoSinkWants modified_wants = sink_pair.wants; modified_wants.black_frames = !enable; - video_source_->AddOrUpdateSink(sink_pair.sink, modified_wants); + source->AddOrUpdateSink(sink_pair.sink, modified_wants); } return MediaStreamTrack::set_enabled(enable); } @@ -102,28 +105,30 @@ bool VideoTrack::enabled() const { } MediaStreamTrackInterface::TrackState VideoTrack::state() const { - RTC_DCHECK_RUN_ON(worker_thread_); + // TODO(bugs.webrtc.org/13540): Re-enable this DCHECK and update the proxy. + // RTC_DCHECK_RUN_ON(&signaling_thread_); return MediaStreamTrack::state(); } void VideoTrack::OnChanged() { RTC_DCHECK_RUN_ON(&signaling_thread_); - worker_thread_->Invoke( - RTC_FROM_HERE, [this, state = video_source_->state()]() { - // TODO(tommi): Calling set_state() this way isn't ideal since we're - // currently blocking the signaling thread and set_state() may - // internally fire notifications via `FireOnChanged()` which may further - // amplify the blocking effect on the signaling thread. - rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls; - set_state(state == MediaSourceInterface::kEnded ? kEnded : kLive); - }); + rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls; + auto* source = video_source_->internal(); + auto state = source->state(); + set_state(state == MediaSourceInterface::kEnded ? kEnded : kLive); } rtc::scoped_refptr VideoTrack::Create( const std::string& id, VideoTrackSourceInterface* source, rtc::Thread* worker_thread) { - return rtc::make_ref_counted(id, source, worker_thread); + rtc::scoped_refptr< + VideoTrackSourceProxyWithInternal> + source_proxy = VideoTrackSourceProxy::Create(rtc::Thread::Current(), + worker_thread, source); + + return rtc::make_ref_counted(id, std::move(source_proxy), + worker_thread); } } // namespace webrtc diff --git a/pc/video_track.h b/pc/video_track.h index 15bfc878f5..772f52a5c2 100644 --- a/pc/video_track.h +++ b/pc/video_track.h @@ -21,11 +21,16 @@ #include "api/video/video_sink_interface.h" #include "api/video/video_source_interface.h" #include "media/base/video_source_base.h" +#include "pc/video_track_source_proxy.h" #include "rtc_base/thread.h" #include "rtc_base/thread_annotations.h" namespace webrtc { +// TODO(tommi): Instead of inheriting from `MediaStreamTrack<>`, implement the +// properties directly in this class. `MediaStreamTrack` doesn't guard against +// conflicting access, so we'd need to override those methods anyway in this +// class in order to make sure things are correctly checked. class VideoTrack : public MediaStreamTrack, public rtc::VideoSourceBaseGuarded, public ObserverInterface { @@ -49,9 +54,11 @@ class VideoTrack : public MediaStreamTrack, std::string kind() const override; protected: - VideoTrack(const std::string& id, - VideoTrackSourceInterface* video_source, - rtc::Thread* worker_thread); + VideoTrack( + const std::string& id, + rtc::scoped_refptr< + VideoTrackSourceProxyWithInternal> source, + rtc::Thread* worker_thread); ~VideoTrack(); private: @@ -60,7 +67,10 @@ class VideoTrack : public MediaStreamTrack, RTC_NO_UNIQUE_ADDRESS webrtc::SequenceChecker signaling_thread_; rtc::Thread* const worker_thread_; - const rtc::scoped_refptr video_source_; + const rtc::scoped_refptr< + VideoTrackSourceProxyWithInternal> + video_source_; + ContentHint content_hint_ RTC_GUARDED_BY(worker_thread_); };