From 5bf8ccdfa0db5763beeadd09e10a770f167c6fb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20M=C3=B6ller?= Date: Thu, 15 Mar 2018 14:16:11 +0100 Subject: [PATCH] Delete encoder caching in WebRtcVideoSendStream. This is a followup to https://webrtc-review.googlesource.com/61640, which ensures that picture id and tl0 pic idx are continuous, independent of how the encoder objects are created and destroyed. The plan is to later move responsibility for encoder creation to VideoSendStream::ReconfigureVideoEncoder, delegating work to VideoStreamEncoder. Bug: webrtc:8830 Change-Id: Idde5c91f24d3c0e3fa6a3bb26eb06f6800896a28 Reviewed-on: https://webrtc-review.googlesource.com/62082 Reviewed-by: Magnus Jedvert Commit-Queue: Niels Moller Cr-Commit-Position: refs/heads/master@{#22473} --- media/engine/webrtcvideoengine.cc | 49 ++++++++++++------------------- media/engine/webrtcvideoengine.h | 4 +-- 2 files changed, 20 insertions(+), 33 deletions(-) diff --git a/media/engine/webrtcvideoengine.cc b/media/engine/webrtcvideoengine.cc index d0516eedaf..bc90c8fcf1 100644 --- a/media/engine/webrtcvideoengine.cc +++ b/media/engine/webrtcvideoengine.cc @@ -1619,8 +1619,7 @@ WebRtcVideoChannel::WebRtcVideoSendStream::WebRtcVideoSendStream( ? webrtc::RtcpMode::kReducedSize : webrtc::RtcpMode::kCompound; if (codec_settings) { - bool force_encoder_allocation = false; - SetCodec(*codec_settings, force_encoder_allocation); + SetCodec(*codec_settings); } } @@ -1651,8 +1650,7 @@ bool WebRtcVideoChannel::WebRtcVideoSendStream::SetVideoSend( // If screen content settings change, we may need to recreate the codec // instance so that the correct type is used. - bool force_encoder_allocation = true; - SetCodec(*parameters_.codec_settings, force_encoder_allocation); + SetCodec(*parameters_.codec_settings); // Mark screenshare parameter as being updated, then test for any other // changes that may require codec reconfiguration. old_options.is_screencast = options->is_screencast; @@ -1701,33 +1699,27 @@ WebRtcVideoChannel::WebRtcVideoSendStream::GetSsrcs() const { } void WebRtcVideoChannel::WebRtcVideoSendStream::SetCodec( - const VideoCodecSettings& codec_settings, - bool force_encoder_allocation) { + const VideoCodecSettings& codec_settings) { RTC_DCHECK_RUN_ON(&thread_checker_); parameters_.encoder_config = CreateVideoEncoderConfig(codec_settings.codec); RTC_DCHECK_GT(parameters_.encoder_config.number_of_streams, 0); - // Do not re-create encoders of the same type. We can't overwrite - // |allocated_encoder_| immediately, because we need to release it after the - // RecreateWebRtcStream() call. - std::unique_ptr new_encoder; - if (force_encoder_allocation || !allocated_encoder_ || - allocated_codec_ != codec_settings.codec) { - const webrtc::SdpVideoFormat format(codec_settings.codec.name, - codec_settings.codec.params); - new_encoder = encoder_factory_->CreateVideoEncoder(format); + const webrtc::SdpVideoFormat format(codec_settings.codec.name, + codec_settings.codec.params); + // We can't overwrite |allocated_encoder_| immediately, because we + // need to release it after the RecreateWebRtcStream() call. + std::unique_ptr new_encoder = + encoder_factory_->CreateVideoEncoder(format); - parameters_.config.encoder_settings.encoder = new_encoder.get(); + parameters_.config.encoder_settings.encoder = new_encoder.get(); + + const webrtc::VideoEncoderFactory::CodecInfo info = + encoder_factory_->QueryVideoEncoder(format); + parameters_.config.encoder_settings.full_overuse_time = + info.is_hardware_accelerated; + parameters_.config.encoder_settings.internal_source = + info.has_internal_source; - const webrtc::VideoEncoderFactory::CodecInfo info = - encoder_factory_->QueryVideoEncoder(format); - parameters_.config.encoder_settings.full_overuse_time = - info.is_hardware_accelerated; - parameters_.config.encoder_settings.internal_source = - info.has_internal_source; - } else { - new_encoder = std::move(allocated_encoder_); - } parameters_.config.encoder_settings.payload_name = codec_settings.codec.name; parameters_.config.encoder_settings.payload_type = codec_settings.codec.id; parameters_.config.rtp.ulpfec = codec_settings.ulpfec; @@ -1754,7 +1746,6 @@ void WebRtcVideoChannel::WebRtcVideoSendStream::SetCodec( RTC_LOG(LS_INFO) << "RecreateWebRtcStream (send) because of SetCodec."; RecreateWebRtcStream(); allocated_encoder_ = std::move(new_encoder); - allocated_codec_ = codec_settings.codec; } void WebRtcVideoChannel::WebRtcVideoSendStream::SetSendParameters( @@ -1781,12 +1772,10 @@ void WebRtcVideoChannel::WebRtcVideoSendStream::SetSendParameters( // Set codecs and options. if (params.codec) { - bool force_encoder_allocation = false; - SetCodec(*params.codec, force_encoder_allocation); + SetCodec(*params.codec); recreate_stream = false; // SetCodec has already recreated the stream. } else if (params.conference_mode && parameters_.codec_settings) { - bool force_encoder_allocation = false; - SetCodec(*parameters_.codec_settings, force_encoder_allocation); + SetCodec(*parameters_.codec_settings); recreate_stream = false; // SetCodec has already recreated the stream. } if (recreate_stream) { diff --git a/media/engine/webrtcvideoengine.h b/media/engine/webrtcvideoengine.h index f481b8bd5f..04096e66c7 100644 --- a/media/engine/webrtcvideoengine.h +++ b/media/engine/webrtcvideoengine.h @@ -312,8 +312,7 @@ class WebRtcVideoChannel : public VideoMediaChannel, public webrtc::Transport { rtc::scoped_refptr ConfigureVideoEncoderSettings(const VideoCodec& codec); - void SetCodec(const VideoCodecSettings& codec, - bool force_encoder_allocation); + void SetCodec(const VideoCodecSettings& codec); void RecreateWebRtcStream(); webrtc::VideoEncoderConfig CreateVideoEncoderConfig( const VideoCodec& codec) const; @@ -355,7 +354,6 @@ class WebRtcVideoChannel : public VideoMediaChannel, public webrtc::Transport { webrtc::RtpParameters rtp_parameters_ RTC_GUARDED_BY(&thread_checker_); std::unique_ptr allocated_encoder_ RTC_GUARDED_BY(&thread_checker_); - VideoCodec allocated_codec_ RTC_GUARDED_BY(&thread_checker_); bool sending_ RTC_GUARDED_BY(&thread_checker_); };