Adapt encoder behavior to target bitrate

remoting/protocol/webrtc_video_encoder_factory.cc

Index: remoting/protocol/webrtc_video_encoder_factory.cc
diff --git a/remoting/protocol/webrtc_video_encoder.cc b/remoting/protocol/webrtc_video_encoder_factory.cc
similarity index 86%
rename from remoting/protocol/webrtc_video_encoder.cc
rename to remoting/protocol/webrtc_video_encoder_factory.cc
index 39c27ec22d4830c87a3ddf9865d4842b26268470..a2d7f7c3919542ffeb84ad73be56e8493cb77eee 100644
--- a/remoting/protocol/webrtc_video_encoder.cc
+++ b/remoting/protocol/webrtc_video_encoder_factory.cc
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "remoting/protocol/webrtc_video_encoder.h"
+#include "remoting/protocol/webrtc_video_encoder_factory.h"
 
 #include <algorithm>
 #include <vector>
@@ -44,7 +44,6 @@ int32_t WebRtcVideoEncoder::InitEncode(const webrtc::VideoCodec* codec_settings,
       }
     }
   }
-  target_bitrate_ = codec_settings->startBitrate;
   state_ = kInitialized;
   return WEBRTC_VIDEO_CODEC_OK;
 }
@@ -84,14 +83,14 @@ int32_t WebRtcVideoEncoder::SetChannelParameters(uint32_t packet_loss,
 int32_t WebRtcVideoEncoder::SetRates(uint32_t bitrate, uint32_t framerate) {
   VLOG(1) << "WebRtcVideoEncoder::SetRates bitrate:framerate " << bitrate << ":"
           << framerate;
-  target_bitrate_ = bitrate;
+  if (!target_bitrate_cb_.is_null())
+    target_bitrate_cb_.Run(bitrate);
   // framerate is not expected to be valid given we never report captured
   // frames
   return WEBRTC_VIDEO_CODEC_OK;
 }
 
-int WebRtcVideoEncoder::SendEncodedFrame(int64_t capture_timestamp_ms,
-                                         std::unique_ptr<VideoPacket> frame) {
+int WebRtcVideoEncoder::SendEncodedFrame(std::unique_ptr<VideoPacket> frame) {
   uint8_t* buffer =
       reinterpret_cast<uint8_t*>(const_cast<char*>(frame->data().data()));
   size_t buffer_size = frame->data().size();
@@ -107,8 +106,9 @@ int WebRtcVideoEncoder::SendEncodedFrame(int64_t capture_timestamp_ms,
   encoded_image._completeFrame = true;
   encoded_image._frameType =
       frame->key_frame() ? webrtc::kVideoFrameKey : webrtc::kVideoFrameDelta;
-  encoded_image.capture_time_ms_ = capture_timestamp_ms;
-  encoded_image._timeStamp = static_cast<uint32_t>(capture_timestamp_ms * 90);
+  encoded_image.capture_time_ms_ = frame->capture_time_ms();

[Sergey Ulanov, 2016/05/10 00:00:58]

VideoPacket::capture_time_ms() returns time it took the capture the frame.
EncodedImage::capture_time_ms is the frame's timestamp.

[Irfan, 2016/05/10 16:30:45]

Agree that this is abuse of the VideoPacket capture_time_ms. The right thing
here is to actually fix to avoid using VideoPacket and have a class appropriate
for webrtc use (since the capture timestamp now is correctly moved from the time
of capture). I am going to leave this as is with a comment since this interface
is getting refactored.

+  encoded_image._timeStamp =
+      static_cast<uint32_t>(frame->capture_time_ms() * 90);

[Sergey Ulanov, 2016/05/10 00:00:58]

same here. capture_time_ms() is not timestamp

 
   webrtc::CodecSpecificInfo codec_specific_info;
   memset(&codec_specific_info, 0, sizeof(codec_specific_info));
@@ -144,6 +144,12 @@ void WebRtcVideoEncoder::SetKeyFrameRequestCallback(
   key_frame_request_ = key_frame_request;
 }
 
+void WebRtcVideoEncoder::SetTargetBitrateCallback(
+    const TargetBitrateCallback& target_bitrate_cb) {
+  base::AutoLock lock(lock_);
+  target_bitrate_cb_ = target_bitrate_cb;
+}
+
 WebRtcVideoEncoderFactory::WebRtcVideoEncoderFactory() {
   // TODO(isheriff): These do not really affect anything internally
   // in webrtc.
@@ -162,6 +168,7 @@ webrtc::VideoEncoder* WebRtcVideoEncoderFactory::CreateVideoEncoder(
   WebRtcVideoEncoder* encoder = new WebRtcVideoEncoder(type);
   base::AutoLock lock(lock_);
   encoder->SetKeyFrameRequestCallback(key_frame_request_);
+  encoder->SetTargetBitrateCallback(target_bitrate_cb_);
   VLOG(1) << "Created " << encoder;
   encoders_.push_back(base::WrapUnique(encoder));
   return encoder;
@@ -196,14 +203,12 @@ void WebRtcVideoEncoderFactory::DestroyVideoEncoder(
 }
 
 int WebRtcVideoEncoderFactory::SendEncodedFrame(
-    int64_t capture_timestamp_ms,
     std::unique_ptr<VideoPacket> frame) {
   if (encoders_.size() != 1) {
     LOG(ERROR) << "Unexpected number of encoders " << encoders_.size();
     return -1;
   }
-  return encoders_.front()->SendEncodedFrame(capture_timestamp_ms,
-                                             std::move(frame));
+  return encoders_.front()->SendEncodedFrame(std::move(frame));
 }
 
 void WebRtcVideoEncoderFactory::SetKeyFrameRequestCallback(
@@ -218,4 +223,17 @@ void WebRtcVideoEncoderFactory::SetKeyFrameRequestCallback(
                << encoders_.size();
   }
 }
+
+void WebRtcVideoEncoderFactory::SetTargetBitrateCallback(
+    const TargetBitrateCallback& target_bitrate_cb) {
+  base::AutoLock lock(lock_);
+  target_bitrate_cb_ = target_bitrate_cb;
+  if (encoders_.size() == 1) {
+    encoders_.front()->SetTargetBitrateCallback(target_bitrate_cb);
+  } else {
+    LOG(ERROR) << "Dropping target bitrate request callback with unexpected"
+                  " number of encoders"
+               << encoders_.size();
+  }
+}
 }  // namespace remoting