From 1a3a6e5340d9aaa933bfb659bd6d8f66ca1c1409 Mon Sep 17 00:00:00 2001 From: "henrik.lundin@webrtc.org" Date: Mon, 28 Oct 2013 10:16:14 +0000 Subject: [PATCH] Removing the threshold from the auto-mute APIs The threshold is now set equal to the minimum bitrate of the encoder. The test is also changed to have the REMB values depend on the minimum bitrate from the encoder. BUG=2436 R=pbos@webrtc.org, stefan@webrtc.org Review URL: https://webrtc-codereview.appspot.com/2919004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@5040 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../main/interface/video_coding.h | 2 +- .../main/source/video_coding_impl.cc | 4 +-- .../main/source/video_coding_impl.h | 2 +- .../video_coding/main/source/video_sender.cc | 22 ++++++++++++-- webrtc/video_engine/include/vie_codec.h | 3 +- .../internal/video_send_stream.cc | 7 ++--- .../new_include/video_send_stream.h | 16 ++++------ webrtc/video_engine/test/send_stream_tests.cc | 30 ++++++++++++------- webrtc/video_engine/vie_codec_impl.cc | 5 ++-- webrtc/video_engine/vie_codec_impl.h | 3 +- webrtc/video_engine/vie_encoder.cc | 5 ++-- webrtc/video_engine/vie_encoder.h | 2 +- 12 files changed, 59 insertions(+), 42 deletions(-) diff --git a/webrtc/modules/video_coding/main/interface/video_coding.h b/webrtc/modules/video_coding/main/interface/video_coding.h index 50d0c1ad20..5a206e9853 100644 --- a/webrtc/modules/video_coding/main/interface/video_coding.h +++ b/webrtc/modules/video_coding/main/interface/video_coding.h @@ -595,7 +595,7 @@ public: // Enables AutoMuter to turn off video when the rate drops below // |threshold_bps|, and turns back on when the rate goes back up above // |threshold_bps| + |window_bps|. - virtual void EnableAutoMuting(int threshold_bps, int window_bps) = 0; + virtual void EnableAutoMuting() = 0; // Disables AutoMuter. virtual void DisableAutoMuting() = 0; diff --git a/webrtc/modules/video_coding/main/source/video_coding_impl.cc b/webrtc/modules/video_coding/main/source/video_coding_impl.cc index 42cb5ad8ea..95249843b3 100644 --- a/webrtc/modules/video_coding/main/source/video_coding_impl.cc +++ b/webrtc/modules/video_coding/main/source/video_coding_impl.cc @@ -197,8 +197,8 @@ class VideoCodingModuleImpl : public VideoCodingModule { return sender_->StopDebugRecording(); } - virtual void EnableAutoMuting(int threshold_bps, int window_bps) { - return sender_->EnableAutoMuting(threshold_bps, window_bps); + virtual void EnableAutoMuting() { + return sender_->EnableAutoMuting(); } virtual void DisableAutoMuting() { diff --git a/webrtc/modules/video_coding/main/source/video_coding_impl.h b/webrtc/modules/video_coding/main/source/video_coding_impl.h index 795c6ce4b5..f9a797351c 100644 --- a/webrtc/modules/video_coding/main/source/video_coding_impl.h +++ b/webrtc/modules/video_coding/main/source/video_coding_impl.h @@ -95,7 +95,7 @@ class VideoSender { int StartDebugRecording(const char* file_name_utf8); int StopDebugRecording(); - void EnableAutoMuting(int threshold_bps, int window_bps); + void EnableAutoMuting(); void DisableAutoMuting(); bool VideoMuted() const; diff --git a/webrtc/modules/video_coding/main/source/video_sender.cc b/webrtc/modules/video_coding/main/source/video_sender.cc index d7ed3cf127..4cbcc895b4 100644 --- a/webrtc/modules/video_coding/main/source/video_sender.cc +++ b/webrtc/modules/video_coding/main/source/video_sender.cc @@ -10,6 +10,8 @@ #include "webrtc/common_types.h" +#include // std::max + #include "webrtc/common_video/libyuv/include/webrtc_libyuv.h" #include "webrtc/modules/video_coding/codecs/interface/video_codec_interface.h" #include "webrtc/modules/video_coding/main/source/encoded_frame.h" @@ -420,14 +422,28 @@ int VideoSender::StopDebugRecording() { return VCM_OK; } -void VideoSender::EnableAutoMuting(int threshold_bps, int window_bps) { +void VideoSender::EnableAutoMuting() { CriticalSectionScoped cs(_sendCritSect); - return _mediaOpt.EnableAutoMuting(threshold_bps, window_bps); + VideoCodec current_send_codec; + if (SendCodec(¤t_send_codec) != 0) { + assert(false); // Must set a send codec before enabling auto-mute. + return; + } + int threshold_bps; + if (current_send_codec.numberOfSimulcastStreams == 0) { + threshold_bps = current_send_codec.minBitrate * 1000; + } else { + threshold_bps = current_send_codec.simulcastStream[0].minBitrate * 1000; + } + // Set the hysteresis window to be at 10% of the threshold, but at least + // 10 kbps. + int window_bps = std::max(threshold_bps / 10, 10000); + _mediaOpt.EnableAutoMuting(threshold_bps, window_bps); } void VideoSender::DisableAutoMuting() { CriticalSectionScoped cs(_sendCritSect); - return _mediaOpt.DisableAutoMuting(); + _mediaOpt.DisableAutoMuting(); } bool VideoSender::VideoMuted() const { diff --git a/webrtc/video_engine/include/vie_codec.h b/webrtc/video_engine/include/vie_codec.h index 92d79b001e..023a36d068 100644 --- a/webrtc/video_engine/include/vie_codec.h +++ b/webrtc/video_engine/include/vie_codec.h @@ -200,8 +200,7 @@ class WEBRTC_DLLEXPORT ViECodec { // |threshold_bps| + |window_bps|. // This is under development; not tested. // TODO(hlundin): Remove the default implementation when possible. - virtual void EnableAutoMuting(int video_channel, int threshold_bps, - int window_bps) {} + virtual void EnableAutoMuting(int video_channel) {} protected: ViECodec() {} diff --git a/webrtc/video_engine/internal/video_send_stream.cc b/webrtc/video_engine/internal/video_send_stream.cc index 242f7fa854..70a6918148 100644 --- a/webrtc/video_engine/internal/video_send_stream.cc +++ b/webrtc/video_engine/internal/video_send_stream.cc @@ -197,11 +197,8 @@ VideoSendStream::VideoSendStream(newapi::Transport* transport, image_process_->RegisterPreEncodeCallback(channel_, config_.pre_encode_callback); - if (config.auto_muter.threshold_bps > 0) { - assert(config.auto_muter.window_bps >= 0); - codec_->EnableAutoMuting(channel_, - config.auto_muter.threshold_bps, - config.auto_muter.window_bps); + if (config.auto_mute) { + codec_->EnableAutoMuting(channel_); } } diff --git a/webrtc/video_engine/new_include/video_send_stream.h b/webrtc/video_engine/new_include/video_send_stream.h index 59da54b774..f83ee08072 100644 --- a/webrtc/video_engine/new_include/video_send_stream.h +++ b/webrtc/video_engine/new_include/video_send_stream.h @@ -81,7 +81,8 @@ class VideoSendStream { target_delay_ms(0), pacing(false), stats_callback(NULL), - start_state(NULL) {} + start_state(NULL), + auto_mute(false) {} VideoCodec codec; static const size_t kDefaultMaxPacketSize = 1500 - 40; // TCP over IPv4. @@ -147,15 +148,10 @@ class VideoSendStream { // Set to resume a previously destroyed send stream. SendStreamState* start_state; - // Parameters for auto muter. If threshold_bps > 0, video will be muted when - // the bandwidth estimate drops below this limit, and enabled again when the - // bandwidth estimate goes above threshold_bps + window_bps. Setting the - // threshold to zero disables the auto muter. - struct AutoMuter { - AutoMuter() : threshold_bps(0), window_bps(0) {} - int threshold_bps; - int window_bps; - } auto_muter; + // True if video should be muted when video goes under the minimum video + // bitrate. Unless muted, video will be sent at a bitrate higher than + // estimated available. + bool auto_mute; }; // Gets interface used to insert captured frames. Valid as long as the diff --git a/webrtc/video_engine/test/send_stream_tests.cc b/webrtc/video_engine/test/send_stream_tests.cc index 68f96859e3..2772104b4c 100644 --- a/webrtc/video_engine/test/send_stream_tests.cc +++ b/webrtc/video_engine/test/send_stream_tests.cc @@ -7,6 +7,8 @@ * in the file PATENTS. All contributing project authors may * be found in the AUTHORS file in the root of the source tree. */ +#include // max + #include "testing/gtest/include/gtest/gtest.h" #include "webrtc/common_video/interface/i420_video_frame.h" #include "webrtc/modules/rtp_rtcp/interface/rtp_header_parser.h" @@ -491,12 +493,6 @@ TEST_F(VideoSendStreamTest, MaxPacketSize) { // When the stream is detected again, the test ends. TEST_F(VideoSendStreamTest, AutoMute) { static const int kMuteTimeFrames = 60; // Mute for 2 seconds @ 30 fps. - static const int kMuteThresholdBps = 70000; - static const int kMuteWindowBps = 10000; - // Let the low REMB value be 10 kbps lower than the muter threshold, and the - // high REMB value be 5 kbps higher than the re-enabling threshold. - static const int kLowRembBps = kMuteThresholdBps - 10000; - static const int kHighRembBps = kMuteThresholdBps + kMuteWindowBps + 5000; class RembObserver : public SendTransportObserver, public I420FrameCallback { public: @@ -508,6 +504,8 @@ TEST_F(VideoSendStreamTest, AutoMute) { rtp_count_(0), last_sequence_number_(0), mute_frame_count_(0), + low_remb_bps_(0), + high_remb_bps_(0), crit_sect_(CriticalSectionWrapper::CreateCriticalSection()) {} void SetReceiver(PacketReceiver* receiver) { @@ -532,7 +530,7 @@ TEST_F(VideoSendStreamTest, AutoMute) { if (test_state_ == kBeforeMute) { // The stream has started. Try to mute it. - SendRtcpFeedback(kLowRembBps); + SendRtcpFeedback(low_remb_bps_); test_state_ = kDuringMute; } else if (test_state_ == kDuringMute) { mute_frame_count_ = 0; @@ -547,11 +545,15 @@ TEST_F(VideoSendStreamTest, AutoMute) { void FrameCallback(I420VideoFrame* video_frame) OVERRIDE { CriticalSectionScoped lock(crit_sect_.get()); if (test_state_ == kDuringMute && ++mute_frame_count_ > kMuteTimeFrames) { - SendRtcpFeedback(kHighRembBps); + SendRtcpFeedback(high_remb_bps_); test_state_ = kWaitingForPacket; } } + void set_low_remb_bps(int value) { low_remb_bps_ = value; } + + void set_high_remb_bps(int value) { high_remb_bps_ = value; } + private: enum TestState { kBeforeMute, @@ -583,6 +585,8 @@ TEST_F(VideoSendStreamTest, AutoMute) { int rtp_count_; int last_sequence_number_; int mute_frame_count_; + int low_remb_bps_; + int high_remb_bps_; scoped_ptr crit_sect_; } observer; @@ -592,9 +596,15 @@ TEST_F(VideoSendStreamTest, AutoMute) { VideoSendStream::Config send_config = GetSendTestConfig(call.get()); send_config.rtp.nack.rtp_history_ms = 1000; - send_config.auto_muter.threshold_bps = kMuteThresholdBps; - send_config.auto_muter.window_bps = kMuteWindowBps; send_config.pre_encode_callback = &observer; + send_config.auto_mute = true; + unsigned int min_bitrate_bps = + send_config.codec.simulcastStream[0].minBitrate * 1000; + observer.set_low_remb_bps(min_bitrate_bps - 10000); + unsigned int threshold_window = std::max(min_bitrate_bps / 10, 10000u); + ASSERT_GT(send_config.codec.simulcastStream[0].maxBitrate * 1000, + min_bitrate_bps + threshold_window + 5000); + observer.set_high_remb_bps(min_bitrate_bps + threshold_window + 5000); RunSendTest(call.get(), send_config, &observer); } diff --git a/webrtc/video_engine/vie_codec_impl.cc b/webrtc/video_engine/vie_codec_impl.cc index bf845d9553..4d927b5902 100644 --- a/webrtc/video_engine/vie_codec_impl.cc +++ b/webrtc/video_engine/vie_codec_impl.cc @@ -715,8 +715,7 @@ int ViECodecImpl::StopDebugRecording(int video_channel) { return vie_encoder->StopDebugRecording(); } -void ViECodecImpl::EnableAutoMuting(int video_channel, int threshold_bps, - int window_bps) { +void ViECodecImpl::EnableAutoMuting(int video_channel) { ViEChannelManagerScoped cs(*(shared_data_->channel_manager())); ViEEncoder* vie_encoder = cs.Encoder(video_channel); if (!vie_encoder) { @@ -725,7 +724,7 @@ void ViECodecImpl::EnableAutoMuting(int video_channel, int threshold_bps, "%s: No encoder %d", __FUNCTION__, video_channel); return; } - return vie_encoder->EnableAutoMuting(threshold_bps, window_bps); + return vie_encoder->EnableAutoMuting(); } bool ViECodecImpl::CodecValid(const VideoCodec& video_codec) { diff --git a/webrtc/video_engine/vie_codec_impl.h b/webrtc/video_engine/vie_codec_impl.h index 5a998c8177..8bcac41d6a 100644 --- a/webrtc/video_engine/vie_codec_impl.h +++ b/webrtc/video_engine/vie_codec_impl.h @@ -70,8 +70,7 @@ class ViECodecImpl virtual int StartDebugRecording(int video_channel, const char* file_name_utf8); virtual int StopDebugRecording(int video_channel); - virtual void EnableAutoMuting(int video_channel, int threshold_bps, - int window_bps); + virtual void EnableAutoMuting(int video_channel); protected: explicit ViECodecImpl(ViESharedData* shared_data); diff --git a/webrtc/video_engine/vie_encoder.cc b/webrtc/video_engine/vie_encoder.cc index 6295920a5f..1c6334afc7 100644 --- a/webrtc/video_engine/vie_encoder.cc +++ b/webrtc/video_engine/vie_encoder.cc @@ -1156,8 +1156,9 @@ int ViEEncoder::StopDebugRecording() { return vcm_.StopDebugRecording(); } -void ViEEncoder::EnableAutoMuting(int threshold_bps, int window_bps) { - vcm_.EnableAutoMuting(threshold_bps, window_bps); +void ViEEncoder::EnableAutoMuting() { + vcm_.EnableAutoMuting(); + bitrate_controller_->EnforceMinBitrate(false); } void ViEEncoder::RegisterPreEncodeCallback( diff --git a/webrtc/video_engine/vie_encoder.h b/webrtc/video_engine/vie_encoder.h index bb606994e6..2d7bae19a7 100644 --- a/webrtc/video_engine/vie_encoder.h +++ b/webrtc/video_engine/vie_encoder.h @@ -166,7 +166,7 @@ class ViEEncoder // Enables AutoMuter to turn off video when the rate drops below // |threshold_bps|, and turns back on when the rate goes back up above // |threshold_bps| + |window_bps|. - virtual void EnableAutoMuting(int threshold_bps, int window_bps); + virtual void EnableAutoMuting(); // New-style callback, used by VideoSendStream. void RegisterPreEncodeCallback(I420FrameCallback* pre_encode_callback);