Reland "Reland "Move rtp-specific config out of EncoderSettings.""
This reverts commit 6c2c13af06b32778b86950681758a7970d1c5d9e.
Reason for revert: Intend to investigate and fix perf problems.
Original change's description:
> Revert "Reland "Move rtp-specific config out of EncoderSettings.""
>
> This reverts commit 04dd1768625eb2241d1fb97fd0137897e703e266.
>
> Reason for revert: Regression in ramp up perf tests.
>
> Original change's description:
> > Reland "Move rtp-specific config out of EncoderSettings."
> >
> > This is a reland of bc900cb1d1810fcf678fe41cf1e3966daa39c88c
> >
> > Original change's description:
> > > Move rtp-specific config out of EncoderSettings.
> > >
> > > In VideoSendStream::Config, move payload_name and payload_type from
> > > EncoderSettings to Rtp.
> > >
> > > EncoderSettings now contains configuration for VideoStreamEncoder only,
> > > and should perhaps be renamed in a follow up cl. It's no longer
> > > passed as an argument to VideoCodecInitializer::SetupCodec.
> > >
> > > The latter then needs a different way to know the codec type,
> > > which is provided by a new codec_type member in VideoEncoderConfig.
> > >
> > > Bug: webrtc:8830
> > > Change-Id: Ifcc691aef1ee6a95e43c0452c5e630d92a511cd6
> > > Reviewed-on: https://webrtc-review.googlesource.com/62062
> > > Commit-Queue: Niels Moller <nisse@webrtc.org>
> > > Reviewed-by: Magnus Jedvert <magjed@webrtc.org>
> > > Reviewed-by: Stefan Holmer <stefan@webrtc.org>
> > > Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
> > > Cr-Commit-Position: refs/heads/master@{#22532}
> >
> > Bug: webrtc:8830
> > Change-Id: If88ef7d57cdaa4fae3c7b2a97ea5a6e1b833e019
> > Reviewed-on: https://webrtc-review.googlesource.com/63721
> > Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
> > Reviewed-by: Stefan Holmer <stefan@webrtc.org>
> > Commit-Queue: Niels Moller <nisse@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#22595}
>
> TBR=brandtr@webrtc.org,magjed@webrtc.org,nisse@webrtc.org,stefan@webrtc.org
>
> Bug: webrtc:8830,chromium:827080
> Change-Id: Iaaf146de91ec5c0d741b8efdf143f7e173084fef
> Reviewed-on: https://webrtc-review.googlesource.com/65520
> Commit-Queue: Niels Moller <nisse@webrtc.org>
> Reviewed-by: Niels Moller <nisse@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#22677}
TBR=brandtr@webrtc.org,magjed@webrtc.org,nisse@webrtc.org,stefan@webrtc.org
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: webrtc:8830, chromium:827080
Change-Id: I9b62987bf5daced90dfeb3ebb6739c80117c487f
Reviewed-on: https://webrtc-review.googlesource.com/66862
Commit-Queue: Niels Moller <nisse@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22751}diff --git a/modules/video_coding/include/video_codec_initializer.h b/modules/video_coding/include/video_codec_initializer.h
index 9b91aa9..498cb3d 100644
--- a/modules/video_coding/include/video_codec_initializer.h
+++ b/modules/video_coding/include/video_codec_initializer.h
@@ -13,8 +13,10 @@
#include <memory>
#include <string>
+#include <utility>
#include <vector>
+#include "call/video_config.h"
#include "call/video_send_stream.h"
namespace webrtc {
@@ -33,12 +35,25 @@
// GetBitrateAllocator is called implicitly from here, no need to call again.
static bool SetupCodec(
const VideoEncoderConfig& config,
- const VideoSendStream::Config::EncoderSettings settings,
const std::vector<VideoStream>& streams,
bool nack_enabled,
VideoCodec* codec,
std::unique_ptr<VideoBitrateAllocator>* bitrate_allocator);
+ // TODO(nisse): Deprecated version, with an additional ignored argument.
+ // Delete as soon as downstream users are updated, together with above
+ // includes of "call/video_send_stream.h" and <utility>.
+ static bool SetupCodec(
+ const VideoEncoderConfig& config,
+ const VideoSendStream::Config::EncoderSettings /* settings */,
+ const std::vector<VideoStream>& streams,
+ bool nack_enabled,
+ VideoCodec* codec,
+ std::unique_ptr<VideoBitrateAllocator>* bitrate_allocator) {
+ return SetupCodec(config, streams, nack_enabled, codec,
+ std::move(bitrate_allocator));
+}
+
// Create a bitrate allocator for the specified codec. |tl_factory| is
// optional, if it is populated, ownership of that instance will be
// transferred to the VideoBitrateAllocator instance.
@@ -49,8 +64,6 @@
static VideoCodec VideoEncoderConfigToVideoCodec(
const VideoEncoderConfig& config,
const std::vector<VideoStream>& streams,
- // TODO(nisse): Delete when we can rely on config.codec_type.
- VideoCodecType codec_type,
bool nack_enabled);
};
diff --git a/modules/video_coding/video_codec_initializer.cc b/modules/video_coding/video_codec_initializer.cc
index e0c40c1..5ba659d 100644
--- a/modules/video_coding/video_codec_initializer.cc
+++ b/modules/video_coding/video_codec_initializer.cc
@@ -28,23 +28,15 @@
bool VideoCodecInitializer::SetupCodec(
const VideoEncoderConfig& config,
- const VideoSendStream::Config::EncoderSettings settings,
const std::vector<VideoStream>& streams,
bool nack_enabled,
VideoCodec* codec,
std::unique_ptr<VideoBitrateAllocator>* bitrate_allocator) {
- VideoCodecType codec_type = config.codec_type;
- // TODO(nisse): Transition hack, the intention is to delete the
- // |settings| argument and require configuration via
- // config.codec_type.
- if (codec_type == kVideoCodecUnknown) {
- codec_type = PayloadStringToCodecType(settings.payload_name);
- }
- if (codec_type == kVideoCodecMultiplex) {
+ if (config.codec_type == kVideoCodecMultiplex) {
VideoEncoderConfig associated_config = config.Copy();
associated_config.codec_type = kVideoCodecVP9;
- if (!SetupCodec(associated_config, settings /* ignored */, streams,
- nack_enabled, codec, bitrate_allocator)) {
+ if (!SetupCodec(associated_config, streams, nack_enabled, codec,
+ bitrate_allocator)) {
RTC_LOG(LS_ERROR) << "Failed to create stereo encoder configuration.";
return false;
}
@@ -53,7 +45,7 @@
}
*codec =
- VideoEncoderConfigToVideoCodec(config, streams, codec_type, nack_enabled);
+ VideoEncoderConfigToVideoCodec(config, streams, nack_enabled);
*bitrate_allocator = CreateBitrateAllocator(*codec);
return true;
@@ -82,7 +74,6 @@
VideoCodec VideoCodecInitializer::VideoEncoderConfigToVideoCodec(
const VideoEncoderConfig& config,
const std::vector<VideoStream>& streams,
- VideoCodecType codec_type,
bool nack_enabled) {
static const int kEncoderMinBitrateKbps = 30;
RTC_DCHECK(!streams.empty());
@@ -90,7 +81,7 @@
VideoCodec video_codec;
memset(&video_codec, 0, sizeof(video_codec));
- video_codec.codecType = codec_type;
+ video_codec.codecType = config.codec_type;
switch (config.content_type) {
case VideoEncoderConfig::ContentType::kRealtimeVideo:
diff --git a/modules/video_coding/video_codec_initializer_unittest.cc b/modules/video_coding/video_codec_initializer_unittest.cc
index 2b3103d..4a39399 100644
--- a/modules/video_coding/video_codec_initializer_unittest.cc
+++ b/modules/video_coding/video_codec_initializer_unittest.cc
@@ -20,10 +20,6 @@
namespace webrtc {
namespace {
-static const char* kVp8PayloadName = "VP8";
-static const int kVp8PayloadType = 100;
-static const char* kVp9PayloadName = "VP9";
-static const int kVp9PayloadType = 120;
static const int kDefaultWidth = 1280;
static const int kDefaultHeight = 720;
static const int kDefaultFrameRate = 30;
@@ -52,6 +48,8 @@
int num_temporal_streams,
bool screenshare) {
config_ = VideoEncoderConfig();
+ config_.codec_type = type;
+
if (screenshare) {
config_.min_transmit_bitrate_bps = kDefaultMinTransmitBitrateBps;
config_.content_type = VideoEncoderConfig::ContentType::kScreen;
@@ -63,16 +61,12 @@
vp8_settings.numberOfTemporalLayers = num_temporal_streams;
config_.encoder_specific_settings = new rtc::RefCountedObject<
webrtc::VideoEncoderConfig::Vp8EncoderSpecificSettings>(vp8_settings);
- settings_.payload_name = kVp8PayloadName;
- settings_.payload_type = kVp8PayloadType;
} else if (type == VideoCodecType::kVideoCodecVP9) {
VideoCodecVP9 vp9_settings = VideoEncoder::GetDefaultVp9Settings();
vp9_settings.numberOfSpatialLayers = num_spatial_streams;
vp9_settings.numberOfTemporalLayers = num_temporal_streams;
config_.encoder_specific_settings = new rtc::RefCountedObject<
webrtc::VideoEncoderConfig::Vp9EncoderSpecificSettings>(vp9_settings);
- settings_.payload_name = kVp9PayloadName;
- settings_.payload_type = kVp9PayloadType;
} else if (type != VideoCodecType::kVideoCodecMultiplex) {
ADD_FAILURE() << "Unexpected codec type: " << type;
}
@@ -82,8 +76,8 @@
codec_out_ = VideoCodec();
bitrate_allocator_out_.reset();
temporal_layers_.clear();
- if (!VideoCodecInitializer::SetupCodec(config_, settings_, streams_,
- nack_enabled_, &codec_out_,
+ if (!VideoCodecInitializer::SetupCodec(config_, streams_, nack_enabled_,
+ &codec_out_,
&bitrate_allocator_out_)) {
return false;
}
@@ -127,7 +121,6 @@
// Input settings.
VideoEncoderConfig config_;
- VideoSendStream::Config::EncoderSettings settings_;
std::vector<VideoStream> streams_;
bool nack_enabled_;
