From 5214a0ab8e2b622fa7aff888a740e46e80f7ed18 Mon Sep 17 00:00:00 2001 From: pbos Date: Fri, 16 Dec 2016 15:39:11 -0800 Subject: [PATCH] Add support for content hints to VideoTrack. Permits overriding the source-default is_screencast option to be able to treat screencast sources as fluid video, preserving motion at the loss of individual frame quality (or vice versa). BUG=chromium:653531 R=deadbeef@webrtc.org Review-Url: https://codereview.webrtc.org/2579993003 Cr-Commit-Position: refs/heads/master@{#15659} --- webrtc/api/mediastreaminterface.h | 12 ++- webrtc/api/mediastreamtrackproxy.h | 2 + webrtc/api/rtpsender.cc | 21 ++++- webrtc/api/rtpsender.h | 2 + webrtc/api/rtpsenderreceiver_unittest.cc | 103 ++++++++++++++++++++++- webrtc/api/test/fakevideotracksource.h | 15 +++- webrtc/api/videotrack.cc | 16 +++- webrtc/api/videotrack.h | 3 + 8 files changed, 160 insertions(+), 14 deletions(-) diff --git a/webrtc/api/mediastreaminterface.h b/webrtc/api/mediastreaminterface.h index baebad7b69..eb51633e2b 100644 --- a/webrtc/api/mediastreaminterface.h +++ b/webrtc/api/mediastreaminterface.h @@ -133,13 +133,21 @@ class VideoTrackInterface : public MediaStreamTrackInterface, public rtc::VideoSourceInterface { public: + // Video track content hint, used to override the source is_screencast + // property. + // See https://crbug.com/653531 and https://github.com/WICG/mst-content-hint. + enum class ContentHint { kNone, kFluid, kDetailed }; + // Register a video sink for this track. void AddOrUpdateSink(rtc::VideoSinkInterface* sink, - const rtc::VideoSinkWants& wants) override{}; - void RemoveSink(rtc::VideoSinkInterface* sink) override{}; + const rtc::VideoSinkWants& wants) override {} + void RemoveSink(rtc::VideoSinkInterface* sink) override {} virtual VideoTrackSourceInterface* GetSource() const = 0; + virtual ContentHint content_hint() const { return ContentHint::kNone; } + virtual void set_content_hint(ContentHint hint) {} + protected: virtual ~VideoTrackInterface() {} }; diff --git a/webrtc/api/mediastreamtrackproxy.h b/webrtc/api/mediastreamtrackproxy.h index 206a467c80..fc73536aff 100644 --- a/webrtc/api/mediastreamtrackproxy.h +++ b/webrtc/api/mediastreamtrackproxy.h @@ -41,6 +41,8 @@ BEGIN_PROXY_MAP(VideoTrack) PROXY_CONSTMETHOD0(TrackState, state) PROXY_CONSTMETHOD0(bool, enabled) PROXY_METHOD1(bool, set_enabled, bool) + PROXY_CONSTMETHOD0(ContentHint, content_hint) + PROXY_METHOD1(void, set_content_hint, ContentHint) PROXY_WORKER_METHOD2(void, AddOrUpdateSink, rtc::VideoSinkInterface*, diff --git a/webrtc/api/rtpsender.cc b/webrtc/api/rtpsender.cc index 2e01d961d4..61f53b4d80 100644 --- a/webrtc/api/rtpsender.cc +++ b/webrtc/api/rtpsender.cc @@ -243,7 +243,8 @@ VideoRtpSender::VideoRtpSender(VideoTrackInterface* track, stream_id_(stream_id), channel_(channel), track_(track), - cached_track_enabled_(track->enabled()) { + cached_track_enabled_(track->enabled()), + cached_track_content_hint_(track->content_hint()) { track_->RegisterObserver(this); } @@ -253,7 +254,8 @@ VideoRtpSender::VideoRtpSender(VideoTrackInterface* track, stream_id_(rtc::CreateRandomUuid()), channel_(channel), track_(track), - cached_track_enabled_(track->enabled()) { + cached_track_enabled_(track->enabled()), + cached_track_content_hint_(track->content_hint()) { track_->RegisterObserver(this); } @@ -269,8 +271,10 @@ VideoRtpSender::~VideoRtpSender() { void VideoRtpSender::OnChanged() { TRACE_EVENT0("webrtc", "VideoRtpSender::OnChanged"); RTC_DCHECK(!stopped_); - if (cached_track_enabled_ != track_->enabled()) { + if (cached_track_enabled_ != track_->enabled() || + cached_track_content_hint_ != track_->content_hint()) { cached_track_enabled_ = track_->enabled(); + cached_track_content_hint_ = track_->content_hint(); if (can_send_track()) { SetVideoSend(); } @@ -303,6 +307,7 @@ bool VideoRtpSender::SetTrack(MediaStreamTrackInterface* track) { track_ = video_track; if (track_) { cached_track_enabled_ = track_->enabled(); + cached_track_content_hint_ = track_->content_hint(); track_->RegisterObserver(this); } @@ -372,6 +377,16 @@ void VideoRtpSender::SetVideoSend() { options.is_screencast = rtc::Optional(source->is_screencast()); options.video_noise_reduction = source->needs_denoising(); } + switch (cached_track_content_hint_) { + case VideoTrackInterface::ContentHint::kNone: + break; + case VideoTrackInterface::ContentHint::kFluid: + options.is_screencast = rtc::Optional(false); + break; + case VideoTrackInterface::ContentHint::kDetailed: + options.is_screencast = rtc::Optional(true); + break; + } if (!channel_->SetVideoSend(ssrc_, track_->enabled(), &options, track_)) { RTC_DCHECK(false); } diff --git a/webrtc/api/rtpsender.h b/webrtc/api/rtpsender.h index 067ae5e5b8..6c0592148b 100644 --- a/webrtc/api/rtpsender.h +++ b/webrtc/api/rtpsender.h @@ -225,6 +225,8 @@ class VideoRtpSender : public ObserverInterface, rtc::scoped_refptr track_; uint32_t ssrc_ = 0; bool cached_track_enabled_ = false; + VideoTrackInterface::ContentHint cached_track_content_hint_ = + VideoTrackInterface::ContentHint::kNone; bool stopped_ = false; }; diff --git a/webrtc/api/rtpsenderreceiver_unittest.cc b/webrtc/api/rtpsenderreceiver_unittest.cc index f4f550e906..a79b2976b6 100644 --- a/webrtc/api/rtpsenderreceiver_unittest.cc +++ b/webrtc/api/rtpsenderreceiver_unittest.cc @@ -101,9 +101,11 @@ class RtpSenderReceiverTest : public testing::Test { void TearDown() override { channel_manager_.Terminate(); } - void AddVideoTrack() { + void AddVideoTrack() { AddVideoTrack(false); } + + void AddVideoTrack(bool is_screencast) { rtc::scoped_refptr source( - FakeVideoTrackSource::Create()); + FakeVideoTrackSource::Create(is_screencast)); video_track_ = VideoTrack::Create(kVideoTrackId, source); EXPECT_TRUE(stream_->AddTrack(video_track_)); } @@ -120,8 +122,10 @@ class RtpSenderReceiverTest : public testing::Test { VerifyVoiceChannelInput(); } - void CreateVideoRtpSender() { - AddVideoTrack(); + void CreateVideoRtpSender() { CreateVideoRtpSender(false); } + + void CreateVideoRtpSender(bool is_screencast) { + AddVideoTrack(is_screencast); video_rtp_sender_ = new VideoRtpSender(stream_->GetVideoTracks()[0], stream_->label(), video_channel_); video_rtp_sender_->SetSsrc(kVideoSsrc); @@ -621,4 +625,95 @@ TEST_F(RtpSenderReceiverTest, VideoReceiverCanSetParameters) { DestroyVideoRtpReceiver(); } +// Test that makes sure that a video track content hint translates to the proper +// value for sources that are not screencast. +TEST_F(RtpSenderReceiverTest, PropagatesVideoTrackContentHint) { + CreateVideoRtpSender(); + + video_track_->set_enabled(true); + + // |video_track_| is not screencast by default. + EXPECT_EQ(rtc::Optional(false), + video_media_channel_->options().is_screencast); + // No content hint should be set by default. + EXPECT_EQ(VideoTrackInterface::ContentHint::kNone, + video_track_->content_hint()); + // Setting detailed should turn a non-screencast source into screencast mode. + video_track_->set_content_hint(VideoTrackInterface::ContentHint::kDetailed); + EXPECT_EQ(rtc::Optional(true), + video_media_channel_->options().is_screencast); + // Removing the content hint should turn the track back into non-screencast + // mode. + video_track_->set_content_hint(VideoTrackInterface::ContentHint::kNone); + EXPECT_EQ(rtc::Optional(false), + video_media_channel_->options().is_screencast); + // Setting fluid should remain in non-screencast mode (its default). + video_track_->set_content_hint(VideoTrackInterface::ContentHint::kFluid); + EXPECT_EQ(rtc::Optional(false), + video_media_channel_->options().is_screencast); + + DestroyVideoRtpSender(); +} + +// Test that makes sure that a video track content hint translates to the proper +// value for screencast sources. +TEST_F(RtpSenderReceiverTest, + PropagatesVideoTrackContentHintForScreencastSource) { + CreateVideoRtpSender(true); + + video_track_->set_enabled(true); + + // |video_track_| with a screencast source should be screencast by default. + EXPECT_EQ(rtc::Optional(true), + video_media_channel_->options().is_screencast); + // No content hint should be set by default. + EXPECT_EQ(VideoTrackInterface::ContentHint::kNone, + video_track_->content_hint()); + // Setting fluid should turn a screencast source into non-screencast mode. + video_track_->set_content_hint(VideoTrackInterface::ContentHint::kFluid); + EXPECT_EQ(rtc::Optional(false), + video_media_channel_->options().is_screencast); + // Removing the content hint should turn the track back into screencast mode. + video_track_->set_content_hint(VideoTrackInterface::ContentHint::kNone); + EXPECT_EQ(rtc::Optional(true), + video_media_channel_->options().is_screencast); + // Setting detailed should still remain in screencast mode (its default). + video_track_->set_content_hint(VideoTrackInterface::ContentHint::kDetailed); + EXPECT_EQ(rtc::Optional(true), + video_media_channel_->options().is_screencast); + + DestroyVideoRtpSender(); +} + +// Test that makes sure any content hints that are set on a track before +// VideoRtpSender is ready to send are still applied when it gets ready to send. +TEST_F(RtpSenderReceiverTest, + PropagatesVideoTrackContentHintSetBeforeEnabling) { + AddVideoTrack(); + // Setting detailed overrides the default non-screencast mode. This should be + // applied even if the track is set on construction. + video_track_->set_content_hint(VideoTrackInterface::ContentHint::kDetailed); + video_rtp_sender_ = new VideoRtpSender(stream_->GetVideoTracks()[0], + stream_->label(), video_channel_); + video_track_->set_enabled(true); + + // Sender is not ready to send (no SSRC) so no option should have been set. + EXPECT_EQ(rtc::Optional(), + video_media_channel_->options().is_screencast); + + // Verify that the content hint is accounted for when video_rtp_sender_ does + // get enabled. + video_rtp_sender_->SetSsrc(kVideoSsrc); + EXPECT_EQ(rtc::Optional(true), + video_media_channel_->options().is_screencast); + + // And removing the hint should go back to false (to verify that false was + // default correctly). + video_track_->set_content_hint(VideoTrackInterface::ContentHint::kNone); + EXPECT_EQ(rtc::Optional(false), + video_media_channel_->options().is_screencast); + + DestroyVideoRtpSender(); +} + } // namespace webrtc diff --git a/webrtc/api/test/fakevideotracksource.h b/webrtc/api/test/fakevideotracksource.h index 1cb264b736..0638a1f16c 100644 --- a/webrtc/api/test/fakevideotracksource.h +++ b/webrtc/api/test/fakevideotracksource.h @@ -19,22 +19,29 @@ namespace webrtc { class FakeVideoTrackSource : public VideoTrackSource { public: + static rtc::scoped_refptr Create(bool is_screencast) { + return new rtc::RefCountedObject(is_screencast); + } + static rtc::scoped_refptr Create() { - return new rtc::RefCountedObject(); + return Create(false); } cricket::FakeVideoCapturer* fake_video_capturer() { return &fake_video_capturer_; } + bool is_screencast() const override { return is_screencast_; } + protected: - FakeVideoTrackSource() - : VideoTrackSource(&fake_video_capturer_, - false /* remote */) {} + explicit FakeVideoTrackSource(bool is_screencast) + : VideoTrackSource(&fake_video_capturer_, false /* remote */), + is_screencast_(is_screencast) {} virtual ~FakeVideoTrackSource() {} private: cricket::FakeVideoCapturer fake_video_capturer_; + const bool is_screencast_; }; } // namespace webrtc diff --git a/webrtc/api/videotrack.cc b/webrtc/api/videotrack.cc index b47a32044f..2fa2ba671a 100644 --- a/webrtc/api/videotrack.cc +++ b/webrtc/api/videotrack.cc @@ -19,7 +19,8 @@ const char MediaStreamTrackInterface::kVideoKind[] = "video"; VideoTrack::VideoTrack(const std::string& label, VideoTrackSourceInterface* video_source) : MediaStreamTrack(label), - video_source_(video_source) { + video_source_(video_source), + content_hint_(ContentHint::kNone) { worker_thread_checker_.DetachFromThread(); video_source_->RegisterObserver(this); } @@ -49,6 +50,19 @@ void VideoTrack::RemoveSink(rtc::VideoSinkInterface* sink) { video_source_->RemoveSink(sink); } +VideoTrackInterface::ContentHint VideoTrack::content_hint() const { + RTC_DCHECK_RUN_ON(&signaling_thread_checker_); + return content_hint_; +} + +void VideoTrack::set_content_hint(ContentHint hint) { + RTC_DCHECK_RUN_ON(&signaling_thread_checker_); + if (content_hint_ == hint) + return; + content_hint_ = hint; + Notifier::FireOnChanged(); +} + bool VideoTrack::set_enabled(bool enable) { RTC_DCHECK(signaling_thread_checker_.CalledOnValidThread()); for (auto& sink_pair : sink_pairs()) { diff --git a/webrtc/api/videotrack.h b/webrtc/api/videotrack.h index cbebec3e86..ebea848519 100644 --- a/webrtc/api/videotrack.h +++ b/webrtc/api/videotrack.h @@ -36,6 +36,8 @@ class VideoTrack : public MediaStreamTrack, VideoTrackSourceInterface* GetSource() const override { return video_source_.get(); } + ContentHint content_hint() const override; + void set_content_hint(ContentHint hint) override; bool set_enabled(bool enable) override; std::string kind() const override; @@ -50,6 +52,7 @@ class VideoTrack : public MediaStreamTrack, rtc::ThreadChecker signaling_thread_checker_; rtc::ThreadChecker worker_thread_checker_; rtc::scoped_refptr video_source_; + ContentHint content_hint_ GUARDED_BY(signaling_thread_checker_); }; } // namespace webrtc