Reland "Use non-proxied source object in VideoTrack."
This is a reland of 3eb29c12358930a60134f185cd849e0d12aa9166
This reland doesn't contain the AudioTrack changes (see original
description) that got triggered in some cases and needs to be
addressed separately.
Original change's description:
> Use non-proxied source object in VideoTrack.
>
> Use the internal representation of the video source object from the
> track. Before there were implicit thread hops due to use of the proxy.
>
> Also, override AudioTrack's enabled methods to enforce thread
> expectations.
>
> Bug: webrtc:13540
> Change-Id: I4bc7aca96d6fc24f31ade45e47f52599f1cc2f97
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/250180
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#35911}
Bug: webrtc:13540
Change-Id: I59997be174cc9278e3e5910d493efd5352e6de68
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/250940
Auto-Submit: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35924}
diff --git a/pc/BUILD.gn b/pc/BUILD.gn
index 4a47335..31ac087 100644
--- a/pc/BUILD.gn
+++ b/pc/BUILD.gn
@@ -903,6 +903,7 @@
"../api:sequence_checker",
"../api/video:video_frame",
"../media:rtc_media_base",
+ "../pc:rtc_pc_base",
"../rtc_base",
"../rtc_base:checks",
"../rtc_base:refcount",
diff --git a/pc/media_stream_track_proxy.h b/pc/media_stream_track_proxy.h
index 0278423..ca57c0b 100644
--- a/pc/media_stream_track_proxy.h
+++ b/pc/media_stream_track_proxy.h
@@ -29,12 +29,12 @@
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<AudioProcessorInterface>, GetAudioProcessor)
-PROXY_METHOD1(bool, set_enabled, bool)
PROXY_METHOD1(void, RegisterObserver, ObserverInterface*)
PROXY_METHOD1(void, UnregisterObserver, ObserverInterface*)
END_PROXY_MAP(AudioTrack)
@@ -43,7 +43,7 @@
PROXY_PRIMARY_THREAD_DESTRUCTOR()
BYPASS_PROXY_CONSTMETHOD0(std::string, kind)
BYPASS_PROXY_CONSTMETHOD0(std::string, id)
-PROXY_SECONDARY_CONSTMETHOD0(TrackState, state)
+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 c5ead9e..d1f8fb0 100644
--- a/pc/video_rtp_receiver.cc
+++ b/pc/video_rtp_receiver.cc
@@ -41,11 +41,7 @@
track_(VideoTrackProxyWithInternal<VideoTrack>::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 ad552ea..b96e7d5 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<VideoTrackSourceInterface>> source,
+ rtc::Thread* worker_thread)
: MediaStreamTrack<VideoTrackInterface>(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 @@
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<VideoFrame>* 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 @@
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<VideoTrackInterface>::set_enabled(enable);
}
@@ -102,28 +105,29 @@
}
MediaStreamTrackInterface::TrackState VideoTrack::state() const {
- RTC_DCHECK_RUN_ON(worker_thread_);
+ RTC_DCHECK_RUN_ON(&signaling_thread_);
return MediaStreamTrack<VideoTrackInterface>::state();
}
void VideoTrack::OnChanged() {
RTC_DCHECK_RUN_ON(&signaling_thread_);
- worker_thread_->Invoke<void>(
- 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);
- });
+ auto* source = video_source_->internal();
+ auto state = source->state();
+ rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+ set_state(state == MediaSourceInterface::kEnded ? kEnded : kLive);
}
rtc::scoped_refptr<VideoTrack> VideoTrack::Create(
const std::string& id,
VideoTrackSourceInterface* source,
rtc::Thread* worker_thread) {
- return rtc::make_ref_counted<VideoTrack>(id, source, worker_thread);
+ rtc::scoped_refptr<
+ VideoTrackSourceProxyWithInternal<VideoTrackSourceInterface>>
+ source_proxy = VideoTrackSourceProxy::Create(rtc::Thread::Current(),
+ worker_thread, source);
+
+ return rtc::make_ref_counted<VideoTrack>(id, std::move(source_proxy),
+ worker_thread);
}
} // namespace webrtc
diff --git a/pc/video_track.h b/pc/video_track.h
index 15bfc87..d5480c4 100644
--- a/pc/video_track.h
+++ b/pc/video_track.h
@@ -21,6 +21,7 @@
#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"
@@ -49,9 +50,11 @@
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<VideoTrackSourceInterface>> source,
+ rtc::Thread* worker_thread);
~VideoTrack();
private:
@@ -60,7 +63,10 @@
RTC_NO_UNIQUE_ADDRESS webrtc::SequenceChecker signaling_thread_;
rtc::Thread* const worker_thread_;
- const rtc::scoped_refptr<VideoTrackSourceInterface> video_source_;
+ const rtc::scoped_refptr<
+ VideoTrackSourceProxyWithInternal<VideoTrackSourceInterface>>
+ video_source_;
+
ContentHint content_hint_ RTC_GUARDED_BY(worker_thread_);
};