diff --git a/webrtc/api/mediastreaminterface.h b/webrtc/api/mediastreaminterface.h index c9ce4a5a7b..54d3fd48c3 100644 --- a/webrtc/api/mediastreaminterface.h +++ b/webrtc/api/mediastreaminterface.h @@ -139,7 +139,17 @@ class VideoTrackSourceInterface virtual void Stop() = 0; virtual void Restart() = 0; - virtual const cricket::VideoOptions* options() const = 0; + // Indicates that parameters suitable for screencasts should be automatically + // applied to RtpSenders. + // TODO(perkj): Remove these once all known applications have moved to + // explicitly setting suitable parameters for screencasts and dont' need this + // implicit behavior. + virtual bool is_screencast() const = 0; + + // Indicates that the encoder should denoise the video before encoding it. + // TODO(perkj): Remove this once denoising is done by the source, and not by + // the encoder. + virtual bool needs_denoising() const = 0; protected: virtual ~VideoTrackSourceInterface() {} diff --git a/webrtc/api/rtpsender.cc b/webrtc/api/rtpsender.cc index 822b7f457e..9c6dfb8092 100644 --- a/webrtc/api/rtpsender.cc +++ b/webrtc/api/rtpsender.cc @@ -320,12 +320,14 @@ void VideoRtpSender::Stop() { void VideoRtpSender::SetVideoSend() { RTC_DCHECK(!stopped_ && can_send_track()); - const cricket::VideoOptions* options = nullptr; + cricket::VideoOptions options; VideoTrackSourceInterface* source = track_->GetSource(); - if (track_->enabled() && source) { - options = source->options(); + if (source) { + options.is_screencast = rtc::Optional(source->is_screencast()); + options.video_noise_reduction = + rtc::Optional(source->needs_denoising()); } - provider_->SetVideoSend(ssrc_, track_->enabled(), options); + provider_->SetVideoSend(ssrc_, track_->enabled(), &options); } } // namespace webrtc diff --git a/webrtc/api/rtpsenderreceiver_unittest.cc b/webrtc/api/rtpsenderreceiver_unittest.cc index ecaa6e5bb7..0a7c408bf0 100644 --- a/webrtc/api/rtpsenderreceiver_unittest.cc +++ b/webrtc/api/rtpsenderreceiver_unittest.cc @@ -19,7 +19,7 @@ #include "webrtc/api/rtpreceiver.h" #include "webrtc/api/rtpsender.h" #include "webrtc/api/streamcollection.h" -#include "webrtc/api/videocapturertracksource.h" +#include "webrtc/api/videotracksource.h" #include "webrtc/api/videotrack.h" #include "webrtc/base/gunit.h" #include "webrtc/media/base/fakevideocapturer.h" @@ -78,32 +78,20 @@ class MockVideoProvider : public VideoProviderInterface { const cricket::VideoOptions* options)); }; -class FakeVideoSource : public Notifier { +class FakeVideoTrackSource : public VideoTrackSource { public: - static rtc::scoped_refptr Create(bool remote) { - return new rtc::RefCountedObject(remote); + static rtc::scoped_refptr Create(bool remote) { + return new rtc::RefCountedObject(remote); } cricket::VideoCapturer* GetVideoCapturer() { return &fake_capturer_; } - void Stop() override {} - void Restart() override {} - void AddOrUpdateSink( - rtc::VideoSinkInterface* sink, - const rtc::VideoSinkWants& wants) override {} - void RemoveSink( - rtc::VideoSinkInterface* output) override {} - SourceState state() const override { return state_; } - bool remote() const override { return remote_; } - const cricket::VideoOptions* options() const override { return &options_; } protected: - explicit FakeVideoSource(bool remote) : state_(kLive), remote_(remote) {} - ~FakeVideoSource() {} + explicit FakeVideoTrackSource(bool remote) + : VideoTrackSource(&fake_capturer_, rtc::Thread::Current(), remote) {} + ~FakeVideoTrackSource() {} private: cricket::FakeVideoCapturer fake_capturer_; - SourceState state_; - bool remote_; - cricket::VideoOptions options_; }; class RtpSenderReceiverTest : public testing::Test { @@ -114,7 +102,7 @@ class RtpSenderReceiverTest : public testing::Test { void AddVideoTrack(bool remote) { rtc::scoped_refptr source( - FakeVideoSource::Create(remote)); + FakeVideoTrackSource::Create(remote)); video_track_ = VideoTrack::Create(kVideoTrackId, source); EXPECT_TRUE(stream_->AddTrack(video_track_)); } diff --git a/webrtc/api/videocapturertracksource.cc b/webrtc/api/videocapturertracksource.cc index c505e86bd5..e292c7a065 100644 --- a/webrtc/api/videocapturertracksource.cc +++ b/webrtc/api/videocapturertracksource.cc @@ -242,31 +242,16 @@ const cricket::VideoFormat& GetBestCaptureFormat( // Return false if the key is mandatory, and the value is invalid. bool ExtractOption(const MediaConstraintsInterface* all_constraints, const std::string& key, - rtc::Optional* option) { + bool* option) { size_t mandatory = 0; - bool value; - if (FindConstraint(all_constraints, key, &value, &mandatory)) { - *option = rtc::Optional(value); + *option = false; + if (FindConstraint(all_constraints, key, option, &mandatory)) { return true; } return mandatory == 0; } -// Search |all_constraints| for known video options. Apply all options that are -// found with valid values, and return false if any mandatory video option was -// found with an invalid value. -bool ExtractVideoOptions(const MediaConstraintsInterface* all_constraints, - cricket::VideoOptions* options) { - bool all_valid = true; - - all_valid &= - ExtractOption(all_constraints, MediaConstraintsInterface::kNoiseReduction, - &(options->video_noise_reduction)); - - return all_valid; -} - } // anonymous namespace namespace webrtc { @@ -302,12 +287,11 @@ VideoCapturerTrackSource::VideoCapturerTrackSource( rtc::Thread* worker_thread, cricket::VideoCapturer* capturer, bool remote) - : signaling_thread_(rtc::Thread::Current()), - worker_thread_(worker_thread), + : VideoTrackSource(capturer, worker_thread, remote), + signaling_thread_(rtc::Thread::Current()), video_capturer_(capturer), started_(false), - state_(kInitializing), - remote_(remote) { + needs_denoising_(false) { video_capturer_->SignalStateChange.connect( this, &VideoCapturerTrackSource::OnStateChange); } @@ -358,22 +342,17 @@ void VideoCapturerTrackSource::Initialize( return; } - cricket::VideoOptions options; - if (!ExtractVideoOptions(constraints, &options)) { - LOG(LS_WARNING) << "Could not satisfy mandatory options."; + if (!ExtractOption(constraints, MediaConstraintsInterface::kNoiseReduction, + &needs_denoising_)) { + LOG(LS_WARNING) << "Invalid mandatory value for" + << MediaConstraintsInterface::kNoiseReduction; SetState(kEnded); return; } - options_.SetAll(options); - options_.is_screencast = rtc::Optional(video_capturer_->IsScreencast()); format_ = GetBestCaptureFormat(formats); // Start the camera with our best guess. - // TODO(perkj): Should we try again with another format it it turns out that - // the camera doesn't produce frames with the correct format? Or will - // cricket::VideCapturer be able to re-scale / crop to the requested - // resolution? - if (!worker_thread_->Invoke( + if (!worker_thread()->Invoke( rtc::Bind(&cricket::VideoCapturer::StartCapturing, video_capturer_.get(), format_))) { SetState(kEnded); @@ -388,7 +367,7 @@ void VideoCapturerTrackSource::Stop() { return; } started_ = false; - worker_thread_->Invoke( + worker_thread()->Invoke( rtc::Bind(&cricket::VideoCapturer::Stop, video_capturer_.get())); } @@ -396,7 +375,7 @@ void VideoCapturerTrackSource::Restart() { if (started_) { return; } - if (!worker_thread_->Invoke( + if (!worker_thread()->Invoke( rtc::Bind(&cricket::VideoCapturer::StartCapturing, video_capturer_.get(), format_))) { SetState(kEnded); @@ -405,20 +384,6 @@ void VideoCapturerTrackSource::Restart() { started_ = true; } -void VideoCapturerTrackSource::AddOrUpdateSink( - rtc::VideoSinkInterface* sink, - const rtc::VideoSinkWants& wants) { - worker_thread_->Invoke( - rtc::Bind(&cricket::VideoCapturer::AddOrUpdateSink, video_capturer_.get(), - sink, wants)); -} - -void VideoCapturerTrackSource::RemoveSink( - rtc::VideoSinkInterface* output) { - worker_thread_->Invoke(rtc::Bind(&cricket::VideoCapturer::RemoveSink, - video_capturer_.get(), output)); -} - // OnStateChange listens to the cricket::VideoCapturer::SignalStateChange. void VideoCapturerTrackSource::OnStateChange( cricket::VideoCapturer* capturer, @@ -435,11 +400,4 @@ void VideoCapturerTrackSource::OnStateChange( } } -void VideoCapturerTrackSource::SetState(SourceState new_state) { - if (state_ != new_state) { - state_ = new_state; - FireOnChanged(); - } -} - } // namespace webrtc diff --git a/webrtc/api/videocapturertracksource.h b/webrtc/api/videocapturertracksource.h index f086d793e7..9a8d05d9ed 100644 --- a/webrtc/api/videocapturertracksource.h +++ b/webrtc/api/videocapturertracksource.h @@ -11,16 +11,11 @@ #ifndef WEBRTC_API_VIDEOCAPTURERTRACKSOURCE_H_ #define WEBRTC_API_VIDEOCAPTURERTRACKSOURCE_H_ -#include - #include "webrtc/api/mediastreaminterface.h" -#include "webrtc/api/notifier.h" -#include "webrtc/api/videosourceinterface.h" -#include "webrtc/api/videotrackrenderers.h" +#include "webrtc/api/videotracksource.h" #include "webrtc/base/asyncinvoker.h" #include "webrtc/base/scoped_ptr.h" #include "webrtc/base/sigslot.h" -#include "webrtc/media/base/videosinkinterface.h" #include "webrtc/media/base/videocapturer.h" #include "webrtc/media/base/videocommon.h" @@ -34,7 +29,7 @@ namespace webrtc { class MediaConstraintsInterface; -class VideoCapturerTrackSource : public Notifier, +class VideoCapturerTrackSource : public VideoTrackSource, public sigslot::has_slots<> { public: // Creates an instance of VideoCapturerTrackSource. @@ -52,22 +47,18 @@ class VideoCapturerTrackSource : public Notifier, cricket::VideoCapturer* capturer, bool remote); - SourceState state() const override { return state_; } - bool remote() const override { return remote_; } - - virtual const cricket::VideoOptions* options() const { return &options_; } - - virtual cricket::VideoCapturer* GetVideoCapturer() { + cricket::VideoCapturer* GetVideoCapturer() override { return video_capturer_.get(); } + bool is_screencast() const override { + return video_capturer_->IsScreencast(); + } + bool needs_denoising() const override { return needs_denoising_; } + void Stop() override; void Restart() override; - void AddOrUpdateSink(rtc::VideoSinkInterface* sink, - const rtc::VideoSinkWants& wants) override; - void RemoveSink(rtc::VideoSinkInterface* sink) override; - protected: VideoCapturerTrackSource(rtc::Thread* worker_thread, cricket::VideoCapturer* capturer, @@ -78,19 +69,13 @@ class VideoCapturerTrackSource : public Notifier, private: void OnStateChange(cricket::VideoCapturer* capturer, cricket::CaptureState capture_state); - void SetState(SourceState new_state); rtc::Thread* signaling_thread_; - rtc::Thread* worker_thread_; rtc::AsyncInvoker invoker_; rtc::scoped_ptr video_capturer_; bool started_; - rtc::scoped_ptr frame_input_; - cricket::VideoFormat format_; - cricket::VideoOptions options_; - SourceState state_; - const bool remote_; + bool needs_denoising_; }; } // namespace webrtc diff --git a/webrtc/api/videocapturertracksource_unittest.cc b/webrtc/api/videocapturertracksource_unittest.cc index e059ef312c..b183421153 100644 --- a/webrtc/api/videocapturertracksource_unittest.cc +++ b/webrtc/api/videocapturertracksource_unittest.cc @@ -354,47 +354,45 @@ TEST_F(VideoCapturerTrackSourceTest, InvalidOptionalConstraint) { kMaxWaitMs); } -TEST_F(VideoCapturerTrackSourceTest, SetValidOptionValues) { +TEST_F(VideoCapturerTrackSourceTest, SetValidDenoisingConstraint) { FakeConstraints constraints; - constraints.AddMandatory(MediaConstraintsInterface::kNoiseReduction, "false"); + constraints.AddMandatory(MediaConstraintsInterface::kNoiseReduction, "true"); CreateVideoCapturerSource(&constraints); - EXPECT_EQ(rtc::Optional(false), - source_->options()->video_noise_reduction); + EXPECT_TRUE(source_->needs_denoising()); } -TEST_F(VideoCapturerTrackSourceTest, OptionNotSet) { +TEST_F(VideoCapturerTrackSourceTest, NoiseReductionConstraintNotSet) { FakeConstraints constraints; CreateVideoCapturerSource(&constraints); - EXPECT_EQ(rtc::Optional(), source_->options()->video_noise_reduction); + EXPECT_FALSE(source_->needs_denoising()); } -TEST_F(VideoCapturerTrackSourceTest, MandatoryOptionOverridesOptional) { +TEST_F(VideoCapturerTrackSourceTest, + MandatoryDenoisingConstraintOverridesOptional) { FakeConstraints constraints; - constraints.AddMandatory(MediaConstraintsInterface::kNoiseReduction, true); - constraints.AddOptional(MediaConstraintsInterface::kNoiseReduction, false); + constraints.AddMandatory(MediaConstraintsInterface::kNoiseReduction, false); + constraints.AddOptional(MediaConstraintsInterface::kNoiseReduction, true); CreateVideoCapturerSource(&constraints); - EXPECT_EQ(rtc::Optional(true), - source_->options()->video_noise_reduction); + EXPECT_FALSE(source_->needs_denoising()); } -TEST_F(VideoCapturerTrackSourceTest, InvalidOptionKeyOptional) { +TEST_F(VideoCapturerTrackSourceTest, NoiseReductionAndInvalidKeyOptional) { FakeConstraints constraints; - constraints.AddOptional(MediaConstraintsInterface::kNoiseReduction, false); + constraints.AddOptional(MediaConstraintsInterface::kNoiseReduction, true); constraints.AddOptional("invalidKey", false); CreateVideoCapturerSource(&constraints); EXPECT_EQ_WAIT(MediaSourceInterface::kLive, state_observer_->state(), kMaxWaitMs); - EXPECT_EQ(rtc::Optional(false), - source_->options()->video_noise_reduction); + EXPECT_TRUE(source_->needs_denoising()); } -TEST_F(VideoCapturerTrackSourceTest, InvalidOptionKeyMandatory) { +TEST_F(VideoCapturerTrackSourceTest, NoiseReductionAndInvalidKeyMandatory) { FakeConstraints constraints; constraints.AddMandatory(MediaConstraintsInterface::kNoiseReduction, false); constraints.AddMandatory("invalidKey", false); @@ -403,10 +401,10 @@ TEST_F(VideoCapturerTrackSourceTest, InvalidOptionKeyMandatory) { EXPECT_EQ_WAIT(MediaSourceInterface::kEnded, state_observer_->state(), kMaxWaitMs); - EXPECT_EQ(rtc::Optional(), source_->options()->video_noise_reduction); + EXPECT_FALSE(source_->needs_denoising()); } -TEST_F(VideoCapturerTrackSourceTest, InvalidOptionValueOptional) { +TEST_F(VideoCapturerTrackSourceTest, InvalidDenoisingValueOptional) { FakeConstraints constraints; constraints.AddOptional(MediaConstraintsInterface::kNoiseReduction, "not a boolean"); @@ -415,10 +413,10 @@ TEST_F(VideoCapturerTrackSourceTest, InvalidOptionValueOptional) { EXPECT_EQ_WAIT(MediaSourceInterface::kLive, state_observer_->state(), kMaxWaitMs); - EXPECT_EQ(rtc::Optional(), source_->options()->video_noise_reduction); + EXPECT_FALSE(source_->needs_denoising()); } -TEST_F(VideoCapturerTrackSourceTest, InvalidOptionValueMandatory) { +TEST_F(VideoCapturerTrackSourceTest, InvalidDenoisingValueMandatory) { FakeConstraints constraints; // Optional constraints should be ignored if the mandatory constraints fail. constraints.AddOptional(MediaConstraintsInterface::kNoiseReduction, "false"); @@ -429,7 +427,7 @@ TEST_F(VideoCapturerTrackSourceTest, InvalidOptionValueMandatory) { EXPECT_EQ_WAIT(MediaSourceInterface::kEnded, state_observer_->state(), kMaxWaitMs); - EXPECT_EQ(rtc::Optional(), source_->options()->video_noise_reduction); + EXPECT_FALSE(source_->needs_denoising()); } TEST_F(VideoCapturerTrackSourceTest, MixedOptionsAndConstraints) { @@ -450,8 +448,7 @@ TEST_F(VideoCapturerTrackSourceTest, MixedOptionsAndConstraints) { EXPECT_EQ(288, format->height); EXPECT_EQ(30, format->framerate()); - EXPECT_EQ(rtc::Optional(false), - source_->options()->video_noise_reduction); + EXPECT_FALSE(source_->needs_denoising()); } // Tests that the source starts video with the default resolution for @@ -461,6 +458,7 @@ TEST_F(VideoCapturerTrackSourceTest, ScreencastResolutionNoConstraint) { capturer_->TestWithoutCameraFormats(); CreateVideoCapturerSource(); + ASSERT_TRUE(source_->is_screencast()); EXPECT_EQ_WAIT(MediaSourceInterface::kLive, state_observer_->state(), kMaxWaitMs); const cricket::VideoFormat* format = capturer_->GetCaptureFormat(); @@ -481,6 +479,7 @@ TEST_F(VideoCapturerTrackSourceTest, ScreencastResolutionWithConstraint) { capturer_->TestWithoutCameraFormats(); CreateVideoCapturerSource(&constraints); + ASSERT_TRUE(source_->is_screencast()); EXPECT_EQ_WAIT(MediaSourceInterface::kLive, state_observer_->state(), kMaxWaitMs); const cricket::VideoFormat* format = capturer_->GetCaptureFormat(); diff --git a/webrtc/api/videosourceproxy.h b/webrtc/api/videosourceproxy.h index 59adf89094..c251b3687c 100644 --- a/webrtc/api/videosourceproxy.h +++ b/webrtc/api/videosourceproxy.h @@ -26,12 +26,13 @@ PROXY_CONSTMETHOD0(bool, remote) PROXY_METHOD0(cricket::VideoCapturer*, GetVideoCapturer) PROXY_METHOD0(void, Stop) PROXY_METHOD0(void, Restart) +PROXY_CONSTMETHOD0(bool, is_screencast) +PROXY_CONSTMETHOD0(bool, needs_denoising) PROXY_METHOD2(void, AddOrUpdateSink, rtc::VideoSinkInterface*, const rtc::VideoSinkWants&) PROXY_METHOD1(void, RemoveSink, rtc::VideoSinkInterface*) -PROXY_CONSTMETHOD0(const cricket::VideoOptions*, options) PROXY_METHOD1(void, RegisterObserver, ObserverInterface*) PROXY_METHOD1(void, UnregisterObserver, ObserverInterface*) END_PROXY() diff --git a/webrtc/api/videotracksource.cc b/webrtc/api/videotracksource.cc index 1e1020c671..3b5e19a5b7 100644 --- a/webrtc/api/videotracksource.cc +++ b/webrtc/api/videotracksource.cc @@ -10,5 +10,41 @@ #include "webrtc/api/videotracksource.h" -// TODO(perkj): This file is added to prepare for splitting VideoSource -// into two parts, VideoCapturerTrackSource that will inherit VideoTrackSource. +#include + +#include "webrtc/base/bind.h" + +namespace webrtc { + +VideoTrackSource::VideoTrackSource( + rtc::VideoSourceInterface* source, + rtc::Thread* worker_thread, + bool remote) + : source_(source), + worker_thread_(worker_thread), + state_(kInitializing), + remote_(remote) {} + +void VideoTrackSource::SetState(SourceState new_state) { + if (state_ != new_state) { + state_ = new_state; + FireOnChanged(); + } +} + +void VideoTrackSource::AddOrUpdateSink( + rtc::VideoSinkInterface* sink, + const rtc::VideoSinkWants& wants) { + worker_thread_->Invoke(rtc::Bind( + &rtc::VideoSourceInterface::AddOrUpdateSink, source_, + sink, wants)); +} + +void VideoTrackSource::RemoveSink( + rtc::VideoSinkInterface* sink) { + worker_thread_->Invoke( + rtc::Bind(&rtc::VideoSourceInterface::RemoveSink, + source_, sink)); +} + +} // namespace webrtc diff --git a/webrtc/api/videotracksource.h b/webrtc/api/videotracksource.h index a1340acf34..59dae42950 100644 --- a/webrtc/api/videotracksource.h +++ b/webrtc/api/videotracksource.h @@ -11,7 +11,45 @@ #ifndef WEBRTC_API_VIDEOTRACKSOURCE_H_ #define WEBRTC_API_VIDEOTRACKSOURCE_H_ -// TODO(perkj): This file is added to prepare for splitting VideoSource -// into two parts, VideoCapturerTrackSource that will inherit VideoTrackSource. +#include "webrtc/api/notifier.h" +#include "webrtc/api/videosourceinterface.h" +#include "webrtc/media/base/mediachannel.h" +#include "webrtc/media/base/videosinkinterface.h" + +// VideoTrackSource implements VideoTrackSourceInterface. +namespace webrtc { + +class VideoTrackSource : public Notifier { + public: + VideoTrackSource(rtc::VideoSourceInterface* source, + rtc::Thread* worker_thread, + bool remote); + + void SetState(SourceState new_state); + SourceState state() const override { return state_; } + bool remote() const override { return remote_; } + + void Stop() override{}; + void Restart() override{}; + + virtual bool is_screencast() const { return false; }; + virtual bool needs_denoising() const { return false; }; + + void AddOrUpdateSink(rtc::VideoSinkInterface* sink, + const rtc::VideoSinkWants& wants) override; + void RemoveSink(rtc::VideoSinkInterface* sink) override; + + protected: + rtc::Thread* worker_thread() { return worker_thread_; } + + private: + rtc::VideoSourceInterface* source_; + rtc::Thread* worker_thread_; + cricket::VideoOptions options_; + SourceState state_; + const bool remote_; +}; + +} // namespace webrtc #endif // WEBRTC_API_VIDEOTRACKSOURCE_H_