From 97b4ee5b4c311bcb28ad941e7b50c4668d8aea68 Mon Sep 17 00:00:00 2001 From: Ilya Nikolaevskiy Date: Mon, 28 May 2018 10:24:22 +0200 Subject: [PATCH] Wire up VAAPI VP8 experimental support in WebRTC. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Experiment flag added to PeerConnectionInterface::RtcConfiguration and propagated down to VideoStreamEncoder. Artificial Sdp parameter is added to the sdp format if the flag is set. Additionally, sdp format is propagated in vp8 simulcast adapters. Bug: chromium:794608 Change-Id: I2dec54d19ae7bfbd5f2777ec682da5a84194da94 Reviewed-on: https://webrtc-review.googlesource.com/78500 Commit-Queue: Ilya Nikolaevskiy Reviewed-by: Fredrik Solenberg Reviewed-by: Per Kjellander Reviewed-by: Erik Språng Cr-Commit-Position: refs/heads/master@{#23412} --- api/peerconnectioninterface.h | 10 ++++++++++ .../builtin_video_encoder_factory.cc | 2 +- call/video_send_stream.h | 5 +++++ media/base/mediaconfig.h | 12 +++++++++--- media/engine/convert_legacy_video_factory.cc | 4 ++-- media/engine/fakewebrtcvideoengine.cc | 2 +- media/engine/simulcast_encoder_adapter.cc | 6 ++++-- media/engine/simulcast_encoder_adapter.h | 4 +++- .../engine/simulcast_encoder_adapter_unittest.cc | 5 +++-- media/engine/vp8_encoder_simulcast_proxy.cc | 12 ++++++++---- media/engine/vp8_encoder_simulcast_proxy.h | 7 +++++++ .../vp8_encoder_simulcast_proxy_unittest.cc | 6 ++++-- media/engine/webrtcvideoengine.cc | 2 ++ .../codecs/test/videocodec_test_fixture_impl.cc | 2 +- pc/peerconnectioninterface_unittest.cc | 14 ++++++++++++++ video/picture_id_tests.cc | 6 +++--- video/video_quality_test.cc | 4 ++-- video/video_stream_encoder.cc | 16 ++++++++++++++-- 18 files changed, 93 insertions(+), 26 deletions(-) diff --git a/api/peerconnectioninterface.h b/api/peerconnectioninterface.h index 8c0602945b..fa940936ef 100644 --- a/api/peerconnectioninterface.h +++ b/api/peerconnectioninterface.h @@ -339,6 +339,16 @@ class PeerConnectionInterface : public rtc::RefCountInterface { void set_experiment_cpu_load_estimator(bool enable) { media_config.video.experiment_cpu_load_estimator = enable; } + + // Hardware VP8 encoding using VA-API on intel kaby-lake processors. + // crbug.com/794608 + bool experiment_vaapi_vp8_hw_encoding() const { + return media_config.video.experiment_vaapi_vp8_hw_encoding; + } + void set_experiment_vaapi_vp8_hw_encoding(bool enable) { + media_config.video.experiment_vaapi_vp8_hw_encoding = enable; + } + static const int kUndefined = -1; // Default maximum number of packets in the audio jitter buffer. static const int kAudioJitterBufferMaxPackets = 50; diff --git a/api/video_codecs/builtin_video_encoder_factory.cc b/api/video_codecs/builtin_video_encoder_factory.cc index b86681087c..b8d747ef5c 100644 --- a/api/video_codecs/builtin_video_encoder_factory.cc +++ b/api/video_codecs/builtin_video_encoder_factory.cc @@ -61,7 +61,7 @@ class BuiltinVideoEncoderFactory : public VideoEncoderFactory { internal_encoder = cricket::CodecNamesEq(format.name.c_str(), cricket::kVp8CodecName) ? rtc::MakeUnique( - internal_encoder_factory_.get()) + internal_encoder_factory_.get(), format) : internal_encoder_factory_->CreateVideoEncoder(format); } diff --git a/call/video_send_stream.h b/call/video_send_stream.h index 848902fc2e..d883285904 100644 --- a/call/video_send_stream.h +++ b/call/video_send_stream.h @@ -117,6 +117,11 @@ class VideoSendStream { // cpu adaptation. bool experiment_cpu_load_estimator = false; + // Enables hardware VAAPI VP8 encoding if supported by the provided + // VideoEncoderFactory. + // TODO(ilnik): remove this when VAAPI VP8 experiment is over. + bool experiment_vaapi_vp8_hw_encoding = false; + // Ownership stays with WebrtcVideoEngine (delegated from PeerConnection). VideoEncoderFactory* encoder_factory = nullptr; } encoder_settings; diff --git a/media/base/mediaconfig.h b/media/base/mediaconfig.h index 5e8487155b..e92b45ed29 100644 --- a/media/base/mediaconfig.h +++ b/media/base/mediaconfig.h @@ -59,12 +59,16 @@ struct MediaConfig { // TODO(bugs.webrtc.org/8504): If all goes well, the flag will be removed // together with the old method of estimation. bool experiment_cpu_load_estimator = false; + + // Enables experimental VAAPI VP8 hardware encoder, if supported by the + // provided VideoEncoderFactory. + // TODO(ilnik): remove this when VAAPI VP8 experiment is over. + bool experiment_vaapi_vp8_hw_encoding = false; } video; bool operator==(const MediaConfig& o) const { return enable_dscp == o.enable_dscp && - video.enable_cpu_adaptation == - o.video.enable_cpu_adaptation && + video.enable_cpu_adaptation == o.video.enable_cpu_adaptation && video.suspend_below_min_bitrate == o.video.suspend_below_min_bitrate && video.enable_prerenderer_smoothing == @@ -72,7 +76,9 @@ struct MediaConfig { video.periodic_alr_bandwidth_probing == o.video.periodic_alr_bandwidth_probing && video.experiment_cpu_load_estimator == - o.video.experiment_cpu_load_estimator; + o.video.experiment_cpu_load_estimator && + video.experiment_vaapi_vp8_hw_encoding == + o.video.experiment_vaapi_vp8_hw_encoding; } bool operator!=(const MediaConfig& o) const { return !(*this == o); } diff --git a/media/engine/convert_legacy_video_factory.cc b/media/engine/convert_legacy_video_factory.cc index a5b46f81ab..8f85828a5d 100644 --- a/media/engine/convert_legacy_video_factory.cc +++ b/media/engine/convert_legacy_video_factory.cc @@ -126,7 +126,7 @@ class EncoderAdapter : public webrtc::VideoEncoderFactory { internal_encoder = CodecNamesEq(format.name.c_str(), kVp8CodecName) ? rtc::MakeUnique( - internal_encoder_factory_.get()) + internal_encoder_factory_.get(), format) : internal_encoder_factory_->CreateVideoEncoder(format); } @@ -137,7 +137,7 @@ class EncoderAdapter : public webrtc::VideoEncoderFactory { external_encoder = CodecNamesEq(format.name.c_str(), kVp8CodecName) ? rtc::MakeUnique( - external_encoder_factory_.get()) + external_encoder_factory_.get(), format) : external_encoder_factory_->CreateVideoEncoder(format); } diff --git a/media/engine/fakewebrtcvideoengine.cc b/media/engine/fakewebrtcvideoengine.cc index 18bd14ad89..8bd9ddb514 100644 --- a/media/engine/fakewebrtcvideoengine.cc +++ b/media/engine/fakewebrtcvideoengine.cc @@ -224,7 +224,7 @@ FakeWebRtcVideoEncoderFactory::CreateVideoEncoder( // encoders. Enter vp8_factory_mode so that we now create these encoders // instead of more adapters. vp8_factory_mode_ = true; - encoder = rtc::MakeUnique(this); + encoder = rtc::MakeUnique(this, format); } else { num_created_encoders_++; created_video_encoder_event_.Set(); diff --git a/media/engine/simulcast_encoder_adapter.cc b/media/engine/simulcast_encoder_adapter.cc index 65d60756ec..140bd0c077 100644 --- a/media/engine/simulcast_encoder_adapter.cc +++ b/media/engine/simulcast_encoder_adapter.cc @@ -108,9 +108,11 @@ class AdapterEncodedImageCallback : public webrtc::EncodedImageCallback { namespace webrtc { -SimulcastEncoderAdapter::SimulcastEncoderAdapter(VideoEncoderFactory* factory) +SimulcastEncoderAdapter::SimulcastEncoderAdapter(VideoEncoderFactory* factory, + const SdpVideoFormat& format) : inited_(0), factory_(factory), + video_format_(format), encoded_complete_callback_(nullptr), implementation_name_("SimulcastEncoderAdapter") { RTC_DCHECK(factory_); @@ -218,7 +220,7 @@ int SimulcastEncoderAdapter::InitEncode(const VideoCodec* inst, encoder = std::move(stored_encoders_.top()); stored_encoders_.pop(); } else { - encoder = factory_->CreateVideoEncoder(SdpVideoFormat("VP8")); + encoder = factory_->CreateVideoEncoder(video_format_); } ret = encoder->InitEncode(&stream_codec, number_of_cores, max_payload_size); diff --git a/media/engine/simulcast_encoder_adapter.h b/media/engine/simulcast_encoder_adapter.h index f3361501c4..2b7a9b031c 100644 --- a/media/engine/simulcast_encoder_adapter.h +++ b/media/engine/simulcast_encoder_adapter.h @@ -34,7 +34,8 @@ class VideoEncoderFactory; // interfaces should be called from the encoder task queue. class SimulcastEncoderAdapter : public VP8Encoder { public: - explicit SimulcastEncoderAdapter(VideoEncoderFactory* factory); + explicit SimulcastEncoderAdapter(VideoEncoderFactory* factory, + const SdpVideoFormat& format); virtual ~SimulcastEncoderAdapter(); // Implements VideoEncoder. @@ -98,6 +99,7 @@ class SimulcastEncoderAdapter : public VP8Encoder { volatile int inited_; // Accessed atomically. VideoEncoderFactory* const factory_; + const SdpVideoFormat video_format_; VideoCodec codec_; std::vector streaminfos_; EncodedImageCallback* encoded_complete_callback_; diff --git a/media/engine/simulcast_encoder_adapter_unittest.cc b/media/engine/simulcast_encoder_adapter_unittest.cc index f046bf74ed..9aced296fb 100644 --- a/media/engine/simulcast_encoder_adapter_unittest.cc +++ b/media/engine/simulcast_encoder_adapter_unittest.cc @@ -31,7 +31,8 @@ class TestSimulcastEncoderAdapter : public TestVp8Simulcast { protected: std::unique_ptr CreateEncoder() override { - return rtc::MakeUnique(factory_.get()); + return rtc::MakeUnique(factory_.get(), + SdpVideoFormat("VP8")); } std::unique_ptr CreateDecoder() override { return VP8Decoder::Create(); @@ -251,7 +252,7 @@ class TestSimulcastEncoderAdapterFakeHelper { // Can only be called once as the SimulcastEncoderAdapter will take the // ownership of |factory_|. VP8Encoder* CreateMockEncoderAdapter() { - return new SimulcastEncoderAdapter(factory_.get()); + return new SimulcastEncoderAdapter(factory_.get(), SdpVideoFormat("VP8")); } void ExpectCallSetChannelParameters(uint32_t packetLoss, int64_t rtt) { diff --git a/media/engine/vp8_encoder_simulcast_proxy.cc b/media/engine/vp8_encoder_simulcast_proxy.cc index 94020c332d..4d858630b5 100644 --- a/media/engine/vp8_encoder_simulcast_proxy.cc +++ b/media/engine/vp8_encoder_simulcast_proxy.cc @@ -15,11 +15,15 @@ #include "rtc_base/checks.h" namespace webrtc { -VP8EncoderSimulcastProxy::VP8EncoderSimulcastProxy(VideoEncoderFactory* factory) - : factory_(factory), callback_(nullptr) { - encoder_ = factory_->CreateVideoEncoder(SdpVideoFormat("VP8")); +VP8EncoderSimulcastProxy::VP8EncoderSimulcastProxy(VideoEncoderFactory* factory, + const SdpVideoFormat& format) + : factory_(factory), video_format_(format), callback_(nullptr) { + encoder_ = factory_->CreateVideoEncoder(format); } +VP8EncoderSimulcastProxy::VP8EncoderSimulcastProxy(VideoEncoderFactory* factory) + : VP8EncoderSimulcastProxy(factory, SdpVideoFormat("VP8")) {} + VP8EncoderSimulcastProxy::~VP8EncoderSimulcastProxy() {} int VP8EncoderSimulcastProxy::Release() { @@ -31,7 +35,7 @@ int VP8EncoderSimulcastProxy::InitEncode(const VideoCodec* inst, size_t max_payload_size) { int ret = encoder_->InitEncode(inst, number_of_cores, max_payload_size); if (ret == WEBRTC_VIDEO_CODEC_ERR_SIMULCAST_PARAMETERS_NOT_SUPPORTED) { - encoder_.reset(new SimulcastEncoderAdapter(factory_)); + encoder_.reset(new SimulcastEncoderAdapter(factory_, video_format_)); if (callback_) { encoder_->RegisterEncodeCompleteCallback(callback_); } diff --git a/media/engine/vp8_encoder_simulcast_proxy.h b/media/engine/vp8_encoder_simulcast_proxy.h index 7763ad01a5..11ab0c55ce 100644 --- a/media/engine/vp8_encoder_simulcast_proxy.h +++ b/media/engine/vp8_encoder_simulcast_proxy.h @@ -15,6 +15,7 @@ #include #include +#include "api/video_codecs/sdp_video_format.h" #include "api/video_codecs/video_encoder_factory.h" #include "modules/video_coding/codecs/vp8/include/vp8.h" @@ -24,7 +25,12 @@ namespace webrtc { // doesn't support simulcast for provided settings. class VP8EncoderSimulcastProxy : public VP8Encoder { public: + VP8EncoderSimulcastProxy(VideoEncoderFactory* factory, + const SdpVideoFormat& format); + // Deprecated. Remove once all clients use constructor with both factory and + // SdpVideoFormat; explicit VP8EncoderSimulcastProxy(VideoEncoderFactory* factory); + ~VP8EncoderSimulcastProxy() override; // Implements VideoEncoder. @@ -47,6 +53,7 @@ class VP8EncoderSimulcastProxy : public VP8Encoder { private: VideoEncoderFactory* const factory_; + SdpVideoFormat video_format_; std::unique_ptr encoder_; EncodedImageCallback* callback_; }; diff --git a/media/engine/vp8_encoder_simulcast_proxy_unittest.cc b/media/engine/vp8_encoder_simulcast_proxy_unittest.cc index 6b61b18856..dfa35cbaf6 100644 --- a/media/engine/vp8_encoder_simulcast_proxy_unittest.cc +++ b/media/engine/vp8_encoder_simulcast_proxy_unittest.cc @@ -83,7 +83,8 @@ TEST(VP8EncoderSimulcastProxy, ChoosesCorrectImplementation) { .Times(1) .WillOnce(Return(mock_encoder)); - VP8EncoderSimulcastProxy simulcast_enabled_proxy(&simulcast_factory); + VP8EncoderSimulcastProxy simulcast_enabled_proxy(&simulcast_factory, + SdpVideoFormat("VP8")); EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, simulcast_enabled_proxy.InitEncode(&codec_settings, 4, 1200)); EXPECT_EQ(kImplementationName, simulcast_enabled_proxy.ImplementationName()); @@ -122,7 +123,8 @@ TEST(VP8EncoderSimulcastProxy, ChoosesCorrectImplementation) { .WillOnce(Return(mock_encoder3)) .WillOnce(Return(mock_encoder4)); - VP8EncoderSimulcastProxy simulcast_disabled_proxy(&nonsimulcast_factory); + VP8EncoderSimulcastProxy simulcast_disabled_proxy(&nonsimulcast_factory, + SdpVideoFormat("VP8")); EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, simulcast_disabled_proxy.InitEncode(&codec_settings, 4, 1200)); EXPECT_EQ(kSimulcastAdaptedImplementationName, diff --git a/media/engine/webrtcvideoengine.cc b/media/engine/webrtcvideoengine.cc index 0b89e1f490..ea66bcb995 100644 --- a/media/engine/webrtcvideoengine.cc +++ b/media/engine/webrtcvideoengine.cc @@ -1125,6 +1125,8 @@ bool WebRtcVideoChannel::AddSendStream(const StreamParams& sp) { video_config_.periodic_alr_bandwidth_probing; config.encoder_settings.experiment_cpu_load_estimator = video_config_.experiment_cpu_load_estimator; + config.encoder_settings.experiment_vaapi_vp8_hw_encoding = + video_config_.experiment_vaapi_vp8_hw_encoding; config.encoder_settings.encoder_factory = encoder_factory_; WebRtcVideoSendStream* stream = new WebRtcVideoSendStream( diff --git a/modules/video_coding/codecs/test/videocodec_test_fixture_impl.cc b/modules/video_coding/codecs/test/videocodec_test_fixture_impl.cc index 1b1a6c6381..7a19529075 100644 --- a/modules/video_coding/codecs/test/videocodec_test_fixture_impl.cc +++ b/modules/video_coding/codecs/test/videocodec_test_fixture_impl.cc @@ -562,7 +562,7 @@ void VideoCodecTestFixtureImpl::CreateEncoderAndDecoder() { SdpVideoFormat format(config_.codec_name); if (config_.simulcast_adapted_encoder) { EXPECT_EQ("VP8", format.name); - encoder_.reset(new SimulcastEncoderAdapter(encoder_factory_.get())); + encoder_.reset(new SimulcastEncoderAdapter(encoder_factory_.get(), format)); } else { encoder_ = encoder_factory_->CreateVideoEncoder(format); } diff --git a/pc/peerconnectioninterface_unittest.cc b/pc/peerconnectioninterface_unittest.cc index ef3f317de4..ce4a7abe90 100644 --- a/pc/peerconnectioninterface_unittest.cc +++ b/pc/peerconnectioninterface_unittest.cc @@ -4043,6 +4043,7 @@ TEST_F(PeerConnectionMediaConfigTest, TestDefaults) { EXPECT_TRUE(media_config.video.enable_prerenderer_smoothing); EXPECT_FALSE(media_config.video.suspend_below_min_bitrate); EXPECT_FALSE(media_config.video.experiment_cpu_load_estimator); + EXPECT_FALSE(media_config.video.experiment_vaapi_vp8_hw_encoding); } // This test verifies the DSCP constraint is recognized and passed to @@ -4098,6 +4099,19 @@ TEST_F(PeerConnectionMediaConfigTest, TestEnableExperimentCpuLoadEstimator) { EXPECT_TRUE(media_config.video.experiment_cpu_load_estimator); } +// This test verifies that the experiment_vaapi_vp8_hw_encoding flag is +// propagated from RTCConfiguration to the PeerConnection. +TEST_F(PeerConnectionMediaConfigTest, TestEnableExperimentVaapiVp8HwEncoding) { + PeerConnectionInterface::RTCConfiguration config; + FakeConstraints constraints; + + config.set_experiment_vaapi_vp8_hw_encoding(true); + const cricket::MediaConfig& media_config = + TestCreatePeerConnection(config, &constraints); + + EXPECT_TRUE(media_config.video.experiment_vaapi_vp8_hw_encoding); +} + // This test verifies the suspend below min bitrate constraint is // recognized and passed to the PeerConnection. TEST_F(PeerConnectionMediaConfigTest, diff --git a/video/picture_id_tests.cc b/video/picture_id_tests.cc index 33aed38f18..32853d0942 100644 --- a/video/picture_id_tests.cc +++ b/video/picture_id_tests.cc @@ -419,7 +419,7 @@ TEST_P(PictureIdTest, ContinuousAfterReconfigureSimulcastEncoderAdapter) { test::FunctionVideoEncoderFactory encoder_factory( [&internal_encoder_factory]() { return rtc::MakeUnique( - &internal_encoder_factory); + &internal_encoder_factory, SdpVideoFormat("VP8")); }); SetupEncoder(&encoder_factory, "VP8"); TestPictureIdContinuousAfterReconfigure({1, 3, 3, 1, 1}); @@ -430,7 +430,7 @@ TEST_P(PictureIdTest, IncreasingAfterRecreateStreamSimulcastEncoderAdapter) { test::FunctionVideoEncoderFactory encoder_factory( [&internal_encoder_factory]() { return rtc::MakeUnique( - &internal_encoder_factory); + &internal_encoder_factory, SdpVideoFormat("VP8")); }); SetupEncoder(&encoder_factory, "VP8"); TestPictureIdIncreaseAfterRecreateStreams({1, 3, 3, 1, 1}); @@ -441,7 +441,7 @@ TEST_P(PictureIdTest, ContinuousAfterStreamCountChangeSimulcastEncoderAdapter) { test::FunctionVideoEncoderFactory encoder_factory( [&internal_encoder_factory]() { return rtc::MakeUnique( - &internal_encoder_factory); + &internal_encoder_factory, SdpVideoFormat("VP8")); }); // Make sure that the picture id is not reset if the stream count goes // down and then up. diff --git a/video/video_quality_test.cc b/video/video_quality_test.cc index b87e604055..49414e989d 100644 --- a/video/video_quality_test.cc +++ b/video/video_quality_test.cc @@ -1095,8 +1095,8 @@ std::unique_ptr VideoQualityTest::TestVideoEncoderFactory::CreateVideoEncoder( const SdpVideoFormat& format) { if (format.name == "VP8") { - return rtc::MakeUnique( - &internal_encoder_factory_); + return rtc::MakeUnique(&internal_encoder_factory_, + format); } else if (format.name == "multiplex") { return rtc::MakeUnique( &internal_encoder_factory_, SdpVideoFormat(cricket::kVp9CodecName)); diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index 1c9ad9fbd6..92062855ba 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -36,6 +36,13 @@ namespace webrtc { namespace { +// This artificial SDP parameter is used to pass the experiment status to +// video encoder factory. It's added explicitly before call to +// |CreateVideoEncoder()| with value "Enabled" if experiment is enabled. +// TODO(ilnik): remove this when VAAPI VP8 experiment is over. +const char kExprimentVaapiVp8HwEncodingParameter[] = + "ExprimentVaapiVp8HwEncoding"; + // Time interval for logging frame counts. const int64_t kFrameLogIntervalMs = 60000; const int kMinFramerateFps = 2; @@ -521,8 +528,13 @@ void VideoStreamEncoder::ReconfigureEncoder() { video_sender_.RegisterExternalEncoder(nullptr, false); } - encoder_ = settings_.encoder_factory->CreateVideoEncoder( - encoder_config_.video_format); + SdpVideoFormat video_format = encoder_config_.video_format; + if (video_format.name == "VP8" && + settings_.experiment_vaapi_vp8_hw_encoding) { + video_format.parameters[kExprimentVaapiVp8HwEncodingParameter] = + "Enabled"; + } + encoder_ = settings_.encoder_factory->CreateVideoEncoder(video_format); // TODO(nisse): What to do if creating the encoder fails? Crash, // or just discard incoming frames? RTC_CHECK(encoder_);