From 3f83f9cae99b605aee71b46839e902653baeb3f6 Mon Sep 17 00:00:00 2001 From: "pbos@webrtc.org" Date: Thu, 13 Mar 2014 12:52:27 +0000 Subject: Implement minimum transmit bitrate. Utilizing minimum transmission bitrate prevents low remote bitrate estimates (bitrate estimation dips) when encoding non-complex content such as screenshare of a static image even though there's nothing wrong with the link. Requires pacing to be enabled for now, pending issue 3036. BUG=3014 R=mflodman@webrtc.org, stefan@webrtc.org Review URL: https://webrtc-codereview.appspot.com/9719004 git-svn-id: http://webrtc.googlecode.com/svn/trunk/webrtc@5694 4adac7df-926f-26a2-2b94-8c16560cd09d --- test/fake_encoder.cc | 29 +++++--- test/fake_encoder.h | 8 +-- test/rtp_rtcp_observer.h | 4 +- video/call_perf_tests.cc | 133 ++++++++++++++++++++++++++++++++++++ video/call_tests.cc | 1 + video/video_send_stream.cc | 4 ++ video_engine/include/vie_rtp_rtcp.h | 9 +++ video_engine/vie_encoder.cc | 33 ++++++--- video_engine/vie_encoder.h | 6 +- video_engine/vie_rtp_rtcp_impl.cc | 10 +++ video_engine/vie_rtp_rtcp_impl.h | 2 + video_send_stream.h | 9 ++- 12 files changed, 222 insertions(+), 26 deletions(-) diff --git a/test/fake_encoder.cc b/test/fake_encoder.cc index f4e5227e..fc8712e5 100644 --- a/test/fake_encoder.cc +++ b/test/fake_encoder.cc @@ -19,6 +19,7 @@ FakeEncoder::FakeEncoder(Clock* clock) : clock_(clock), callback_(NULL), target_bitrate_kbps_(0), + max_target_bitrate_kbps_(-1), last_encode_time_ms_(0) { // Generate some arbitrary not-all-zero data for (size_t i = 0; i < sizeof(encoded_buffer_); ++i) { @@ -62,6 +63,11 @@ void FakeEncoder::SetCodecSettings(VideoCodec* codec, strcpy(codec->plName, "FAKE"); } +void FakeEncoder::SetMaxBitrate(int max_kbps) { + assert(max_kbps >= -1); // max_kbps == -1 disables it. + max_target_bitrate_kbps_ = max_kbps; +} + int32_t FakeEncoder::InitEncode(const VideoCodec* config, int32_t number_of_cores, uint32_t max_payload_size) { @@ -75,19 +81,22 @@ int32_t FakeEncoder::Encode( const CodecSpecificInfo* codec_specific_info, const std::vector* frame_types) { assert(config_.maxFramerate > 0); - int delta_since_last_encode = 1000 / config_.maxFramerate; + int time_since_last_encode_ms = 1000 / config_.maxFramerate; int64_t time_now_ms = clock_->TimeInMilliseconds(); if (last_encode_time_ms_ > 0) { // For all frames but the first we can estimate the display time by looking // at the display time of the previous frame. - delta_since_last_encode = time_now_ms - last_encode_time_ms_; + time_since_last_encode_ms = time_now_ms - last_encode_time_ms_; } - int bits_available = target_bitrate_kbps_ * delta_since_last_encode; + int bits_available = target_bitrate_kbps_ * time_since_last_encode_ms; int min_bits = - config_.simulcastStream[0].minBitrate * delta_since_last_encode; + config_.simulcastStream[0].minBitrate * time_since_last_encode_ms; if (bits_available < min_bits) bits_available = min_bits; + int max_bits = max_target_bitrate_kbps_ * time_since_last_encode_ms; + if (max_bits > 0 && max_bits < bits_available) + bits_available = max_bits; last_encode_time_ms_ = time_now_ms; for (int i = 0; i < config_.numberOfSimulcastStreams; ++i) { @@ -95,10 +104,10 @@ int32_t FakeEncoder::Encode( memset(&specifics, 0, sizeof(specifics)); specifics.codecType = kVideoCodecGeneric; specifics.codecSpecific.generic.simulcast_idx = i; - int min_stream_bits = config_.simulcastStream[i].minBitrate * - delta_since_last_encode; - int max_stream_bits = config_.simulcastStream[i].maxBitrate * - delta_since_last_encode; + int min_stream_bits = + config_.simulcastStream[i].minBitrate * time_since_last_encode_ms; + int max_stream_bits = + config_.simulcastStream[i].maxBitrate * time_since_last_encode_ms; int stream_bits = (bits_available > max_stream_bits) ? max_stream_bits : bits_available; int stream_bytes = (stream_bits + 7) / 8; @@ -110,7 +119,8 @@ int32_t FakeEncoder::Encode( encoded._timeStamp = input_image.timestamp(); encoded.capture_time_ms_ = input_image.render_time_ms(); encoded._frameType = (*frame_types)[i]; - if (min_stream_bits > bits_available) { + // Always encode something on the first frame. + if (min_stream_bits > bits_available && i > 0) { encoded._length = 0; encoded._frameType = kSkipFrame; } @@ -138,5 +148,6 @@ int32_t FakeEncoder::SetRates(uint32_t new_target_bitrate, uint32_t framerate) { target_bitrate_kbps_ = new_target_bitrate; return 0; } + } // namespace test } // namespace webrtc diff --git a/test/fake_encoder.h b/test/fake_encoder.h index c57a4dce..e2d8d6b4 100644 --- a/test/fake_encoder.h +++ b/test/fake_encoder.h @@ -25,23 +25,20 @@ class FakeEncoder : public VideoEncoder { virtual ~FakeEncoder(); static void SetCodecSettings(VideoCodec* codec, size_t num_streams); + // Sets max bitrate. Not thread-safe, call before registering the encoder. + void SetMaxBitrate(int max_kbps); virtual int32_t InitEncode(const VideoCodec* config, int32_t number_of_cores, uint32_t max_payload_size) OVERRIDE; - virtual int32_t Encode( const I420VideoFrame& input_image, const CodecSpecificInfo* codec_specific_info, const std::vector* frame_types) OVERRIDE; - virtual int32_t RegisterEncodeCompleteCallback( EncodedImageCallback* callback) OVERRIDE; - virtual int32_t Release() OVERRIDE; - virtual int32_t SetChannelParameters(uint32_t packet_loss, int rtt) OVERRIDE; - virtual int32_t SetRates(uint32_t new_target_bitrate, uint32_t framerate) OVERRIDE; @@ -50,6 +47,7 @@ class FakeEncoder : public VideoEncoder { VideoCodec config_; EncodedImageCallback* callback_; int target_bitrate_kbps_; + int max_target_bitrate_kbps_; int64_t last_encode_time_ms_; uint8_t encoded_buffer_[100000]; }; diff --git a/test/rtp_rtcp_observer.h b/test/rtp_rtcp_observer.h index 5ed9a3f3..00422cce 100644 --- a/test/rtp_rtcp_observer.h +++ b/test/rtp_rtcp_observer.h @@ -33,8 +33,8 @@ class RtpRtcpObserver { return &receive_transport_; } - void SetReceivers(PacketReceiver* send_transport_receiver, - PacketReceiver* receive_transport_receiver) { + virtual void SetReceivers(PacketReceiver* send_transport_receiver, + PacketReceiver* receive_transport_receiver) { send_transport_.SetReceiver(send_transport_receiver); receive_transport_.SetReceiver(receive_transport_receiver); } diff --git a/video/call_perf_tests.cc b/video/call_perf_tests.cc index 4c0f5ed2..31cfab5f 100644 --- a/video/call_perf_tests.cc +++ b/video/call_perf_tests.cc @@ -50,6 +50,7 @@ class CallPerfTest : public ::testing::Test { public: CallPerfTest() : send_stream_(NULL), fake_encoder_(Clock::GetRealTimeClock()) {} + protected: VideoSendStream::Config GetSendTestConfig(Call* call) { VideoSendStream::Config config = call->GetDefaultSendConfig(); @@ -60,6 +61,7 @@ class CallPerfTest : public ::testing::Test { config.codec.plType = kSendPayloadType; return config; } + void RunVideoSendTest(Call* call, const VideoSendStream::Config& config, test::RtpRtcpObserver* observer) { @@ -78,6 +80,8 @@ class CallPerfTest : public ::testing::Test { call->DestroyVideoSendStream(send_stream_); } + void TestMinTransmitBitrate(bool pad_to_min_bitrate); + VideoSendStream* send_stream_; test::FakeEncoder fake_encoder_; }; @@ -388,4 +392,133 @@ TEST_F(CallPerfTest, RegisterCpuOveruseObserver) { VideoSendStream::Config send_config = GetSendTestConfig(call.get()); RunVideoSendTest(call.get(), send_config, &observer); } + +void CallPerfTest::TestMinTransmitBitrate(bool pad_to_min_bitrate) { + static const int kMaxEncodeBitrateKbps = 30; + static const int kMinTransmitBitrateKbps = 150; + static const int kMinAcceptableTransmitBitrate = 130; + static const int kMaxAcceptableTransmitBitrate = 170; + static const int kNumBitrateObservationsInRange = 100; + class BitrateObserver : public test::RtpRtcpObserver, public PacketReceiver { + public: + explicit BitrateObserver(bool using_min_transmit_bitrate) + : test::RtpRtcpObserver(kLongTimeoutMs), + send_stream_(NULL), + send_transport_receiver_(NULL), + using_min_transmit_bitrate_(using_min_transmit_bitrate), + num_bitrate_observations_in_range_(0) {} + + virtual void SetReceivers(PacketReceiver* send_transport_receiver, + PacketReceiver* receive_transport_receiver) + OVERRIDE { + send_transport_receiver_ = send_transport_receiver; + test::RtpRtcpObserver::SetReceivers(this, receive_transport_receiver); + } + + void SetSendStream(VideoSendStream* send_stream) { + send_stream_ = send_stream; + } + + private: + virtual bool DeliverPacket(const uint8_t* packet, size_t length) OVERRIDE { + VideoSendStream::Stats stats = send_stream_->GetStats(); + if (stats.substreams.size() > 0) { + assert(stats.substreams.size() == 1); + int bitrate_kbps = stats.substreams.begin()->second.bitrate_bps / 1000; + if (bitrate_kbps > 0) { + test::PrintResult( + "bitrate_stats_", + (using_min_transmit_bitrate_ ? "min_transmit_bitrate" + : "without_min_transmit_bitrate"), + "bitrate_kbps", + static_cast(bitrate_kbps), + "kbps", + false); + if (using_min_transmit_bitrate_) { + if (bitrate_kbps > kMinAcceptableTransmitBitrate && + bitrate_kbps < kMaxAcceptableTransmitBitrate) { + ++num_bitrate_observations_in_range_; + } + } else { + // Expect bitrate stats to roughly match the max encode bitrate. + if (bitrate_kbps > kMaxEncodeBitrateKbps - 5 && + bitrate_kbps < kMaxEncodeBitrateKbps + 5) { + ++num_bitrate_observations_in_range_; + } + } + if (num_bitrate_observations_in_range_ == + kNumBitrateObservationsInRange) + observation_complete_->Set(); + } + } + return send_transport_receiver_->DeliverPacket(packet, length); + } + + VideoSendStream* send_stream_; + PacketReceiver* send_transport_receiver_; + const bool using_min_transmit_bitrate_; + int num_bitrate_observations_in_range_; + } observer(pad_to_min_bitrate); + + scoped_ptr sender_call( + Call::Create(Call::Config(observer.SendTransport()))); + scoped_ptr receiver_call( + Call::Create(Call::Config(observer.ReceiveTransport()))); + + VideoSendStream::Config send_config = GetSendTestConfig(sender_call.get()); + fake_encoder_.SetMaxBitrate(kMaxEncodeBitrateKbps); + + observer.SetReceivers(receiver_call->Receiver(), sender_call->Receiver()); + + send_config.pacing = true; + if (pad_to_min_bitrate) { + send_config.rtp.min_transmit_bitrate_kbps = kMinTransmitBitrateKbps; + } else { + assert(send_config.rtp.min_transmit_bitrate_kbps == 0); + } + + VideoReceiveStream::Config receive_config = + receiver_call->GetDefaultReceiveConfig(); + receive_config.codecs.clear(); + receive_config.codecs.push_back(send_config.codec); + test::FakeDecoder fake_decoder; + ExternalVideoDecoder decoder; + decoder.decoder = &fake_decoder; + decoder.payload_type = send_config.codec.plType; + receive_config.external_decoders.push_back(decoder); + receive_config.rtp.remote_ssrc = send_config.rtp.ssrcs[0]; + receive_config.rtp.local_ssrc = kReceiverLocalSsrc; + + VideoSendStream* send_stream = + sender_call->CreateVideoSendStream(send_config); + VideoReceiveStream* receive_stream = + receiver_call->CreateVideoReceiveStream(receive_config); + scoped_ptr capturer( + test::FrameGeneratorCapturer::Create(send_stream->Input(), + send_config.codec.width, + send_config.codec.height, + 30, + Clock::GetRealTimeClock())); + observer.SetSendStream(send_stream); + receive_stream->StartReceiving(); + send_stream->StartSending(); + capturer->Start(); + + EXPECT_EQ(kEventSignaled, observer.Wait()) + << "Timeout while waiting for send-bitrate stats."; + + send_stream->StopSending(); + receive_stream->StopReceiving(); + observer.StopSending(); + capturer->Stop(); + sender_call->DestroyVideoSendStream(send_stream); + receiver_call->DestroyVideoReceiveStream(receive_stream); +} + +TEST_F(CallPerfTest, PadsToMinTransmitBitrate) { TestMinTransmitBitrate(true); } + +TEST_F(CallPerfTest, NoPadWithoutMinTransmitBitrate) { + TestMinTransmitBitrate(false); +} + } // namespace webrtc diff --git a/video/call_tests.cc b/video/call_tests.cc index a945f64c..6e922d3d 100644 --- a/video/call_tests.cc +++ b/video/call_tests.cc @@ -1475,4 +1475,5 @@ TEST_F(CallTest, ReceiverReferenceTimeReportEnabled) { TEST_F(CallTest, ReceiverReferenceTimeReportDisabled) { TestXrReceiverReferenceTimeReport(false); } + } // namespace webrtc diff --git a/video/video_send_stream.cc b/video/video_send_stream.cc index 8ec85759..5cacb565 100644 --- a/video/video_send_stream.cc +++ b/video/video_send_stream.cc @@ -49,6 +49,10 @@ VideoSendStream::VideoSendStream(newapi::Transport* transport, config_.pacing = true; rtp_rtcp_->SetTransmissionSmoothingStatus(channel_, config_.pacing); + assert(config_.rtp.min_transmit_bitrate_kbps >= 0); + rtp_rtcp_->SetMinTransmitBitrate(channel_, + config_.rtp.min_transmit_bitrate_kbps); + 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; diff --git a/video_engine/include/vie_rtp_rtcp.h b/video_engine/include/vie_rtp_rtcp.h index a358e480..e4ef74a0 100644 --- a/video_engine/include/vie_rtp_rtcp.h +++ b/video_engine/include/vie_rtp_rtcp.h @@ -266,6 +266,15 @@ class WEBRTC_DLLEXPORT ViERTP_RTCP { virtual int SetTransmissionSmoothingStatus(int video_channel, bool enable) = 0; + // Sets a minimal bitrate which will be padded to when the encoder doesn't + // produce enough bitrate. + // TODO(pbos): Remove default implementation when libjingle's + // FakeWebRtcVideoEngine is updated. + virtual int SetMinTransmitBitrate(int video_channel, + int min_transmit_bitrate_kbps) { + return -1; + }; + // This function returns our locally created statistics of the received RTP // stream. virtual int GetReceiveChannelRtcpStatistics(const int video_channel, diff --git a/video_engine/vie_encoder.cc b/video_engine/vie_encoder.cc index 940358c6..22a74fb7 100644 --- a/video_engine/vie_encoder.cc +++ b/video_engine/vie_encoder.cc @@ -149,6 +149,7 @@ ViEEncoder::ViEEncoder(int32_t engine_id, bitrate_controller_(bitrate_controller), time_of_last_incoming_frame_ms_(0), send_padding_(false), + min_transmit_bitrate_kbps_(0), target_delay_ms_(0), network_is_transmitting_(true), encoder_paused_(false), @@ -459,9 +460,14 @@ int32_t ViEEncoder::SetEncoder(const webrtc::VideoCodec& video_codec) { kTransmissionMaxBitrateMultiplier * video_codec.maxBitrate * 1000); - paced_sender_->UpdateBitrate(video_codec.startBitrate, - video_codec.startBitrate, - video_codec.startBitrate); + CriticalSectionScoped crit(data_cs_.get()); + int pad_up_to_bitrate_kbps = video_codec.startBitrate; + if (pad_up_to_bitrate_kbps < min_transmit_bitrate_kbps_) + pad_up_to_bitrate_kbps = min_transmit_bitrate_kbps_; + + paced_sender_->UpdateBitrate( + video_codec.startBitrate, pad_up_to_bitrate_kbps, pad_up_to_bitrate_kbps); + return 0; } @@ -527,7 +533,8 @@ int ViEEncoder::TimeToSendPadding(int bytes) { bool send_padding; { CriticalSectionScoped cs(data_cs_.get()); - send_padding = send_padding_ || video_suspended_; + send_padding = + send_padding_ || video_suspended_ || min_transmit_bitrate_kbps_ > 0; } if (send_padding) { return default_rtp_rtcp_->TimeToSendPadding(bytes); @@ -1028,6 +1035,12 @@ bool ViEEncoder::SetSsrcs(const std::list& ssrcs) { return true; } +void ViEEncoder::SetMinTransmitBitrate(int min_transmit_bitrate_kbps) { + assert(min_transmit_bitrate_kbps >= 0); + CriticalSectionScoped crit(data_cs_.get()); + min_transmit_bitrate_kbps_ = min_transmit_bitrate_kbps; +} + // Called from ViEBitrateObserver. void ViEEncoder::OnNetworkChanged(const uint32_t bitrate_bps, const uint8_t fraction_lost, @@ -1091,17 +1104,21 @@ void ViEEncoder::OnNetworkChanged(const uint32_t bitrate_bps, max_padding_bitrate_kbps = 0; } - paced_sender_->UpdateBitrate(bitrate_kbps, - max_padding_bitrate_kbps, - pad_up_to_bitrate_kbps); - default_rtp_rtcp_->SetTargetSendBitrate(stream_bitrates); { CriticalSectionScoped cs(data_cs_.get()); + if (pad_up_to_bitrate_kbps < min_transmit_bitrate_kbps_) + pad_up_to_bitrate_kbps = min_transmit_bitrate_kbps_; + if (max_padding_bitrate_kbps < min_transmit_bitrate_kbps_) + max_padding_bitrate_kbps = min_transmit_bitrate_kbps_; + paced_sender_->UpdateBitrate( + bitrate_kbps, max_padding_bitrate_kbps, pad_up_to_bitrate_kbps); + default_rtp_rtcp_->SetTargetSendBitrate(stream_bitrates); if (video_suspended_ == video_is_suspended) return; video_suspended_ = video_is_suspended; } // State changed, inform codec observer. + CriticalSectionScoped crit(callback_cs_.get()); if (codec_observer_) { WEBRTC_TRACE(webrtc::kTraceInfo, webrtc::kTraceVideo, ViEId(engine_id_, channel_id_), diff --git a/video_engine/vie_encoder.h b/video_engine/vie_encoder.h index d3271c31..8e22ecf4 100644 --- a/video_engine/vie_encoder.h +++ b/video_engine/vie_encoder.h @@ -20,6 +20,7 @@ #include "webrtc/modules/video_coding/main/interface/video_coding_defines.h" #include "webrtc/modules/video_processing/main/interface/video_processing.h" #include "webrtc/system_wrappers/interface/scoped_ptr.h" +#include "webrtc/system_wrappers/interface/thread_annotations.h" #include "webrtc/typedefs.h" #include "webrtc/frame_callback.h" #include "webrtc/video_engine/vie_defines.h" @@ -154,6 +155,8 @@ class ViEEncoder // Sets SSRCs for all streams. bool SetSsrcs(const std::list& ssrcs); + void SetMinTransmitBitrate(int min_transmit_bitrate_kbps); + // Effect filter. int32_t RegisterEffectFilter(ViEEffectFilter* effect_filter); @@ -207,6 +210,7 @@ class ViEEncoder int64_t time_of_last_incoming_frame_ms_; bool send_padding_; + int min_transmit_bitrate_kbps_ GUARDED_BY(data_cs_); int target_delay_ms_; bool network_is_transmitting_; bool encoder_paused_; @@ -216,7 +220,7 @@ class ViEEncoder bool fec_enabled_; bool nack_enabled_; - ViEEncoderObserver* codec_observer_; + ViEEncoderObserver* codec_observer_ GUARDED_BY(callback_cs_); ViEEffectFilter* effect_filter_; ProcessThread& module_process_thread_; diff --git a/video_engine/vie_rtp_rtcp_impl.cc b/video_engine/vie_rtp_rtcp_impl.cc index 54afa93c..6717e84d 100644 --- a/video_engine/vie_rtp_rtcp_impl.cc +++ b/video_engine/vie_rtp_rtcp_impl.cc @@ -850,6 +850,16 @@ int ViERTP_RTCPImpl::SetTransmissionSmoothingStatus(int video_channel, return 0; } +int ViERTP_RTCPImpl::SetMinTransmitBitrate(int video_channel, + int min_transmit_bitrate_kbps) { + ViEChannelManagerScoped cs(*(shared_data_->channel_manager())); + ViEEncoder* vie_encoder = cs.Encoder(video_channel); + if (vie_encoder == NULL) + return -1; + vie_encoder->SetMinTransmitBitrate(min_transmit_bitrate_kbps); + return 0; +} + int ViERTP_RTCPImpl::GetReceiveChannelRtcpStatistics( const int video_channel, RtcpStatistics& basic_stats, diff --git a/video_engine/vie_rtp_rtcp_impl.h b/video_engine/vie_rtp_rtcp_impl.h index 227fa5e4..3120beed 100644 --- a/video_engine/vie_rtp_rtcp_impl.h +++ b/video_engine/vie_rtp_rtcp_impl.h @@ -90,6 +90,8 @@ class ViERTP_RTCPImpl int id); virtual int SetRtcpXrRrtrStatus(int video_channel, bool enable); virtual int SetTransmissionSmoothingStatus(int video_channel, bool enable); + virtual int SetMinTransmitBitrate(int video_channel, + int min_transmit_bitrate_kbps); virtual int GetReceiveChannelRtcpStatistics(const int video_channel, RtcpStatistics& basic_stats, int& rtt_ms) const; diff --git a/video_send_stream.h b/video_send_stream.h index c3140c07..5a4c89d8 100644 --- a/video_send_stream.h +++ b/video_send_stream.h @@ -68,13 +68,20 @@ class VideoSendStream { static const size_t kDefaultMaxPacketSize = 1500 - 40; // TCP over IPv4. struct Rtp { - Rtp() : max_packet_size(kDefaultMaxPacketSize) {} + Rtp() + : max_packet_size(kDefaultMaxPacketSize), + min_transmit_bitrate_kbps(0) {} std::vector ssrcs; // 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_kbps; + // RTP header extensions to use for this send stream. std::vector extensions; -- cgit v1.2.3