From 143451d2590ef951f6e66a983a38a18fcd4c66a5 Mon Sep 17 00:00:00 2001 From: "pbos@webrtc.org" Date: Wed, 18 Mar 2015 14:40:03 +0000 Subject: [PATCH] Base start bitrate on last observed bitrate. Instead of setting bitrates based on codec target settings (which may have previously been capped by a codec max bitrate), fetch the last bandwidth allocated for this channel. This fixes broken low start bitrates due to QCIF being set as default codec in WebRtcVideoEngine2 which caps the max bitrate to 200kbps. BUG=1788 R=mflodman@webrtc.org, stefan@webrtc.org Review URL: https://webrtc-codereview.appspot.com/43789004 Cr-Commit-Position: refs/heads/master@{#8780} git-svn-id: http://webrtc.googlecode.com/svn/trunk@8780 4adac7df-926f-26a2-2b94-8c16560cd09d --- talk/media/webrtc/fakewebrtcvideoengine.h | 3 ++ webrtc/video/video_send_stream.cc | 10 +--- webrtc/video/video_send_stream_tests.cc | 60 +++++++++++++++++++++++ webrtc/video_engine/include/vie_codec.h | 2 + webrtc/video_engine/vie_codec_impl.cc | 6 +++ webrtc/video_engine/vie_codec_impl.h | 1 + webrtc/video_engine/vie_encoder.cc | 7 +++ webrtc/video_engine/vie_encoder.h | 2 + 8 files changed, 83 insertions(+), 8 deletions(-) diff --git a/talk/media/webrtc/fakewebrtcvideoengine.h b/talk/media/webrtc/fakewebrtcvideoengine.h index 2719c96e0..a26c5136d 100644 --- a/talk/media/webrtc/fakewebrtcvideoengine.h +++ b/talk/media/webrtc/fakewebrtcvideoengine.h @@ -831,6 +831,9 @@ class FakeWebRtcVideoEngine unsigned int&, unsigned int&)); WEBRTC_STUB_CONST(GetReceiveSideDelay, (const int video_channel, int* delay_ms)); + virtual uint32_t GetLastObservedBitrateBps(int channel) const override { + return 0; + } WEBRTC_FUNC_CONST(GetCodecTargetBitrate, (const int channel, unsigned int* codec_target_bitrate)) { WEBRTC_CHECK_CHANNEL(channel); diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index cff867216..ad3bb9c3c 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -405,9 +405,8 @@ bool VideoSendStream::ReconfigureVideoEncoder( static_cast(bitrate_config_.max_bitrate_bps / 1000)) { video_codec.maxBitrate = bitrate_config_.max_bitrate_bps / 1000; } - unsigned int start_bitrate_bps; - if (codec_->GetCodecTargetBitrate(channel_, &start_bitrate_bps) != 0 || - use_config_bitrate_) { + uint32_t start_bitrate_bps = codec_->GetLastObservedBitrateBps(channel_); + if (start_bitrate_bps == 0 || use_config_bitrate_) { start_bitrate_bps = bitrate_config_.start_bitrate_bps; } video_codec.startBitrate = @@ -422,11 +421,6 @@ bool VideoSendStream::ReconfigureVideoEncoder( if (video_codec.startBitrate > video_codec.maxBitrate) video_codec.startBitrate = video_codec.maxBitrate; - if (video_codec.startBitrate < video_codec.minBitrate) - video_codec.startBitrate = video_codec.minBitrate; - if (video_codec.startBitrate > video_codec.maxBitrate) - video_codec.startBitrate = video_codec.maxBitrate; - assert(streams[0].max_framerate > 0); video_codec.maxFramerate = streams[0].max_framerate; diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc index f8cba094d..746b140a0 100644 --- a/webrtc/video/video_send_stream_tests.cc +++ b/webrtc/video/video_send_stream_tests.cc @@ -964,6 +964,66 @@ TEST_F(VideoSendStreamTest, MinTransmitBitrateRespectsRemb) { RunBaseTest(&test); } +TEST_F(VideoSendStreamTest, CanReconfigureToUseStartBitrateAbovePreviousMax) { + class StartBitrateObserver : public test::FakeEncoder { + public: + StartBitrateObserver() + : FakeEncoder(Clock::GetRealTimeClock()), start_bitrate_kbps_(0) {} + int32_t InitEncode(const VideoCodec* config, + int32_t number_of_cores, + size_t max_payload_size) override { + rtc::CritScope lock(&crit_); + start_bitrate_kbps_ = config->startBitrate; + return FakeEncoder::InitEncode(config, number_of_cores, max_payload_size); + } + + int32_t SetRates(uint32_t new_target_bitrate, uint32_t framerate) override { + rtc::CritScope lock(&crit_); + start_bitrate_kbps_ = new_target_bitrate; + return FakeEncoder::SetRates(new_target_bitrate, framerate); + } + + int GetStartBitrateKbps() const { + rtc::CritScope lock(&crit_); + return start_bitrate_kbps_; + } + + private: + mutable rtc::CriticalSection crit_; + int start_bitrate_kbps_ GUARDED_BY(crit_); + }; + + test::NullTransport transport; + CreateSenderCall(Call::Config(&transport)); + + CreateSendConfig(1); + + Call::Config::BitrateConfig bitrate_config; + bitrate_config.start_bitrate_bps = + 2 * encoder_config_.streams[0].max_bitrate_bps; + sender_call_->SetBitrateConfig(bitrate_config); + + StartBitrateObserver encoder; + send_config_.encoder_settings.encoder = &encoder; + + CreateStreams(); + + EXPECT_EQ(encoder_config_.streams[0].max_bitrate_bps / 1000, + encoder.GetStartBitrateKbps()); + + encoder_config_.streams[0].max_bitrate_bps = + 2 * bitrate_config.start_bitrate_bps; + send_stream_->ReconfigureVideoEncoder(encoder_config_); + + // New bitrate should be reconfigured above the previous max. As there's no + // network connection this shouldn't be flaky, as no bitrate should've been + // reported in between. + EXPECT_EQ(bitrate_config.start_bitrate_bps / 1000, + encoder.GetStartBitrateKbps()); + + DestroyStreams(); +} + TEST_F(VideoSendStreamTest, CapturesTextureAndI420VideoFrames) { class FrameObserver : public I420FrameCallback { public: diff --git a/webrtc/video_engine/include/vie_codec.h b/webrtc/video_engine/include/vie_codec.h index c6354d330..4190d6bdd 100644 --- a/webrtc/video_engine/include/vie_codec.h +++ b/webrtc/video_engine/include/vie_codec.h @@ -144,6 +144,8 @@ class WEBRTC_DLLEXPORT ViECodec { virtual int GetReceiveSideDelay(const int video_channel, int* delay_ms) const = 0; + // Current target bitrate for this channel. + virtual uint32_t GetLastObservedBitrateBps(int video_channel) const = 0; // Gets the bitrate targeted by the video codec rate control in kbit/s. virtual int GetCodecTargetBitrate(const int video_channel, unsigned int* bitrate) const = 0; diff --git a/webrtc/video_engine/vie_codec_impl.cc b/webrtc/video_engine/vie_codec_impl.cc index c442f59df..74d4f6838 100644 --- a/webrtc/video_engine/vie_codec_impl.cc +++ b/webrtc/video_engine/vie_codec_impl.cc @@ -402,6 +402,12 @@ int ViECodecImpl::GetReceiveSideDelay(const int video_channel, return 0; } +uint32_t ViECodecImpl::GetLastObservedBitrateBps(int video_channel) const { + ViEChannelManagerScoped cs(*(shared_data_->channel_manager())); + ViEEncoder* vie_encoder = cs.Encoder(video_channel); + assert(vie_encoder != nullptr); + return vie_encoder->LastObservedBitrateBps(); +} int ViECodecImpl::GetCodecTargetBitrate(const int video_channel, unsigned int* bitrate) const { diff --git a/webrtc/video_engine/vie_codec_impl.h b/webrtc/video_engine/vie_codec_impl.h index ed8bf8a7b..8460fec09 100644 --- a/webrtc/video_engine/vie_codec_impl.h +++ b/webrtc/video_engine/vie_codec_impl.h @@ -51,6 +51,7 @@ class ViECodecImpl unsigned int& delta_frames) const; virtual int GetReceiveSideDelay(const int video_channel, int* delay_ms) const; + uint32_t GetLastObservedBitrateBps(int video_channel) const override; virtual int GetCodecTargetBitrate(const int video_channel, unsigned int* bitrate) const; virtual int GetNumDiscardedPackets(int video_channel) const; diff --git a/webrtc/video_engine/vie_encoder.cc b/webrtc/video_engine/vie_encoder.cc index e5f6b96d0..c388e39d0 100644 --- a/webrtc/video_engine/vie_encoder.cc +++ b/webrtc/video_engine/vie_encoder.cc @@ -148,6 +148,7 @@ ViEEncoder::ViEEncoder(int32_t channel_id, time_of_last_incoming_frame_ms_(0), send_padding_(false), min_transmit_bitrate_kbps_(0), + last_observed_bitrate_bps_(0), target_delay_ms_(0), network_is_transmitting_(true), encoder_paused_(false), @@ -690,6 +691,11 @@ int64_t ViEEncoder::PacerQueuingDelayMs() const { return paced_sender_->QueueInMs(); } +uint32_t ViEEncoder::LastObservedBitrateBps() const { + CriticalSectionScoped cs(data_cs_.get()); + return last_observed_bitrate_bps_; +} + int ViEEncoder::CodecTargetBitrate(uint32_t* bitrate) const { if (vcm_.Bitrate(bitrate) != 0) return -1; @@ -924,6 +930,7 @@ void ViEEncoder::OnNetworkChanged(uint32_t bitrate_bps, { CriticalSectionScoped cs(data_cs_.get()); + last_observed_bitrate_bps_ = bitrate_bps; if (video_suspended_ == video_is_suspended) return; video_suspended_ = video_is_suspended; diff --git a/webrtc/video_engine/vie_encoder.h b/webrtc/video_engine/vie_encoder.h index a8be5266d..784a2423c 100644 --- a/webrtc/video_engine/vie_encoder.h +++ b/webrtc/video_engine/vie_encoder.h @@ -123,6 +123,7 @@ class ViEEncoder int64_t PacerQueuingDelayMs() const; + uint32_t LastObservedBitrateBps() const; int CodecTargetBitrate(uint32_t* bitrate) const; // Loss protection. int32_t UpdateProtectionMethod(bool nack, bool fec); @@ -221,6 +222,7 @@ class ViEEncoder int64_t time_of_last_incoming_frame_ms_ GUARDED_BY(data_cs_); bool send_padding_ GUARDED_BY(data_cs_); int min_transmit_bitrate_kbps_ GUARDED_BY(data_cs_); + uint32_t last_observed_bitrate_bps_ GUARDED_BY(data_cs_); int target_delay_ms_ GUARDED_BY(data_cs_); bool network_is_transmitting_ GUARDED_BY(data_cs_); bool encoder_paused_ GUARDED_BY(data_cs_);