Connect pacer/padding to SuspendBelowMinBitrate
The suspend function must not be engaged unless padding is also enabled.
This CL makes the connection so that the pacer and padding is enabled
when SuspendBelowMinBitrate is.
Had to change the unit test to make it aware of the padding packets.
BUG=2606
R=mflodman@webrtc.org, pbos@webrtc.org, stefan@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/4089004
git-svn-id: http://webrtc.googlecode.com/svn/trunk@5153 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc
index cc723b6..5b8c23c 100644
--- a/webrtc/video/video_send_stream.cc
+++ b/webrtc/video/video_send_stream.cc
@@ -104,6 +104,8 @@
static_cast<unsigned char>(i));
}
}
+ if (config_.suspend_below_min_bitrate)
+ config_.pacing = true;
rtp_rtcp_->SetTransmissionSmoothingStatus(channel_, config_.pacing);
if (!config_.rtp.rtx.ssrcs.empty()) {
assert(config_.rtp.rtx.ssrcs.size() == config_.rtp.ssrcs.size());
diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc
index 67555ec..6570134 100644
--- a/webrtc/video/video_send_stream_tests.cc
+++ b/webrtc/video/video_send_stream_tests.cc
@@ -596,9 +596,20 @@
SendRtcpFeedback(low_remb_bps_);
test_state_ = kDuringSuspend;
} else if (test_state_ == kDuringSuspend) {
- suspended_frame_count_ = 0;
+ if (header.paddingLength == 0) {
+ // Received non-padding packet during suspension period. Reset the
+ // counter.
+ // TODO(hlundin): We should probably make this test more advanced in
+ // the future, so that it verifies that the bitrate can go below the
+ // min_bitrate. This requires that the fake encoder sees the
+ // min_bitrate, and never goes below it. See WebRTC Issue 2655.
+ suspended_frame_count_ = 0;
+ }
} else if (test_state_ == kWaitingForPacket) {
- observation_complete_->Set();
+ if (header.paddingLength == 0) {
+ // Non-padding packet observed. Test is complete.
+ observation_complete_->Set();
+ }
}
return SEND_PACKET;
diff --git a/webrtc/video_engine/vie_codec_impl.cc b/webrtc/video_engine/vie_codec_impl.cc
index 94e4d7c..364862f 100644
--- a/webrtc/video_engine/vie_codec_impl.cc
+++ b/webrtc/video_engine/vie_codec_impl.cc
@@ -724,7 +724,18 @@
"%s: No encoder %d", __FUNCTION__, video_channel);
return;
}
- return vie_encoder->SuspendBelowMinBitrate();
+ vie_encoder->SuspendBelowMinBitrate();
+ ViEChannel* vie_channel = cs.Channel(video_channel);
+ if (!vie_channel) {
+ WEBRTC_TRACE(kTraceError, kTraceVideo,
+ ViEId(shared_data_->instance_id(), video_channel),
+ "%s: No channel %d", __FUNCTION__, video_channel);
+ return;
+ }
+ // Must enable pacing when enabling SuspendBelowMinBitrate. Otherwise, no
+ // padding will be sent when the video is suspended so the video will be
+ // unable to recover.
+ vie_channel->SetTransmissionSmoothingStatus(true);
}
bool ViECodecImpl::CodecValid(const VideoCodec& video_codec) {
diff --git a/webrtc/video_engine/vie_encoder.cc b/webrtc/video_engine/vie_encoder.cc
index 65a9e8b..ecebbd2 100644
--- a/webrtc/video_engine/vie_encoder.cc
+++ b/webrtc/video_engine/vie_encoder.cc
@@ -525,7 +525,12 @@
}
int ViEEncoder::TimeToSendPadding(int bytes) {
- if (send_padding_) {
+ bool send_padding;
+ {
+ CriticalSectionScoped cs(data_cs_.get());
+ send_padding = send_padding_ || video_suspended_;
+ }
+ if (send_padding) {
return default_rtp_rtcp_->TimeToSendPadding(bytes);
}
return 0;
@@ -1109,7 +1114,10 @@
default_rtp_rtcp_->SetTargetSendBitrate(stream_bitrates);
if (video_is_suspended != video_suspended_) {
// State changed now. Send callback to inform about that.
- video_suspended_ = video_is_suspended;
+ {
+ CriticalSectionScoped cs(data_cs_.get());
+ video_suspended_ = video_is_suspended;
+ }
if (codec_observer_) {
WEBRTC_TRACE(webrtc::kTraceInfo, webrtc::kTraceVideo,
ViEId(engine_id_, channel_id_),
diff --git a/webrtc/video_send_stream.h b/webrtc/video_send_stream.h
index 11a37ae..85d7519 100644
--- a/webrtc/video_send_stream.h
+++ b/webrtc/video_send_stream.h
@@ -145,6 +145,8 @@
// True if the stream should be suspended when the available bitrate fall
// below the minimum configured bitrate. If this variable is false, the
// stream may send at a rate higher than the estimated available bitrate.
+ // Enabling suspend_below_min_bitrate will also enable pacing and padding,
+ // otherwise, the video will be unable to recover from suspension.
bool suspend_below_min_bitrate;
};
