From 66712b024f1e3490c1aecee431e0897c961c54fe Mon Sep 17 00:00:00 2001 From: nisse Date: Mon, 24 Oct 2016 00:22:31 -0700 Subject: [PATCH] Revert of Add method cricket::VideoCapturer::NeedsDenoising, use in VideoCapturerTrackSource. (patchset #5 id:80001 of https://codereview.webrtc.org/2334683002/ ) Reason for revert: This was a workaround to help Chrome wire up the googNoiseReduction constraint. However, it was fixed in a different way in Chrome, and this hack is not actually needed. Original issue's description: > Add method cricket::VideoCapturer::NeedsDenoising, use in VideoCapturerTrackSource. > > BUG=chromium:645907 > > Committed: https://crrev.com/0d14c6abba19295725519ce38105688ae0a62513 > Cr-Commit-Position: refs/heads/master@{#14219} TBR=pbos@webrtc.org,hta@webrtc.org,perkj@webrtc.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=chromium:645907 Review-Url: https://codereview.webrtc.org/2433293003 Cr-Commit-Position: refs/heads/master@{#14729} --- webrtc/api/videocapturertracksource.cc | 3 -- .../api/videocapturertracksource_unittest.cc | 29 ------------------- webrtc/media/base/fakevideocapturer.h | 8 ----- webrtc/media/base/videocapturer.h | 12 -------- 4 files changed, 52 deletions(-) diff --git a/webrtc/api/videocapturertracksource.cc b/webrtc/api/videocapturertracksource.cc index f8897a2846..1bf4bea1d0 100644 --- a/webrtc/api/videocapturertracksource.cc +++ b/webrtc/api/videocapturertracksource.cc @@ -320,9 +320,6 @@ void VideoCapturerTrackSource::Initialize( } } - // Get default from the capturer, overridden by constraints, if any. - needs_denoising_ = video_capturer_->NeedsDenoising(); - if (constraints) { MediaConstraintsInterface::Constraints mandatory_constraints = constraints->GetMandatory(); diff --git a/webrtc/api/videocapturertracksource_unittest.cc b/webrtc/api/videocapturertracksource_unittest.cc index d8f4b0c761..67c249827b 100644 --- a/webrtc/api/videocapturertracksource_unittest.cc +++ b/webrtc/api/videocapturertracksource_unittest.cc @@ -444,35 +444,6 @@ TEST_F(VideoCapturerTrackSourceTest, ScreencastResolutionWithConstraint) { EXPECT_EQ(30, format->framerate()); } -TEST_F(VideoCapturerTrackSourceTest, DenoisingDefault) { - CreateVideoCapturerSource(); - EXPECT_FALSE(source_->needs_denoising()); -} - -TEST_F(VideoCapturerTrackSourceTest, DenoisingConstraintOn) { - FakeConstraints constraints; - constraints.AddOptional(MediaConstraintsInterface::kNoiseReduction, true); - CreateVideoCapturerSource(&constraints); - ASSERT_TRUE(source_->needs_denoising()); - EXPECT_TRUE(*source_->needs_denoising()); -} - -TEST_F(VideoCapturerTrackSourceTest, DenoisingCapturerOff) { - capturer_->SetNeedsDenoising(rtc::Optional(false)); - CreateVideoCapturerSource(); - ASSERT_TRUE(source_->needs_denoising()); - EXPECT_FALSE(*source_->needs_denoising()); -} - -TEST_F(VideoCapturerTrackSourceTest, DenoisingConstraintOverridesCapturer) { - capturer_->SetNeedsDenoising(rtc::Optional(false)); - FakeConstraints constraints; - constraints.AddOptional(MediaConstraintsInterface::kNoiseReduction, true); - CreateVideoCapturerSource(&constraints); - ASSERT_TRUE(source_->needs_denoising()); - EXPECT_TRUE(*source_->needs_denoising()); -} - TEST_F(VideoCapturerTrackSourceTest, MandatorySubOneFpsConstraints) { FakeConstraints constraints; constraints.AddMandatory(MediaConstraintsInterface::kMaxFrameRate, 0.5); diff --git a/webrtc/media/base/fakevideocapturer.h b/webrtc/media/base/fakevideocapturer.h index 5b6a65cfa4..ba8f01eddb 100644 --- a/webrtc/media/base/fakevideocapturer.h +++ b/webrtc/media/base/fakevideocapturer.h @@ -121,9 +121,6 @@ class FakeVideoCapturer : public cricket::VideoCapturer { } bool IsRunning() override { return running_; } bool IsScreencast() const override { return is_screencast_; } - rtc::Optional NeedsDenoising() const override { - return needs_denoising_; - } bool GetPreferredFourccs(std::vector* fourccs) override { fourccs->push_back(cricket::FOURCC_I420); fourccs->push_back(cricket::FOURCC_MJPG); @@ -136,16 +133,11 @@ class FakeVideoCapturer : public cricket::VideoCapturer { webrtc::VideoRotation GetRotation() { return rotation_; } - void SetNeedsDenoising(rtc::Optional needs_denoising) { - needs_denoising_ = needs_denoising; - } - private: bool running_; int64_t initial_timestamp_; int64_t next_timestamp_; const bool is_screencast_; - rtc::Optional needs_denoising_; webrtc::VideoRotation rotation_; }; diff --git a/webrtc/media/base/videocapturer.h b/webrtc/media/base/videocapturer.h index bf3bc64838..909c838fa5 100644 --- a/webrtc/media/base/videocapturer.h +++ b/webrtc/media/base/videocapturer.h @@ -140,18 +140,6 @@ class VideoCapturer : public sigslot::has_slots<>, // implement screencast specific behavior. virtual bool IsScreencast() const = 0; - // Indicates that the encoder should denoise video before encoding - // it, wired up to VideoCapturerTrackSource::needs_denoising. If it - // is not set, the default configuration is used which is different - // depending on video codec. - // TODO(nisse): This is a workaround needed to fix - // https://bugs.chromium.org/p/chromium/issues/detail?id=645907. - // Chrome should migrate to implement VideoTrackSourceInterface - // directly, and then this method is no longer needed. - virtual rtc::Optional NeedsDenoising() const { - return rtc::Optional(); - } - // Caps the VideoCapturer's format according to max_format. It can e.g. be // used to prevent cameras from capturing at a resolution or framerate that // the capturer is capable of but not performing satisfactorily at.