From 331d4402fca65fcccf0f4c93958d79f47fe58165 Mon Sep 17 00:00:00 2001 From: "henrik.lundin@webrtc.org" Date: Thu, 21 Nov 2013 14:05:40 +0000 Subject: [PATCH] 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 --- webrtc/video/video_send_stream.cc | 2 ++ webrtc/video/video_send_stream_tests.cc | 15 +++++++++++++-- webrtc/video_engine/vie_codec_impl.cc | 13 ++++++++++++- webrtc/video_engine/vie_encoder.cc | 12 ++++++++++-- webrtc/video_send_stream.h | 2 ++ 5 files changed, 39 insertions(+), 5 deletions(-) diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index cc723b61c2..5b8c23c8c9 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -104,6 +104,8 @@ VideoSendStream::VideoSendStream(newapi::Transport* transport, static_cast(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 67555ec7a5..6570134c2c 100644 --- a/webrtc/video/video_send_stream_tests.cc +++ b/webrtc/video/video_send_stream_tests.cc @@ -596,9 +596,20 @@ TEST_F(VideoSendStreamTest, SuspendBelowMinBitrate) { 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 94e4d7cadd..364862fe2d 100644 --- a/webrtc/video_engine/vie_codec_impl.cc +++ b/webrtc/video_engine/vie_codec_impl.cc @@ -724,7 +724,18 @@ void ViECodecImpl::SuspendBelowMinBitrate(int video_channel) { "%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 65a9e8b505..ecebbd2b3b 100644 --- a/webrtc/video_engine/vie_encoder.cc +++ b/webrtc/video_engine/vie_encoder.cc @@ -525,7 +525,12 @@ bool ViEEncoder::TimeToSendPacket(uint32_t ssrc, } 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 @@ void ViEEncoder::OnNetworkChanged(const uint32_t bitrate_bps, 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 11a37ae1af..85d7519cc1 100644 --- a/webrtc/video_send_stream.h +++ b/webrtc/video_send_stream.h @@ -145,6 +145,8 @@ class VideoSendStream { // 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; };