From 153c9e51074b3004ffeaa6fdb9749e4269b71ba8 Mon Sep 17 00:00:00 2001 From: Sergey Silkin Date: Mon, 21 Feb 2022 11:05:25 +0000 Subject: [PATCH] Revert "Use internal() in VideoTrack when invoking the source." This reverts commit 962bf1896185c0d84232f2bfae492eeb04e1236d. Reason for revert: Regressions in PC tests https://crbug.com/webrtc/13697 Original change's description: > Use internal() in VideoTrack when invoking the source. > > This skips going through the proxy and potentially hide a thread hop > should a regression occur. > > This CL contains a part of a previously reviewed, landed, reverted, > relanded and re-reverted CL: > https://webrtc-review.googlesource.com/c/src/+/250180 > > Bug: webrtc:13540 > Change-Id: If098f5c04a263547fb53f44e9f9738b8e941a294 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/251861 > Reviewed-by: Harald Alvestrand > Commit-Queue: Tomas Gunnarsson > Cr-Commit-Position: refs/heads/main@{#36026} Bug: webrtc:13540 Change-Id: Iea76094aedda91271154f89c356b140c95717976 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/251981 Bot-Commit: rubber-stamper@appspot.gserviceaccount.com Reviewed-by: Harald Alvestrand Commit-Queue: Sergey Silkin Cr-Commit-Position: refs/heads/main@{#36035} --- pc/video_track.cc | 10 +++------- pc/video_track.h | 3 --- pc/video_track_unittest.cc | 11 +---------- 3 files changed, 4 insertions(+), 20 deletions(-) diff --git a/pc/video_track.cc b/pc/video_track.cc index 4559181ce7..27274dc095 100644 --- a/pc/video_track.cc +++ b/pc/video_track.cc @@ -57,18 +57,18 @@ void VideoTrack::AddOrUpdateSink(rtc::VideoSinkInterface* sink, VideoSourceBaseGuarded::AddOrUpdateSink(sink, wants); rtc::VideoSinkWants modified_wants = wants; modified_wants.black_frames = !enabled_w_; - video_source_->internal()->AddOrUpdateSink(sink, modified_wants); + video_source_->AddOrUpdateSink(sink, modified_wants); } void VideoTrack::RemoveSink(rtc::VideoSinkInterface* sink) { RTC_DCHECK_RUN_ON(worker_thread_); VideoSourceBaseGuarded::RemoveSink(sink); - video_source_->internal()->RemoveSink(sink); + video_source_->RemoveSink(sink); } void VideoTrack::RequestRefreshFrame() { RTC_DCHECK_RUN_ON(worker_thread_); - video_source_->internal()->RequestRefreshFrame(); + video_source_->RequestRefreshFrame(); } VideoTrackSourceInterface* VideoTrack::GetSource() const { @@ -76,10 +76,6 @@ VideoTrackSourceInterface* VideoTrack::GetSource() const { return video_source_.get(); } -VideoTrackSourceInterface* VideoTrack::GetSourceInternal() const { - return video_source_->internal(); -} - VideoTrackInterface::ContentHint VideoTrack::content_hint() const { RTC_DCHECK_RUN_ON(&signaling_thread_); return content_hint_; diff --git a/pc/video_track.h b/pc/video_track.h index 66262d22d1..2bf7f31741 100644 --- a/pc/video_track.h +++ b/pc/video_track.h @@ -54,9 +54,6 @@ class VideoTrack : public MediaStreamTrack, MediaStreamTrackInterface::TrackState state() const override; std::string kind() const override; - // Direct access to the non-proxied source object for internal implementation. - VideoTrackSourceInterface* GetSourceInternal() const; - protected: VideoTrack( const std::string& id, diff --git a/pc/video_track_unittest.cc b/pc/video_track_unittest.cc index e6dcce7939..6342b608f1 100644 --- a/pc/video_track_unittest.cc +++ b/pc/video_track_unittest.cc @@ -40,19 +40,10 @@ class VideoTrackTest : public ::testing::Test { protected: rtc::scoped_refptr video_track_source_; - rtc::scoped_refptr video_track_; + rtc::scoped_refptr video_track_; cricket::FakeFrameSource frame_source_; }; -// VideoTrack::Create will create an API proxy around the source object. -// The `GetSource` method provides access to the proxy object intented for API -// use while the GetSourceInternal() provides direct access to the source object -// as provided to the `VideoTrack::Create` factory function. -TEST_F(VideoTrackTest, CheckApiProxyAndInternalSource) { - EXPECT_NE(video_track_->GetSource(), video_track_source_.get()); - EXPECT_EQ(video_track_->GetSourceInternal(), video_track_source_.get()); -} - // Test changing the source state also changes the track state. TEST_F(VideoTrackTest, SourceStateChangeTrackState) { EXPECT_EQ(MediaStreamTrackInterface::kLive, video_track_->state());