Change VideoSourceInterface::needs_denoising() to return rtc::Optional<bool>

It turns out that it is used as if it has three states: on/off default.
This reverts back to the behaviour prior to https://codereview.webrtc.org/1773993002

BUG=chromium:594434
R=pbos@webrtc.org, pthatcher@webrtc.org

Review URL: https://codereview.webrtc.org/1842073002 .

Cr-Commit-Position: refs/heads/master@{#12181}

webrtc/api/mediastreaminterface.h

@@ -23,6 +23,7 @@
 #include "webrtc/base/basictypes.h"
 #include "webrtc/base/refcount.h"
 #include "webrtc/base/scoped_ref_ptr.h"
+#include "webrtc/base/optional.h"
 #include "webrtc/media/base/mediachannel.h"
 #include "webrtc/media/base/videosinkinterface.h"
 #include "webrtc/media/base/videosourceinterface.h"
@@ -124,10 +125,12 @@ class VideoTrackSourceInterface
   // implicit behavior.
   virtual bool is_screencast() const = 0;
 
-  // Indicates that the encoder should denoise the video before encoding it.
+  // Indicates that the encoder should denoise video before encoding it.
+  // If it is not set, the default configuration is used which is different
+  // depending on video codec.
   // TODO(perkj): Remove this once denoising is done by the source, and not by
   // the encoder.
-  virtual bool needs_denoising() const = 0;
+  virtual rtc::Optional<bool> needs_denoising() const = 0;
 
  protected:
   virtual ~VideoTrackSourceInterface() {}

webrtc/api/rtpsender.cc

@@ -332,8 +332,7 @@ void VideoRtpSender::SetVideoSend() {
   VideoTrackSourceInterface* source = track_->GetSource();
   if (source) {
     options.is_screencast = rtc::Optional<bool>(source->is_screencast());
-    options.video_noise_reduction =
-        rtc::Optional<bool>(source->needs_denoising());
+    options.video_noise_reduction = source->needs_denoising();
   }
   provider_->SetVideoSend(ssrc_, track_->enabled(), &options);
 }

webrtc/api/videocapturertracksource.cc

@@ -240,10 +240,11 @@ 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,
-                   bool* option) {
+                   rtc::Optional<bool>* option) {
   size_t mandatory = 0;
-  *option = false;
-  if (FindConstraint(all_constraints, key, option, &mandatory)) {
+  bool value;
+  if (FindConstraint(all_constraints, key, &value, &mandatory)) {
+    *option = rtc::Optional<bool>(value);
     return true;
   }
 
@@ -288,8 +289,7 @@ VideoCapturerTrackSource::VideoCapturerTrackSource(
     : VideoTrackSource(capturer, worker_thread, remote),
       signaling_thread_(rtc::Thread::Current()),
       video_capturer_(capturer),
-      started_(false),
-      needs_denoising_(false) {
+      started_(false) {
   video_capturer_->SignalStateChange.connect(
       this, &VideoCapturerTrackSource::OnStateChange);
 }

webrtc/api/videocapturertracksource.h

@@ -54,7 +54,9 @@ class VideoCapturerTrackSource : public VideoTrackSource,
   bool is_screencast() const override {
     return video_capturer_->IsScreencast();
   }
-  bool needs_denoising() const override { return needs_denoising_; }
+  rtc::Optional<bool> needs_denoising() const override {
+    return needs_denoising_;
+  }
 
   void Stop() override;
   void Restart() override;
@@ -75,7 +77,7 @@ class VideoCapturerTrackSource : public VideoTrackSource,
   rtc::scoped_ptr<cricket::VideoCapturer> video_capturer_;
   bool started_;
   cricket::VideoFormat format_;
-  bool needs_denoising_;
+  rtc::Optional<bool> needs_denoising_;
 };
 
 }  // namespace webrtc

webrtc/api/videocapturertracksource_unittest.cc

@@ -334,17 +334,17 @@ TEST_F(VideoCapturerTrackSourceTest, InvalidOptionalConstraint) {
 
 TEST_F(VideoCapturerTrackSourceTest, SetValidDenoisingConstraint) {
   FakeConstraints constraints;
-  constraints.AddMandatory(MediaConstraintsInterface::kNoiseReduction, "true");
+  constraints.AddMandatory(MediaConstraintsInterface::kNoiseReduction, "false");
 
   CreateVideoCapturerSource(&constraints);
 
-  EXPECT_TRUE(source_->needs_denoising());
+  EXPECT_EQ(rtc::Optional<bool>(false), source_->needs_denoising());
 }
 
 TEST_F(VideoCapturerTrackSourceTest, NoiseReductionConstraintNotSet) {
   FakeConstraints constraints;
   CreateVideoCapturerSource(&constraints);
-  EXPECT_FALSE(source_->needs_denoising());
+  EXPECT_EQ(rtc::Optional<bool>(), source_->needs_denoising());
 }
 
 TEST_F(VideoCapturerTrackSourceTest,
@@ -355,7 +355,7 @@ TEST_F(VideoCapturerTrackSourceTest,
 
   CreateVideoCapturerSource(&constraints);
 
-  EXPECT_FALSE(source_->needs_denoising());
+  EXPECT_EQ(rtc::Optional<bool>(false), source_->needs_denoising());
 }
 
 TEST_F(VideoCapturerTrackSourceTest, NoiseReductionAndInvalidKeyOptional) {
@@ -367,7 +367,7 @@ TEST_F(VideoCapturerTrackSourceTest, NoiseReductionAndInvalidKeyOptional) {
 
   EXPECT_EQ_WAIT(MediaSourceInterface::kLive, state_observer_->state(),
                  kMaxWaitMs);
-  EXPECT_TRUE(source_->needs_denoising());
+  EXPECT_EQ(rtc::Optional<bool>(true), source_->needs_denoising());
 }
 
 TEST_F(VideoCapturerTrackSourceTest, NoiseReductionAndInvalidKeyMandatory) {
@@ -379,7 +379,7 @@ TEST_F(VideoCapturerTrackSourceTest, NoiseReductionAndInvalidKeyMandatory) {
 
   EXPECT_EQ_WAIT(MediaSourceInterface::kEnded, state_observer_->state(),
                  kMaxWaitMs);
-  EXPECT_FALSE(source_->needs_denoising());
+  EXPECT_EQ(rtc::Optional<bool>(), source_->needs_denoising());
 }
 
 TEST_F(VideoCapturerTrackSourceTest, InvalidDenoisingValueOptional) {
@@ -391,7 +391,8 @@ TEST_F(VideoCapturerTrackSourceTest, InvalidDenoisingValueOptional) {
 
   EXPECT_EQ_WAIT(MediaSourceInterface::kLive, state_observer_->state(),
                  kMaxWaitMs);
-  EXPECT_FALSE(source_->needs_denoising());
+
+  EXPECT_EQ(rtc::Optional<bool>(), source_->needs_denoising());
 }
 
 TEST_F(VideoCapturerTrackSourceTest, InvalidDenoisingValueMandatory) {
@@ -405,7 +406,7 @@ TEST_F(VideoCapturerTrackSourceTest, InvalidDenoisingValueMandatory) {
 
   EXPECT_EQ_WAIT(MediaSourceInterface::kEnded, state_observer_->state(),
                  kMaxWaitMs);
-  EXPECT_FALSE(source_->needs_denoising());
+  EXPECT_EQ(rtc::Optional<bool>(), source_->needs_denoising());
 }
 
 TEST_F(VideoCapturerTrackSourceTest, MixedOptionsAndConstraints) {
@@ -426,7 +427,7 @@ TEST_F(VideoCapturerTrackSourceTest, MixedOptionsAndConstraints) {
   EXPECT_EQ(288, format->height);
   EXPECT_EQ(30, format->framerate());
 
-  EXPECT_FALSE(source_->needs_denoising());
+  EXPECT_EQ(rtc::Optional<bool>(false), source_->needs_denoising());
 }
 
 // Tests that the source starts video with the default resolution for

webrtc/api/videosourceproxy.h

@@ -27,7 +27,7 @@ PROXY_METHOD0(cricket::VideoCapturer*, GetVideoCapturer)
 PROXY_METHOD0(void, Stop)
 PROXY_METHOD0(void, Restart)
 PROXY_CONSTMETHOD0(bool, is_screencast)
-PROXY_CONSTMETHOD0(bool, needs_denoising)
+PROXY_CONSTMETHOD0(rtc::Optional<bool>, needs_denoising)
 PROXY_METHOD2(void,
               AddOrUpdateSink,
               rtc::VideoSinkInterface<cricket::VideoFrame>*,

webrtc/api/videotracksource.h

@@ -37,7 +37,8 @@ class VideoTrackSource : public Notifier<VideoTrackSourceInterface> {
   void Restart() override{};
 
   virtual bool is_screencast() const { return false; }
-  virtual bool needs_denoising() const { return false; }
+  virtual rtc::Optional<bool> needs_denoising() const {
+    return rtc::Optional<bool>(); }
 
   void AddOrUpdateSink(rtc::VideoSinkInterface<cricket::VideoFrame>* sink,
                        const rtc::VideoSinkWants& wants) override;