Cast: Use fixed bitrate and set it once for hardware encoder

Some hardware encoder cannot handle bitrate changing too quickly. The
solution is to have a fixed bitrate and only set it once.

BUG=392086

Review URL: https://codereview.chromium.org/439863003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@287721 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 62 insertions, 13 deletions

diff --git a/media/cast/sender/external_video_encoder_unittest.cc b/media/cast/sender/external_video_encoder_unittest.cc
index 0b446f8..9461a11 100644
--- a/media/cast/sender/external_video_encoder_unittest.cc
+++ b/media/cast/sender/external_video_encoder_unittest.cc
@@ -113,7 +113,8 @@ class ExternalVideoEncoderTest : public ::testing::Test {
                             task_runner_,
                             task_runner_);
 
-    fake_vea_ = new test::FakeVideoEncodeAccelerator(task_runner_);
+    fake_vea_ = new test::FakeVideoEncodeAccelerator(task_runner_,
+                                                     &stored_bitrates_);
     scoped_ptr<VideoEncodeAccelerator> fake_vea(fake_vea_);
     video_encoder_.reset(
         new ExternalVideoEncoder(cast_environment_,
@@ -128,6 +129,7 @@ class ExternalVideoEncoderTest : public ::testing::Test {
 
   base::SimpleTestTickClock* testing_clock_;  // Owned by CastEnvironment.
   test::FakeVideoEncodeAccelerator* fake_vea_;  // Owned by video_encoder_.
+  std::vector<uint32> stored_bitrates_;
   scoped_refptr<TestVideoEncoderCallback> test_video_encoder_callback_;
   VideoSenderConfig video_config_;
   scoped_refptr<test::FakeSingleThreadTaskRunner> task_runner_;
@@ -148,6 +150,7 @@ TEST_F(ExternalVideoEncoderTest, EncodePattern30fpsRunningOutOfAck) {
   base::TimeTicks capture_time;
   capture_time += base::TimeDelta::FromMilliseconds(33);
   test_video_encoder_callback_->SetExpectedResult(0, 0, capture_time);
+  video_encoder_->SetBitRate(2000);
   EXPECT_TRUE(video_encoder_->EncodeVideoFrame(
       video_frame_, capture_time, frame_encoded_callback));
   task_runner_->RunTasks();
@@ -162,6 +165,9 @@ TEST_F(ExternalVideoEncoderTest, EncodePattern30fpsRunningOutOfAck) {
   // We need to run the task to cleanup the GPU instance.
   video_encoder_.reset(NULL);
   task_runner_->RunTasks();
+
+  ASSERT_EQ(1u, stored_bitrates_.size());
+  EXPECT_EQ(2000u, stored_bitrates_[0]);
 }
 
 TEST_F(ExternalVideoEncoderTest, StreamHeader) {
diff --git a/media/cast/sender/video_sender.cc b/media/cast/sender/video_sender.cc
index 598e0d3..7f72038 100644
--- a/media/cast/sender/video_sender.cc
+++ b/media/cast/sender/video_sender.cc
@@ -22,6 +22,20 @@ namespace cast {
 const int kNumAggressiveReportsSentAtStart = 100;
 const int kMinSchedulingDelayMs = 1;
 
+namespace {
+
+// Returns a fixed bitrate value when external video encoder is used.
+// Some hardware encoder shows bad behavior if we set the bitrate too
+// frequently, e.g. quality drop, not abiding by target bitrate, etc.
+// See details: crbug.com/392086.
+size_t GetFixedBitrate(const VideoSenderConfig& video_config) {
+  if (!video_config.use_external_encoder)
+    return 0;
+  return (video_config.min_bitrate + video_config.max_bitrate) / 2;
+}
+
+}  // namespace
+
 VideoSender::VideoSender(
     scoped_refptr<CastEnvironment> cast_environment,
     const VideoSenderConfig& video_config,
@@ -40,6 +54,7 @@ VideoSender::VideoSender(
                    1 + static_cast<int>(target_playout_delay_ *
                                         video_config.max_frame_rate /
                                         base::TimeDelta::FromSeconds(1)))),
+      fixed_bitrate_(GetFixedBitrate(video_config)),
       num_aggressive_rtcp_reports_sent_(0),
       frames_in_encoder_(0),
       last_sent_frame_id_(0),
@@ -117,10 +132,18 @@ void VideoSender::InsertRawVideoFrame(
     return;
   }
 
-  uint32 bitrate = congestion_control_.GetBitrate(
-      capture_time + target_playout_delay_, target_playout_delay_);
-
-  video_encoder_->SetBitRate(bitrate);
+  uint32 bitrate = fixed_bitrate_;
+  if (!bitrate) {
+    bitrate = congestion_control_.GetBitrate(
+        capture_time + target_playout_delay_, target_playout_delay_);
+    DCHECK(bitrate);
+    video_encoder_->SetBitRate(bitrate);
+  } else if (last_send_time_.is_null()) {
+    // Set the fixed bitrate value to codec until a frame is sent. We might
+    // set this value a couple times at the very beginning of the stream but
+    // it is not harmful.
+    video_encoder_->SetBitRate(bitrate);
+  }
 
   if (video_encoder_->EncodeVideoFrame(
           video_frame,
diff --git a/media/cast/sender/video_sender.h b/media/cast/sender/video_sender.h
index 09fac8d..bf96e7b 100644
--- a/media/cast/sender/video_sender.h
+++ b/media/cast/sender/video_sender.h
@@ -92,6 +92,11 @@ class VideoSender : public FrameSender,
   // new frames shall halt.
   const int max_unacked_frames_;
 
+  // If this value is non zero then a fixed value is used for bitrate.
+  // If external video encoder is used then bitrate will be fixed to
+  // (min_bitrate + max_bitrate) / 2.
+  const size_t fixed_bitrate_;
+
   // Encodes media::VideoFrame images into EncodedFrames.  Per configuration,
   // this will point to either the internal software-based encoder or a proxy to
   // a hardware-based encoder.
diff --git a/media/cast/sender/video_sender_unittest.cc b/media/cast/sender/video_sender_unittest.cc
index 38dfa51..06e9dc0 100644
--- a/media/cast/sender/video_sender_unittest.cc
+++ b/media/cast/sender/video_sender_unittest.cc
@@ -174,7 +174,8 @@ class VideoSenderTest : public ::testing::Test {
 
     if (external) {
       scoped_ptr<VideoEncodeAccelerator> fake_vea(
-          new test::FakeVideoEncodeAccelerator(task_runner_));
+          new test::FakeVideoEncodeAccelerator(task_runner_,
+                                               &stored_bitrates_));
       video_sender_.reset(
           new PeerVideoSender(cast_environment_,
                               video_config,
@@ -221,6 +222,7 @@ class VideoSenderTest : public ::testing::Test {
   scoped_ptr<CastTransportSenderImpl> transport_sender_;
   scoped_refptr<test::FakeSingleThreadTaskRunner> task_runner_;
   scoped_ptr<PeerVideoSender> video_sender_;
+  std::vector<uint32> stored_bitrates_;
   scoped_refptr<CastEnvironment> cast_environment_;
   int last_pixel_value_;
 
@@ -247,8 +249,15 @@ TEST_F(VideoSenderTest, ExternalEncoder) {
 
   const base::TimeTicks capture_time = testing_clock_->NowTicks();
   video_sender_->InsertRawVideoFrame(video_frame, capture_time);
-
   task_runner_->RunTasks();
+  video_sender_->InsertRawVideoFrame(video_frame, capture_time);
+  task_runner_->RunTasks();
+  video_sender_->InsertRawVideoFrame(video_frame, capture_time);
+  task_runner_->RunTasks();
+
+  // Fixed bitrate is used for external encoder. Bitrate is only once
+  // to the encoder.
+  EXPECT_EQ(1u, stored_bitrates_.size());
 
   // We need to run the task to cleanup the GPU instance.
   video_sender_.reset(NULL);
diff --git a/media/cast/test/fake_video_encode_accelerator.cc b/media/cast/test/fake_video_encode_accelerator.cc
index d1bfab3..a391a9a 100644
--- a/media/cast/test/fake_video_encode_accelerator.cc
+++ b/media/cast/test/fake_video_encode_accelerator.cc
@@ -17,11 +17,15 @@ static const unsigned int kMinimumInputCount = 1;
 static const size_t kMinimumOutputBufferSize = 123456;
 
 FakeVideoEncodeAccelerator::FakeVideoEncodeAccelerator(
-    const scoped_refptr<base::SingleThreadTaskRunner>& task_runner)
+    const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
+    std::vector<uint32>* stored_bitrates)
     : task_runner_(task_runner),
+      stored_bitrates_(stored_bitrates),
       client_(NULL),
       first_(true),
-      weak_this_factory_(this) {}
+      weak_this_factory_(this) {
+  DCHECK(stored_bitrates_);
+}
 
 FakeVideoEncodeAccelerator::~FakeVideoEncodeAccelerator() {
   weak_this_factory_.InvalidateWeakPtrs();
@@ -80,7 +84,7 @@ void FakeVideoEncodeAccelerator::UseOutputBitstreamBuffer(
 void FakeVideoEncodeAccelerator::RequestEncodingParametersChange(
     uint32 bitrate,
     uint32 framerate) {
-  // No-op.
+  stored_bitrates_->push_back(bitrate);
 }
 
 void FakeVideoEncodeAccelerator::Destroy() { delete this; }
diff --git a/media/cast/test/fake_video_encode_accelerator.h b/media/cast/test/fake_video_encode_accelerator.h
index a4f834f..30e772b 100644
--- a/media/cast/test/fake_video_encode_accelerator.h
+++ b/media/cast/test/fake_video_encode_accelerator.h
@@ -8,6 +8,7 @@
 #include "media/video/video_encode_accelerator.h"
 
 #include <list>
+#include <vector>
 
 #include "base/memory/weak_ptr.h"
 #include "media/base/bitstream_buffer.h"
@@ -23,7 +24,8 @@ namespace test {
 class FakeVideoEncodeAccelerator : public VideoEncodeAccelerator {
  public:
   explicit FakeVideoEncodeAccelerator(
-      const scoped_refptr<base::SingleThreadTaskRunner>& task_runner);
+      const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
+      std::vector<uint32>* stored_bitrates);
   virtual ~FakeVideoEncodeAccelerator();
 
   virtual bool Initialize(media::VideoFrame::Format input_format,
@@ -52,8 +54,8 @@ class FakeVideoEncodeAccelerator : public VideoEncodeAccelerator {
                               size_t payload_size,
                               bool key_frame) const;
 
-  scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
-
+  const scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
+  std::vector<uint32>* const stored_bitrates_;
   VideoEncodeAccelerator::Client* client_;
   bool first_;