Revert "Add ability to specify if rate controller of video encoder is…

… trusted." This reverts commit 3e335d1. Reason for revert: breaks downstream project Original change's description: > Add ability to specify if rate controller of video encoder is trusted. > > If rate controller is trusted, we disable the frame dropper in the > media optimization module. > > Bug: webrtc:9722 > Change-Id: I821f21fd74a400ee9d5aa3f6b42d4e569033acbe > Reviewed-on: https://webrtc-review.googlesource.com/c/105020 > Commit-Queue: Erik Språng <[email protected]> > Reviewed-by: Per Kjellander <[email protected]> > Reviewed-by: Niels Moller <[email protected]> > Reviewed-by: Rasmus Brandt <[email protected]> > Reviewed-by: Ilya Nikolaevskiy <[email protected]> > Cr-Commit-Position: refs/heads/master@{#25107} [email protected],[email protected],[email protected],[email protected],[email protected] Change-Id: Ifdb0aae684894854a184ec1e7423a7c62e7ba237 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: webrtc:9722 Reviewed-on: https://webrtc-review.googlesource.com/c/105360 Commit-Queue: Oleh Prypin <[email protected]> Reviewed-by: Oleh Prypin <[email protected]> Cr-Commit-Position: refs/heads/master@{#25117}
basharalbashier · Oct 11, 2018 · a1d9ca4 · a1d9ca4
1 parent cdc959f
commit a1d9ca4
Show file tree

Hide file tree

Showing 31 changed files with 42 additions and 406 deletions.
diff --git a/api/BUILD.gn b/api/BUILD.gn
@@ -515,19 +515,6 @@ if (rtc_include_tests) {
     ]
   }
 
-  rtc_source_set("mock_video_encoder") {
-    testonly = true
-    sources = [
-      "test/mock_video_encoder.cc",
-      "test/mock_video_encoder.h",
-    ]
-
-    deps = [
-      "../api/video_codecs:video_codecs_api",
-      "../test:test_support",
-    ]
-  }
-
   rtc_source_set("rtc_api_unittests") {
     testonly = true
 

diff --git a/api/test/mock_video_encoder.cc b/api/test/mock_video_encoder.cc
diff --git a/api/test/mock_video_encoder.h b/api/test/mock_video_encoder.h
diff --git a/api/video_codecs/test/BUILD.gn b/api/video_codecs/test/BUILD.gn
@@ -21,7 +21,6 @@ if (rtc_include_tests) {
       "..:builtin_video_encoder_factory",
       "..:rtc_software_fallback_wrappers",
       "..:video_codecs_api",
-      "../../../api:mock_video_encoder",
       "../../../modules/video_coding:video_codec_interface",
       "../../../modules/video_coding:video_coding_utility",
       "../../../modules/video_coding:webrtc_vp8",

diff --git a/api/video_codecs/test/video_encoder_software_fallback_wrapper_unittest.cc b/api/video_codecs/test/video_encoder_software_fallback_wrapper_unittest.cc
@@ -12,7 +12,6 @@
 
 #include <utility>
 
-#include "api/test/mock_video_encoder.h"
 #include "api/video/i420_buffer.h"
 #include "api/video/video_bitrate_allocation.h"
 #include "modules/video_coding/codecs/vp8/include/vp8.h"
@@ -23,12 +22,9 @@
 #include "rtc_base/checks.h"
 #include "rtc_base/fakeclock.h"
 #include "test/field_trial.h"
-#include "test/gmock.h"
 #include "test/gtest.h"
 
 namespace webrtc {
-using ::testing::Return;
-
 namespace {
 const int kWidth = 320;
 const int kHeight = 240;
@@ -533,88 +529,4 @@ TEST_F(ForcedFallbackTestEnabled, ScalingDisabledIfResizeOff) {
   EXPECT_FALSE(settings.thresholds.has_value());
 }
 
-TEST(SoftwareFallbackEncoderTest, BothRateControllersNotTrusted) {
-  auto* sw_encoder = new testing::NiceMock<MockVideoEncoder>();
-  auto* hw_encoder = new testing::NiceMock<MockVideoEncoder>();
-  EXPECT_CALL(*sw_encoder, HasTrustedRateController())
-      .WillRepeatedly(Return(false));
-  EXPECT_CALL(*hw_encoder, HasTrustedRateController())
-      .WillRepeatedly(Return(false));
-
-  std::unique_ptr<VideoEncoder> wrapper =
-      CreateVideoEncoderSoftwareFallbackWrapper(
-          std::unique_ptr<VideoEncoder>(sw_encoder),
-          std::unique_ptr<VideoEncoder>(hw_encoder));
-  EXPECT_FALSE(wrapper->HasTrustedRateController());
-}
-
-TEST(SoftwareFallbackEncoderTest, SwRateControllerTrusted) {
-  auto* sw_encoder = new testing::NiceMock<MockVideoEncoder>();
-  auto* hw_encoder = new testing::NiceMock<MockVideoEncoder>();
-  EXPECT_CALL(*sw_encoder, HasTrustedRateController())
-      .WillRepeatedly(Return(true));
-  EXPECT_CALL(*hw_encoder, HasTrustedRateController())
-      .WillRepeatedly(Return(false));
-
-  std::unique_ptr<VideoEncoder> wrapper =
-      CreateVideoEncoderSoftwareFallbackWrapper(
-          std::unique_ptr<VideoEncoder>(sw_encoder),
-          std::unique_ptr<VideoEncoder>(hw_encoder));
-  EXPECT_FALSE(wrapper->HasTrustedRateController());
-}
-
-TEST(SoftwareFallbackEncoderTest, SwRateControllerTrustedNoHw) {
-  auto* sw_encoder = new testing::NiceMock<MockVideoEncoder>();
-  EXPECT_CALL(*sw_encoder, HasTrustedRateController())
-      .WillRepeatedly(Return(true));
-
-  std::unique_ptr<VideoEncoder> wrapper =
-      CreateVideoEncoderSoftwareFallbackWrapper(
-          std::unique_ptr<VideoEncoder>(sw_encoder),
-          std::unique_ptr<VideoEncoder>());
-  EXPECT_TRUE(wrapper->HasTrustedRateController());
-}
-
-TEST(SoftwareFallbackEncoderTest, HwRateControllerTrusted) {
-  auto* sw_encoder = new testing::NiceMock<MockVideoEncoder>();
-  auto* hw_encoder = new testing::NiceMock<MockVideoEncoder>();
-  EXPECT_CALL(*sw_encoder, HasTrustedRateController())
-      .WillRepeatedly(Return(false));
-  EXPECT_CALL(*hw_encoder, HasTrustedRateController())
-      .WillRepeatedly(Return(true));
-
-  std::unique_ptr<VideoEncoder> wrapper =
-      CreateVideoEncoderSoftwareFallbackWrapper(
-          std::unique_ptr<VideoEncoder>(sw_encoder),
-          std::unique_ptr<VideoEncoder>(hw_encoder));
-  EXPECT_FALSE(wrapper->HasTrustedRateController());
-}
-
-TEST(SoftwareFallbackEncoderTest, HwRateControllerTrustedNoSw) {
-  auto* hw_encoder = new testing::NiceMock<MockVideoEncoder>();
-  EXPECT_CALL(*hw_encoder, HasTrustedRateController())
-      .WillRepeatedly(Return(true));
-
-  std::unique_ptr<VideoEncoder> wrapper =
-      CreateVideoEncoderSoftwareFallbackWrapper(
-          std::unique_ptr<VideoEncoder>(),
-          std::unique_ptr<VideoEncoder>(hw_encoder));
-  EXPECT_TRUE(wrapper->HasTrustedRateController());
-}
-
-TEST(SoftwareFallbackEncoderTest, BothRateControllersTrusted) {
-  auto* sw_encoder = new testing::NiceMock<MockVideoEncoder>();
-  auto* hw_encoder = new testing::NiceMock<MockVideoEncoder>();
-  EXPECT_CALL(*sw_encoder, HasTrustedRateController())
-      .WillRepeatedly(Return(true));
-  EXPECT_CALL(*hw_encoder, HasTrustedRateController())
-      .WillRepeatedly(Return(true));
-
-  std::unique_ptr<VideoEncoder> wrapper =
-      CreateVideoEncoderSoftwareFallbackWrapper(
-          std::unique_ptr<VideoEncoder>(sw_encoder),
-          std::unique_ptr<VideoEncoder>(hw_encoder));
-  EXPECT_TRUE(wrapper->HasTrustedRateController());
-}
-
 }  // namespace webrtc
diff --git a/api/video_codecs/video_encoder.cc b/api/video_codecs/video_encoder.cc
@@ -101,8 +101,4 @@ bool VideoEncoder::SupportsNativeHandle() const {
 const char* VideoEncoder::ImplementationName() const {
   return "unknown";
 }
-
-bool VideoEncoder::HasTrustedRateController() const {
-  return false;
-}
 }  // namespace webrtc
diff --git a/api/video_codecs/video_encoder.h b/api/video_codecs/video_encoder.h
@@ -202,21 +202,6 @@ class VideoEncoder {
 
   virtual bool SupportsNativeHandle() const;
   virtual const char* ImplementationName() const;
-
-  // If this method returns true, the encoder rate controller must perform well
-  // even in difficult situations, and produce close to the specified target
-  // bitrate seen over a reasonable time window, drop frames if necessary in
-  // order to keep the rate correct, and react quickly to changing bitrate
-  // targets.
-  // If this method returns true, we disable the frame dropper in the media
-  // optimization module and rely entirely on the encoder to produce media at a
-  // bitrate that closely matches the target. Any overshooting may result in
-  // delay buildup.
-  // If this method returns false (default behavior), the media opt frame
-  // dropper will drop input frames if it suspect encoder misbehavior.
-  // Misbehavior is common, especially in hardware codecs. Disable media opt at
-  // your own risk.
-  virtual bool HasTrustedRateController() const;
 };
 }  // namespace webrtc
 #endif  // API_VIDEO_CODECS_VIDEO_ENCODER_H_
diff --git a/api/video_codecs/video_encoder_software_fallback_wrapper.cc b/api/video_codecs/video_encoder_software_fallback_wrapper.cc
@@ -93,7 +93,6 @@ class VideoEncoderSoftwareFallbackWrapper final : public VideoEncoder {
   bool SupportsNativeHandle() const override;
   ScalingSettings GetScalingSettings() const override;
   const char* ImplementationName() const override;
-  bool HasTrustedRateController() const override;
 
  private:
   bool InitFallbackEncoder();
@@ -143,8 +142,6 @@ class VideoEncoderSoftwareFallbackWrapper final : public VideoEncoder {
 
   bool forced_fallback_possible_;
   ForcedFallbackParams forced_fallback_;
-
-  const bool trust_rate_controller_;
 };
 
 VideoEncoderSoftwareFallbackWrapper::VideoEncoderSoftwareFallbackWrapper(
@@ -161,11 +158,7 @@ VideoEncoderSoftwareFallbackWrapper::VideoEncoderSoftwareFallbackWrapper(
       encoder_(std::move(hw_encoder)),
       fallback_encoder_(std::move(sw_encoder)),
       callback_(nullptr),
-      forced_fallback_possible_(EnableForcedFallback()),
-      trust_rate_controller_(
-          (encoder_ ? encoder_->HasTrustedRateController() : true) &&
-          (fallback_encoder_ ? fallback_encoder_->HasTrustedRateController()
-                             : true)) {
+      forced_fallback_possible_(EnableForcedFallback()) {
   if (forced_fallback_possible_) {
     GetForcedFallbackParamsFromFieldTrialGroup(
         &forced_fallback_.min_pixels_, &forced_fallback_.max_pixels_,
@@ -326,10 +319,6 @@ const char* VideoEncoderSoftwareFallbackWrapper::ImplementationName() const {
                                : encoder_->ImplementationName();
 }
 
-bool VideoEncoderSoftwareFallbackWrapper::HasTrustedRateController() const {
-  return trust_rate_controller_;
-}
-
 bool VideoEncoderSoftwareFallbackWrapper::IsForcedFallbackActive() const {
   return (forced_fallback_possible_ && use_fallback_encoder_ &&
           forced_fallback_.active_);

diff --git a/media/engine/scopedvideoencoder.cc b/media/engine/scopedvideoencoder.cc
@@ -40,7 +40,6 @@ class ScopedVideoEncoder : public webrtc::VideoEncoder {
   ScalingSettings GetScalingSettings() const override;
   bool SupportsNativeHandle() const override;
   const char* ImplementationName() const override;
-  bool HasTrustedRateController() const override;
 
   ~ScopedVideoEncoder() override;
 
@@ -104,10 +103,6 @@ const char* ScopedVideoEncoder::ImplementationName() const {
   return encoder_->ImplementationName();
 }
 
-bool ScopedVideoEncoder::HasTrustedRateController() const {
-  return encoder_->HasTrustedRateController();
-}
-
 ScopedVideoEncoder::~ScopedVideoEncoder() {
   factory_->DestroyVideoEncoder(encoder_);
 }

diff --git a/media/engine/simulcast_encoder_adapter.cc b/media/engine/simulcast_encoder_adapter.cc
@@ -128,8 +128,7 @@ SimulcastEncoderAdapter::SimulcastEncoderAdapter(VideoEncoderFactory* factory,
       video_format_(format),
       encoded_complete_callback_(nullptr),
       implementation_name_("SimulcastEncoderAdapter"),
-      experimental_boosted_screenshare_qp_(GetScreenshareBoostedQpValue()),
-      trusted_rate_controller_(false) {
+      experimental_boosted_screenshare_qp_(GetScreenshareBoostedQpValue()) {
   RTC_DCHECK(factory_);
 
   // The adapter is typically created on the worker thread, but operated on
@@ -205,7 +204,6 @@ int SimulcastEncoderAdapter::InitEncode(const VideoCodec* inst,
   }
 
   std::string implementation_name;
-  trusted_rate_controller_ = true;
   // Create |number_of_streams| of encoder instances and init them.
   for (int i = 0; i < number_of_streams; ++i) {
     VideoCodec stream_codec;
@@ -252,7 +250,6 @@ int SimulcastEncoderAdapter::InitEncode(const VideoCodec* inst,
     std::unique_ptr<EncodedImageCallback> callback(
         new AdapterEncodedImageCallback(this, i));
     encoder->RegisterEncodeCompleteCallback(callback.get());
-    trusted_rate_controller_ &= encoder->HasTrustedRateController();
     streaminfos_.emplace_back(std::move(encoder), std::move(callback),
                               stream_codec.width, stream_codec.height,
                               send_stream);
@@ -548,9 +545,4 @@ const char* SimulcastEncoderAdapter::ImplementationName() const {
   return implementation_name_.c_str();
 }
 
-bool SimulcastEncoderAdapter::HasTrustedRateController() const {
-  RTC_DCHECK_CALLED_SEQUENTIALLY(&encoder_queue_);
-  return trusted_rate_controller_;
-}
-
 }  // namespace webrtc
diff --git a/media/engine/simulcast_encoder_adapter.h b/media/engine/simulcast_encoder_adapter.h
@@ -65,7 +65,6 @@ class SimulcastEncoderAdapter : public VideoEncoder {
 
   bool SupportsNativeHandle() const override;
   const char* ImplementationName() const override;
-  bool HasTrustedRateController() const override;
 
  private:
   struct StreamInfo {
@@ -115,7 +114,6 @@ class SimulcastEncoderAdapter : public VideoEncoder {
   std::stack<std::unique_ptr<VideoEncoder>> stored_encoders_;
 
   const absl::optional<unsigned int> experimental_boosted_screenshare_qp_;
-  bool trusted_rate_controller_;
 };
 
 }  // namespace webrtc