From dfd69c22100e1d2e83295f33d06fa9916caa5c53 Mon Sep 17 00:00:00 2001
From: Tomas Gunnarsson <tommi@webrtc.org>
Date: Tue, 15 Feb 2022 13:56:50 +0100
Subject: [PATCH] Move VideoTrack's content_hint property to the signaling
 thread.

Bug: webrtc:13673, webrtc:13681
Change-Id: I06810338bf5e44665e4d005d35636e9a98b1bd0b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/251684
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36010}
---
 pc/media_stream_track_proxy.h |  4 ++--
 pc/rtp_sender.cc              |  8 ++++++--
 pc/video_track.cc             | 17 ++++++-----------
 pc/video_track.h              |  2 +-
 4 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/pc/media_stream_track_proxy.h b/pc/media_stream_track_proxy.h
index 0c13f83df8..2af3aedb22 100644
--- a/pc/media_stream_track_proxy.h
+++ b/pc/media_stream_track_proxy.h
@@ -46,8 +46,8 @@ BYPASS_PROXY_CONSTMETHOD0(std::string, id)
 PROXY_SECONDARY_CONSTMETHOD0(TrackState, state)
 PROXY_CONSTMETHOD0(bool, enabled)
 PROXY_METHOD1(bool, set_enabled, bool)
-PROXY_SECONDARY_CONSTMETHOD0(ContentHint, content_hint)
-PROXY_SECONDARY_METHOD1(void, set_content_hint, ContentHint)
+PROXY_CONSTMETHOD0(ContentHint, content_hint)
+PROXY_METHOD1(void, set_content_hint, ContentHint)
 PROXY_SECONDARY_METHOD2(void,
                         AddOrUpdateSink,
                         rtc::VideoSinkInterface<VideoFrame>*,
diff --git a/pc/rtp_sender.cc b/pc/rtp_sender.cc
index d4286371be..957f514205 100644
--- a/pc/rtp_sender.cc
+++ b/pc/rtp_sender.cc
@@ -483,6 +483,7 @@ sigslot::signal0<>* AudioRtpSender::GetOnDestroyedSignal() {
 }
 
 void AudioRtpSender::OnChanged() {
+  // Running on the signaling thread.
   TRACE_EVENT0("webrtc", "AudioRtpSender::OnChanged");
   RTC_DCHECK(!stopped_);
   if (cached_track_enabled_ != track_->enabled()) {
@@ -584,10 +585,13 @@ VideoRtpSender::~VideoRtpSender() {
 }
 
 void VideoRtpSender::OnChanged() {
+  // Running on the signaling thread.
   TRACE_EVENT0("webrtc", "VideoRtpSender::OnChanged");
   RTC_DCHECK(!stopped_);
-  if (cached_track_content_hint_ != video_track()->content_hint()) {
-    cached_track_content_hint_ = video_track()->content_hint();
+
+  auto content_hint = video_track()->content_hint();
+  if (cached_track_content_hint_ != content_hint) {
+    cached_track_content_hint_ = content_hint;
     if (can_send_track()) {
       SetSend();
     }
diff --git a/pc/video_track.cc b/pc/video_track.cc
index 57af724511..6e749b0526 100644
--- a/pc/video_track.cc
+++ b/pc/video_track.cc
@@ -18,6 +18,7 @@
 #include "api/sequence_checker.h"
 #include "rtc_base/checks.h"
 #include "rtc_base/location.h"
+#include "rtc_base/logging.h"
 #include "rtc_base/ref_counted_object.h"
 
 namespace webrtc {
@@ -74,12 +75,12 @@ VideoTrackSourceInterface* VideoTrack::GetSource() const {
 }
 
 VideoTrackInterface::ContentHint VideoTrack::content_hint() const {
-  RTC_DCHECK_RUN_ON(worker_thread_);
+  RTC_DCHECK_RUN_ON(&signaling_thread_);
   return content_hint_;
 }
 
 void VideoTrack::set_content_hint(ContentHint hint) {
-  RTC_DCHECK_RUN_ON(worker_thread_);
+  RTC_DCHECK_RUN_ON(&signaling_thread_);
   if (content_hint_ == hint)
     return;
   content_hint_ = hint;
@@ -120,15 +121,9 @@ MediaStreamTrackInterface::TrackState VideoTrack::state() const {
 
 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);
-      });
+  rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+  MediaSourceInterface::SourceState state = video_source_->state();
+  set_state(state == MediaSourceInterface::kEnded ? kEnded : kLive);
 }
 
 rtc::scoped_refptr<VideoTrack> VideoTrack::Create(
diff --git a/pc/video_track.h b/pc/video_track.h
index a23f7a02ba..4328f188af 100644
--- a/pc/video_track.h
+++ b/pc/video_track.h
@@ -61,7 +61,7 @@ class VideoTrack : public MediaStreamTrack<VideoTrackInterface>,
   RTC_NO_UNIQUE_ADDRESS webrtc::SequenceChecker signaling_thread_;
   rtc::Thread* const worker_thread_;
   const rtc::scoped_refptr<VideoTrackSourceInterface> video_source_;
-  ContentHint content_hint_ RTC_GUARDED_BY(worker_thread_);
+  ContentHint content_hint_ RTC_GUARDED_BY(&signaling_thread_);
   // Cached `enabled` state for the worker thread. This is kept in sync with
   // the state maintained on the signaling thread via set_enabled() but can
   // be queried without blocking on the worker thread by callers that don't