From 6f48f1bf68a10669c9bcd81262c1a98ed2a8d462 Mon Sep 17 00:00:00 2001 From: "pbos@webrtc.org" Date: Tue, 22 Jul 2014 16:29:54 +0000 Subject: [PATCH] Implement encoder options in WebRtcVideoEngine2. Implementing default options to enable denoising by default and wiring up encoder settings to propagate VP8 settings. BUG=1788 R=wu@webrtc.org Review URL: https://webrtc-codereview.appspot.com/19999004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@6757 4adac7df-926f-26a2-2b94-8c16560cd09d --- talk/media/webrtc/webrtcvideoengine2.cc | 74 +++++++++++++++++-- talk/media/webrtc/webrtcvideoengine2.h | 8 ++ .../webrtc/webrtcvideoengine2_unittest.cc | 52 +++++++++++-- .../webrtc/webrtcvideoengine2_unittest.h | 6 +- 4 files changed, 129 insertions(+), 11 deletions(-) diff --git a/talk/media/webrtc/webrtcvideoengine2.cc b/talk/media/webrtc/webrtcvideoengine2.cc index e08f17050..75937b046 100644 --- a/talk/media/webrtc/webrtcvideoengine2.cc +++ b/talk/media/webrtc/webrtcvideoengine2.cc @@ -197,7 +197,47 @@ webrtc::VideoEncoder* WebRtcVideoEncoderFactory2::CreateVideoEncoder( const VideoCodec& codec, const VideoOptions& options) { assert(SupportsCodec(codec)); - return webrtc::VP8Encoder::Create(); + if (_stricmp(codec.name.c_str(), kVp8CodecName) == 0) { + return webrtc::VP8Encoder::Create(); + } + // This shouldn't happen, we should be able to create encoders for all codecs + // we support. + assert(false); + return NULL; +} + +void* WebRtcVideoEncoderFactory2::CreateVideoEncoderSettings( + const VideoCodec& codec, + const VideoOptions& options) { + assert(SupportsCodec(codec)); + if (_stricmp(codec.name.c_str(), kVp8CodecName) == 0) { + webrtc::VideoCodecVP8* settings = new webrtc::VideoCodecVP8(); + settings->resilience = webrtc::kResilientStream; + settings->numberOfTemporalLayers = 1; + options.video_noise_reduction.Get(&settings->denoisingOn); + settings->errorConcealmentOn = false; + settings->automaticResizeOn = false; + settings->frameDroppingOn = true; + settings->keyFrameInterval = 3000; + return settings; + } + return NULL; +} + +void WebRtcVideoEncoderFactory2::DestroyVideoEncoderSettings( + const VideoCodec& codec, + void* encoder_settings) { + assert(SupportsCodec(codec)); + if (encoder_settings == NULL) { + return; + } + + if (_stricmp(codec.name.c_str(), kVp8CodecName) == 0) { + delete reinterpret_cast(encoder_settings); + return; + } + // We should be able to destroy all encoder settings we've allocated. + assert(false); } bool WebRtcVideoEncoderFactory2::SupportsCodec(const VideoCodec& codec) { @@ -618,6 +658,12 @@ void WebRtcVideoChannel2::Construct(webrtc::Call* call, default_renderer_ = NULL; default_send_ssrc_ = 0; default_recv_ssrc_ = 0; + + SetDefaultOptions(); +} + +void WebRtcVideoChannel2::SetDefaultOptions() { + options_.video_noise_reduction.Set(true); } WebRtcVideoChannel2::~WebRtcVideoChannel2() { @@ -1494,8 +1540,18 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::SetDimensions(int width, parameters_.video_streams.back().width = width; parameters_.video_streams.back().height = height; - // TODO(pbos): Wire up encoder_parameters, webrtc:3424. - if (!stream_->ReconfigureVideoEncoder(parameters_.video_streams, NULL)) { + VideoCodecSettings codec_settings; + parameters_.codec_settings.Get(&codec_settings); + void* encoder_settings = encoder_factory_->CreateVideoEncoderSettings( + codec_settings.codec, parameters_.options); + + bool stream_reconfigured = stream_->ReconfigureVideoEncoder( + parameters_.video_streams, encoder_settings); + + encoder_factory_->DestroyVideoEncoderSettings(codec_settings.codec, + encoder_settings); + + if (!stream_reconfigured) { LOG(LS_WARNING) << "Failed to reconfigure video encoder for dimensions: " << width << "x" << height; return; @@ -1576,9 +1632,17 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::RecreateWebRtcStream() { call_->DestroyVideoSendStream(stream_); } - // TODO(pbos): Wire up encoder_parameters, webrtc:3424. + VideoCodecSettings codec_settings; + parameters_.codec_settings.Get(&codec_settings); + void* encoder_settings = encoder_factory_->CreateVideoEncoderSettings( + codec_settings.codec, parameters_.options); + stream_ = call_->CreateVideoSendStream( - parameters_.config, parameters_.video_streams, NULL); + parameters_.config, parameters_.video_streams, encoder_settings); + + encoder_factory_->DestroyVideoEncoderSettings(codec_settings.codec, + encoder_settings); + if (sending_) { stream_->Start(); } diff --git a/talk/media/webrtc/webrtcvideoengine2.h b/talk/media/webrtc/webrtcvideoengine2.h index e93f57145..79371ab4c 100644 --- a/talk/media/webrtc/webrtcvideoengine2.h +++ b/talk/media/webrtc/webrtcvideoengine2.h @@ -94,6 +94,13 @@ class WebRtcVideoEncoderFactory2 { const VideoCodec& codec, const VideoOptions& options); + virtual void* CreateVideoEncoderSettings( + const VideoCodec& codec, + const VideoOptions& options); + + virtual void DestroyVideoEncoderSettings(const VideoCodec& codec, + void* encoder_settings); + virtual bool SupportsCodec(const cricket::VideoCodec& codec); }; @@ -358,6 +365,7 @@ class WebRtcVideoChannel2 : public talk_base::MessageHandler, }; void Construct(webrtc::Call* call, WebRtcVideoEngine2* engine); + void SetDefaultOptions(); virtual bool SendRtp(const uint8_t* data, size_t len) OVERRIDE; virtual bool SendRtcp(const uint8_t* data, size_t len) OVERRIDE; diff --git a/talk/media/webrtc/webrtcvideoengine2_unittest.cc b/talk/media/webrtc/webrtcvideoengine2_unittest.cc index 5bbec3362..c2f2dc35c 100644 --- a/talk/media/webrtc/webrtcvideoengine2_unittest.cc +++ b/talk/media/webrtc/webrtcvideoengine2_unittest.cc @@ -68,8 +68,13 @@ void VerifyCodecHasDefaultFeedbackParams(const cricket::VideoCodec& codec) { namespace cricket { FakeVideoSendStream::FakeVideoSendStream( const webrtc::VideoSendStream::Config& config, - const std::vector& video_streams) - : sending_(false), config_(config), video_streams_(video_streams) { + const std::vector& video_streams, + const void* encoder_settings) + : sending_(false), + config_(config), + codec_settings_set_(false) { + assert(config.encoder_settings.encoder != NULL); + ReconfigureVideoEncoder(video_streams, encoder_settings); } webrtc::VideoSendStream::Config FakeVideoSendStream::GetConfig() { @@ -84,6 +89,16 @@ bool FakeVideoSendStream::IsSending() const { return sending_; } +bool FakeVideoSendStream::GetVp8Settings( + webrtc::VideoCodecVP8* settings) const { + if (!codec_settings_set_) { + return false; + } + + *settings = vp8_settings_; + return true; +} + webrtc::VideoSendStream::Stats FakeVideoSendStream::GetStats() const { return webrtc::VideoSendStream::Stats(); } @@ -92,6 +107,12 @@ bool FakeVideoSendStream::ReconfigureVideoEncoder( const std::vector& streams, const void* encoder_specific) { video_streams_ = streams; + if (encoder_specific != NULL) { + assert(config_.encoder_settings.payload_name == "VP8"); + vp8_settings_ = + *reinterpret_cast(encoder_specific); + } + codec_settings_set_ = encoder_specific != NULL; return true; } @@ -202,7 +223,7 @@ webrtc::VideoSendStream* FakeCall::CreateVideoSendStream( const std::vector& video_streams, const void* encoder_settings) { FakeVideoSendStream* fake_stream = - new FakeVideoSendStream(config, video_streams); + new FakeVideoSendStream(config, video_streams, encoder_settings); video_send_streams_.push_back(fake_stream); return fake_stream; } @@ -1046,8 +1067,29 @@ TEST_F(WebRtcVideoChannel2Test, FAIL() << "Not implemented."; // TODO(pbos): Implement. } -TEST_F(WebRtcVideoChannel2Test, DISABLED_SetOptionsWithDenoising) { - FAIL() << "Not implemented."; // TODO(pbos): Implement. +TEST_F(WebRtcVideoChannel2Test, Vp8DenoisingEnabledByDefault) { + FakeVideoSendStream* stream = AddSendStream(); + webrtc::VideoCodecVP8 vp8_settings; + ASSERT_TRUE(stream->GetVp8Settings(&vp8_settings)) << "No VP8 config set."; + EXPECT_TRUE(vp8_settings.denoisingOn); +} + +TEST_F(WebRtcVideoChannel2Test, SetOptionsWithDenoising) { + VideoOptions options; + options.video_noise_reduction.Set(false); + channel_->SetOptions(options); + + FakeVideoSendStream* stream = AddSendStream(); + webrtc::VideoCodecVP8 vp8_settings; + ASSERT_TRUE(stream->GetVp8Settings(&vp8_settings)) << "No VP8 config set."; + EXPECT_FALSE(vp8_settings.denoisingOn); + + options.video_noise_reduction.Set(true); + channel_->SetOptions(options); + + stream = fake_channel_->GetFakeCall()->GetVideoSendStreams()[0]; + ASSERT_TRUE(stream->GetVp8Settings(&vp8_settings)) << "No VP8 config set."; + EXPECT_TRUE(vp8_settings.denoisingOn); } TEST_F(WebRtcVideoChannel2Test, DISABLED_MultipleSendStreamsWithOneCapturer) { diff --git a/talk/media/webrtc/webrtcvideoengine2_unittest.h b/talk/media/webrtc/webrtcvideoengine2_unittest.h index b5baa341d..54e6f0621 100644 --- a/talk/media/webrtc/webrtcvideoengine2_unittest.h +++ b/talk/media/webrtc/webrtcvideoengine2_unittest.h @@ -39,11 +39,13 @@ namespace cricket { class FakeVideoSendStream : public webrtc::VideoSendStream { public: FakeVideoSendStream(const webrtc::VideoSendStream::Config& config, - const std::vector& video_streams); + const std::vector& video_streams, + const void* encoder_settings); webrtc::VideoSendStream::Config GetConfig(); std::vector GetVideoStreams(); bool IsSending() const; + bool GetVp8Settings(webrtc::VideoCodecVP8* settings) const; private: virtual webrtc::VideoSendStream::Stats GetStats() const OVERRIDE; @@ -60,6 +62,8 @@ class FakeVideoSendStream : public webrtc::VideoSendStream { bool sending_; webrtc::VideoSendStream::Config config_; std::vector video_streams_; + bool codec_settings_set_; + webrtc::VideoCodecVP8 vp8_settings_; }; class FakeVideoReceiveStream : public webrtc::VideoReceiveStream {