From ad3b5a5c16ff768def84138147d592ecb669a8cd Mon Sep 17 00:00:00 2001 From: "pbos@webrtc.org" Date: Fri, 24 Oct 2014 09:23:21 +0000 Subject: [PATCH] Move min transmit bitrate to VideoEncoderConfig. min_transmit_bitrate_bps needs to be reconfigurable during a call (since this is currently set only for screensharing through libjingle and can't be set once and for all for the entire Call. R=mflodman@webrtc.org, stefan@webrtc.org BUG=1667 Review URL: https://webrtc-codereview.appspot.com/28779004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@7518 4adac7df-926f-26a2-2b94-8c16560cd09d --- webrtc/config.cc | 28 +++++++++++++++++++++++++ webrtc/config.h | 12 ++++++++++- webrtc/video/call_perf_tests.cc | 4 ++-- webrtc/video/end_to_end_tests.cc | 7 ++++--- webrtc/video/video_send_stream.cc | 11 +++++----- webrtc/video/video_send_stream_tests.cc | 2 +- webrtc/video_send_stream.h | 10 ++------- 7 files changed, 53 insertions(+), 21 deletions(-) diff --git a/webrtc/config.cc b/webrtc/config.cc index e0324b9e4..3d205f17e 100644 --- a/webrtc/config.cc +++ b/webrtc/config.cc @@ -50,4 +50,32 @@ std::string VideoStream::ToString() const { ss << '}'; return ss.str(); } + +std::string VideoEncoderConfig::ToString() const { + std::stringstream ss; + + ss << "{streams: {"; + for (size_t i = 0; i < streams.size(); ++i) { + ss << streams[i].ToString(); + if (i != streams.size() - 1) + ss << "}, {"; + } + ss << '}'; + ss << ", content_type: "; + switch (content_type) { + case kRealtimeVideo: + ss << "kRealtimeVideo"; + break; + case kScreenshare: + ss << "kScreenshare"; + break; + } + ss << ", encoder_specific_settings: "; + ss << (encoder_specific_settings != NULL ? "(ptr)" : "NULL"); + + ss << ", min_transmit_bitrate_bps: " << min_transmit_bitrate_bps; + ss << '}'; + return ss.str(); +} + } // namespace webrtc diff --git a/webrtc/config.h b/webrtc/config.h index 6f3fb1d66..af78d94c8 100644 --- a/webrtc/config.h +++ b/webrtc/config.h @@ -115,11 +115,21 @@ struct VideoEncoderConfig { }; VideoEncoderConfig() - : content_type(kRealtimeVideo), encoder_specific_settings(NULL) {} + : content_type(kRealtimeVideo), + encoder_specific_settings(NULL), + min_transmit_bitrate_bps(0) {} + + std::string ToString() const; std::vector streams; ContentType content_type; void* encoder_specific_settings; + + // Padding will be used up to this bitrate regardless of the bitrate produced + // by the encoder. Padding above what's actually produced by the encoder helps + // maintaining a higher bitrate estimate. Padding will however not be sent + // unless the estimated bandwidth indicates that the link can handle it. + int min_transmit_bitrate_bps; }; } // namespace webrtc diff --git a/webrtc/video/call_perf_tests.cc b/webrtc/video/call_perf_tests.cc index 1087239be..9776fb7e1 100644 --- a/webrtc/video/call_perf_tests.cc +++ b/webrtc/video/call_perf_tests.cc @@ -543,9 +543,9 @@ void CallPerfTest::TestMinTransmitBitrate(bool pad_to_min_bitrate) { std::vector* receive_configs, VideoEncoderConfig* encoder_config) OVERRIDE { if (pad_to_min_bitrate_) { - send_config->rtp.min_transmit_bitrate_bps = kMinTransmitBitrateBps; + encoder_config->min_transmit_bitrate_bps = kMinTransmitBitrateBps; } else { - assert(send_config->rtp.min_transmit_bitrate_bps == 0); + assert(encoder_config->min_transmit_bitrate_bps == 0); } } diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc index ee36dd5db..3d7d3fa75 100644 --- a/webrtc/video/end_to_end_tests.cc +++ b/webrtc/video/end_to_end_tests.cc @@ -1653,15 +1653,16 @@ TEST_F(EndToEndTest, DISABLED_RedundantPayloadsTransmittedOnAllSsrcs) { encoder_config->streams[i].target_bitrate_bps = 15000; encoder_config->streams[i].max_bitrate_bps = 20000; } - // Significantly higher than max bitrates for all video streams -> forcing - // padding to trigger redundant padding on all RTX SSRCs. - send_config->rtp.min_transmit_bitrate_bps = 100000; send_config->rtp.rtx.payload_type = kSendRtxPayloadType; send_config->rtp.rtx.pad_with_redundant_payloads = true; for (size_t i = 0; i < kNumSsrcs; ++i) send_config->rtp.rtx.ssrcs.push_back(kSendRtxSsrcs[i]); + + // Significantly higher than max bitrates for all video streams -> forcing + // padding to trigger redundant padding on all RTX SSRCs. + encoder_config->min_transmit_bitrate_bps = 100000; } virtual void PerformTest() OVERRIDE { diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index 635c37f72..7657250d3 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -66,8 +66,6 @@ std::string VideoSendStream::Config::Rtp::ToString() const { ss << '}'; ss << ", max_packet_size: " << max_packet_size; - if (min_transmit_bitrate_bps != 0) - ss << ", min_transmit_bitrate_bps: " << min_transmit_bitrate_bps; ss << ", extensions: {"; for (size_t i = 0; i < extensions.size(); ++i) { @@ -137,10 +135,6 @@ VideoSendStream::VideoSendStream( assert(config_.rtp.ssrcs.size() > 0); - assert(config_.rtp.min_transmit_bitrate_bps >= 0); - rtp_rtcp_->SetMinTransmitBitrate(channel_, - config_.rtp.min_transmit_bitrate_bps / 1000); - for (size_t i = 0; i < config_.rtp.extensions.size(); ++i) { const std::string& extension = config_.rtp.extensions[i].name; int id = config_.rtp.extensions[i].id; @@ -298,6 +292,7 @@ void VideoSendStream::Stop() { bool VideoSendStream::ReconfigureVideoEncoder( const VideoEncoderConfig& config) { + LOG(LS_INFO) << "(Re)configureVideoEncoder: " << config.ToString(); const std::vector& streams = config.streams; assert(!streams.empty()); assert(config_.rtp.ssrcs.size() >= streams.size()); @@ -407,6 +402,10 @@ bool VideoSendStream::ReconfigureVideoEncoder( if (codec_->SetSendCodec(channel_, video_codec) != 0) return false; + assert(config.min_transmit_bitrate_bps >= 0); + rtp_rtcp_->SetMinTransmitBitrate(channel_, + config.min_transmit_bitrate_bps / 1000); + use_default_bitrate_ = false; return true; } diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc index 6c2fa39f4..79b1cd52d 100644 --- a/webrtc/video/video_send_stream_tests.cc +++ b/webrtc/video/video_send_stream_tests.cc @@ -1075,7 +1075,7 @@ TEST_F(VideoSendStreamTest, MinTransmitBitrateRespectsRemb) { VideoSendStream::Config* send_config, std::vector* receive_configs, VideoEncoderConfig* encoder_config) OVERRIDE { - send_config->rtp.min_transmit_bitrate_bps = kMinTransmitBitrateBps; + encoder_config->min_transmit_bitrate_bps = kMinTransmitBitrateBps; } virtual void PerformTest() OVERRIDE { diff --git a/webrtc/video_send_stream.h b/webrtc/video_send_stream.h index dd2bec127..aa5033afc 100644 --- a/webrtc/video_send_stream.h +++ b/webrtc/video_send_stream.h @@ -60,6 +60,7 @@ class VideoSendStream { struct EncoderSettings { EncoderSettings() : payload_type(-1), encoder(NULL) {} + std::string ToString() const; std::string payload_name; @@ -72,9 +73,7 @@ class VideoSendStream { static const size_t kDefaultMaxPacketSize = 1500 - 40; // TCP over IPv4. struct Rtp { - Rtp() - : max_packet_size(kDefaultMaxPacketSize), - min_transmit_bitrate_bps(0) {} + Rtp() : max_packet_size(kDefaultMaxPacketSize) {} std::string ToString() const; std::vector ssrcs; @@ -82,11 +81,6 @@ class VideoSendStream { // Max RTP packet size delivered to send transport from VideoEngine. size_t max_packet_size; - // Padding will be used up to this bitrate regardless of the bitrate - // produced by the encoder. Padding above what's actually produced by the - // encoder helps maintaining a higher bitrate estimate. - int min_transmit_bitrate_bps; - // RTP header extensions to use for this send stream. std::vector extensions;