Remove ViEEncoder::Pause / Start
This cl change so that VideoSendStream::Start adds the stream as a BitrateObserver and VideoSendStream::Stop removes the stream as observer.
That also means that start will trigger a VideoEncoder::SetRate call with the most recent bitrate estimate.
VideoSendStream::Stop will trigger a VideoEncoder::SetRate with bitrate = 0.
BUG=webrtc:5687 b/28636240
Review-Url: https://codereview.webrtc.org/2070343002
Cr-Commit-Position: refs/heads/master@{#13192}
diff --git a/webrtc/modules/video_coding/generic_encoder.cc b/webrtc/modules/video_coding/generic_encoder.cc
index abc6369..176a14f 100644
--- a/webrtc/modules/video_coding/generic_encoder.cc
+++ b/webrtc/modules/video_coding/generic_encoder.cc
@@ -44,12 +44,6 @@
int32_t number_of_cores,
size_t max_payload_size) {
TRACE_EVENT0("webrtc", "VCMGenericEncoder::InitEncode");
- {
- rtc::CritScope lock(¶ms_lock_);
- encoder_params_.target_bitrate = settings->startBitrate * 1000;
- encoder_params_.input_frame_rate = settings->maxFramerate;
- }
-
is_screenshare_ = settings->mode == VideoCodecMode::kScreensharing;
if (encoder_->InitEncode(settings, number_of_cores, max_payload_size) != 0) {
LOG(LS_ERROR) << "Failed to initialize the encoder associated with "
diff --git a/webrtc/modules/video_coding/protection_bitrate_calculator.cc b/webrtc/modules/video_coding/protection_bitrate_calculator.cc
index c92c3c4..ba93442 100644
--- a/webrtc/modules/video_coding/protection_bitrate_calculator.cc
+++ b/webrtc/modules/video_coding/protection_bitrate_calculator.cc
@@ -8,7 +8,7 @@
* be found in the AUTHORS file in the root of the source tree.
*/
-#include <webrtc/modules/video_coding/protection_bitrate_calculator.h>
+#include "webrtc/modules/video_coding/protection_bitrate_calculator.h"
namespace webrtc {
diff --git a/webrtc/modules/video_coding/protection_bitrate_calculator.h b/webrtc/modules/video_coding/protection_bitrate_calculator.h
index cdbab10..01edc6c 100644
--- a/webrtc/modules/video_coding/protection_bitrate_calculator.h
+++ b/webrtc/modules/video_coding/protection_bitrate_calculator.h
@@ -59,7 +59,6 @@
int actual_framerate,
uint8_t fraction_lost,
int64_t round_trip_time_ms);
-
// Informs of encoded output.
void UpdateWithEncodedData(const EncodedImage& encoded_image);
diff --git a/webrtc/modules/video_coding/protection_bitrate_calculator_unittest.cc b/webrtc/modules/video_coding/protection_bitrate_calculator_unittest.cc
index 667dd6a..7855bbd 100644
--- a/webrtc/modules/video_coding/protection_bitrate_calculator_unittest.cc
+++ b/webrtc/modules/video_coding/protection_bitrate_calculator_unittest.cc
@@ -86,4 +86,15 @@
EXPECT_EQ(kMaxBitrateBps / 2, target_bitrate);
}
+TEST_F(ProtectionBitrateCalculatorTest, NoProtection) {
+ static const uint32_t kMaxBitrateBps = 130000;
+
+ media_opt_.SetProtectionMethod(false /*enable_fec*/, false /* enable_nack */);
+ media_opt_.SetEncodingData(kCodecBitrateBps, 640, 480, 30, 1, 1000);
+
+ uint32_t target_bitrate =
+ media_opt_.SetTargetRates(kMaxBitrateBps, 30, 128, 100);
+ EXPECT_EQ(kMaxBitrateBps, target_bitrate);
+}
+
} // namespace webrtc
diff --git a/webrtc/modules/video_coding/video_coding_impl.h b/webrtc/modules/video_coding/video_coding_impl.h
index 2aabf05..6f48a23 100644
--- a/webrtc/modules/video_coding/video_coding_impl.h
+++ b/webrtc/modules/video_coding/video_coding_impl.h
@@ -96,7 +96,7 @@
void Process() override;
private:
- void SetEncoderParameters(EncoderParameters params)
+ void SetEncoderParameters(EncoderParameters params, bool has_internal_source)
EXCLUSIVE_LOCKS_REQUIRED(encoder_crit_);
Clock* const clock_;
diff --git a/webrtc/modules/video_coding/video_sender.cc b/webrtc/modules/video_coding/video_sender.cc
index e2c0d1f..d7d8110 100644
--- a/webrtc/modules/video_coding/video_sender.cc
+++ b/webrtc/modules/video_coding/video_sender.cc
@@ -219,19 +219,23 @@
if (encoder_has_internal_source) {
rtc::CritScope cs(&encoder_crit_);
if (_encoder) {
- SetEncoderParameters(encoder_params);
+ SetEncoderParameters(encoder_params, encoder_has_internal_source);
}
}
return VCM_OK;
}
-void VideoSender::SetEncoderParameters(EncoderParameters params) {
+void VideoSender::SetEncoderParameters(EncoderParameters params,
+ bool has_internal_source) {
// |target_bitrate == 0 | means that the network is down or the send pacer is
- // full.
- // TODO(perkj): Consider setting |target_bitrate| == 0 to the encoders.
- // Especially if |encoder_has_internal_source_ | == true.
- if (params.target_bitrate == 0)
+ // full. We currently only report this if the encoder has an internal source.
+ // If the encoder does not have an internal source, higher levels are expected
+ // to not call AddVideoFrame. We do this since its unclear how current
+ // encoder implementations behave when given a zero target bitrate.
+ // TODO(perkj): Make sure all known encoder implementations handle zero
+ // target bitrate and remove this check.
+ if (!has_internal_source && params.target_bitrate == 0)
return;
if (params.input_frame_rate == 0) {
@@ -258,15 +262,17 @@
const CodecSpecificInfo* codecSpecificInfo) {
EncoderParameters encoder_params;
std::vector<FrameType> next_frame_types;
+ bool encoder_has_internal_source = false;
{
rtc::CritScope lock(¶ms_crit_);
encoder_params = encoder_params_;
next_frame_types = next_frame_types_;
+ encoder_has_internal_source = encoder_has_internal_source_;
}
rtc::CritScope lock(&encoder_crit_);
if (_encoder == nullptr)
return VCM_UNINITIALIZED;
- SetEncoderParameters(encoder_params);
+ SetEncoderParameters(encoder_params, encoder_has_internal_source);
if (_mediaOpt.DropFrame()) {
LOG(LS_VERBOSE) << "Drop Frame "
<< "target bitrate " << encoder_params.target_bitrate