Revert "Add support for RtpEncodingParameters::max_framerate"
This reverts commit 15be5282e91ba38894e6ad51fe9a35a38a6b7f29.
Reason for revert: crbug.com/1028937
Original change's description:
> Add support for RtpEncodingParameters::max_framerate
>
> This adds the framework support for the max_framerate parameter.
> It doesn't implement it in any encoder yet.
>
> Bug: webrtc:11117
> Change-Id: I329624cc0205c828498d3623a2e13dd3f97e1629
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/160184
> Reviewed-by: Steve Anton <steveanton@webrtc.org>
> Reviewed-by: Erik Språng <sprang@webrtc.org>
> Reviewed-by: Åsa Persson <asapersson@webrtc.org>
> Commit-Queue: Florent Castelli <orphis@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#29907}
TBR=steveanton@webrtc.org,asapersson@webrtc.org,sprang@webrtc.org,orphis@webrtc.org
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: webrtc:11117
Change-Id: Ic44dd36bea66561f0c46e73db89d451cb3e22773
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/160941
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29935}
diff --git a/api/rtp_parameters.h b/api/rtp_parameters.h
index 4af0833..77db960 100644
--- a/api/rtp_parameters.h
+++ b/api/rtp_parameters.h
@@ -447,7 +447,10 @@
absl::optional<int> min_bitrate_bps;
// Specifies the maximum framerate in fps for video.
- absl::optional<double> max_framerate;
+ // TODO(asapersson): Different framerates are not supported per simulcast
+ // layer. If set, the maximum |max_framerate| is currently used.
+ // Not supported for screencast.
+ absl::optional<int> max_framerate;
// Specifies the number of temporal layers for video (if the feature is
// supported by the codec implementation).
diff --git a/api/video_codecs/video_codec.cc b/api/video_codecs/video_codec.cc
index 15a3a66..b841575 100644
--- a/api/video_codecs/video_codec.cc
+++ b/api/video_codecs/video_codec.cc
@@ -48,7 +48,6 @@
bool SpatialLayer::operator==(const SpatialLayer& other) const {
return (width == other.width && height == other.height &&
- maxFramerate == other.maxFramerate &&
numberOfTemporalLayers == other.numberOfTemporalLayers &&
maxBitrate == other.maxBitrate &&
targetBitrate == other.targetBitrate &&
diff --git a/media/base/media_engine.cc b/media/base/media_engine.cc
index 44ca3a9..bf5e959 100644
--- a/media/base/media_engine.cc
+++ b/media/base/media_engine.cc
@@ -70,13 +70,7 @@
LOG_AND_RETURN_ERROR(
RTCErrorType::INVALID_RANGE,
"Attempted to set RtpParameters scale_resolution_down_by to an "
- "invalid value. scale_resolution_down_by must be >= 1.0");
- }
- if (rtp_parameters.encodings[i].max_framerate &&
- *rtp_parameters.encodings[i].max_framerate < 0.0) {
- LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_RANGE,
- "Attempted to set RtpParameters max_framerate to an "
- "invalid value. max_framerate must be >= 0.0");
+ "invalid number. scale_resolution_down_by must be >= 1.0");
}
if (rtp_parameters.encodings[i].min_bitrate_bps &&
rtp_parameters.encodings[i].max_bitrate_bps) {
diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc
index f36314f..cab8e29 100644
--- a/media/engine/webrtc_video_engine.cc
+++ b/media/engine/webrtc_video_engine.cc
@@ -146,6 +146,18 @@
: std::vector<VideoCodec>();
}
+int GetMaxFramerate(const webrtc::VideoEncoderConfig& encoder_config,
+ size_t num_layers) {
+ int max_fps = -1;
+ for (size_t i = 0; i < num_layers; ++i) {
+ int fps = (encoder_config.simulcast_layers[i].max_framerate > 0)
+ ? encoder_config.simulcast_layers[i].max_framerate
+ : kDefaultVideoMaxFramerate;
+ max_fps = std::max(fps, max_fps);
+ }
+ return max_fps;
+}
+
bool IsTemporalLayersSupported(const std::string& codec_name) {
return absl::EqualsIgnoreCase(codec_name, kVp8CodecName) ||
absl::EqualsIgnoreCase(codec_name, kVp9CodecName);
@@ -296,12 +308,6 @@
return std::min(a, b);
}
-bool IsLayerActive(const webrtc::RtpEncodingParameters& layer) {
- return layer.active &&
- (!layer.max_bitrate_bps || *layer.max_bitrate_bps > 0) &&
- (!layer.max_framerate || *layer.max_framerate > 0);
-}
-
} // namespace
// This constant is really an on/off, lower-level configurable NACK history
@@ -2066,9 +2072,8 @@
// allocator and the video bitrate allocator.
bool new_send_state = false;
for (size_t i = 0; i < rtp_parameters_.encodings.size(); ++i) {
- bool new_active = IsLayerActive(new_parameters.encodings[i]);
- bool old_active = IsLayerActive(rtp_parameters_.encodings[i]);
- if (new_active != old_active) {
+ if (new_parameters.encodings[i].active !=
+ rtp_parameters_.encodings[i].active) {
new_send_state = true;
}
}
@@ -2116,7 +2121,7 @@
}
std::vector<bool> active_layers(num_layers);
for (size_t i = 0; i < num_layers; ++i) {
- active_layers[i] = IsLayerActive(rtp_parameters_.encodings[i]);
+ active_layers[i] = rtp_parameters_.encodings[i].active;
}
// This updates what simulcast layers are sending, and possibly starts
// or stops the VideoSendStream.
@@ -3041,6 +3046,8 @@
layers[0].min_bitrate_bps =
rtc::saturated_cast<int>(experimental_min_bitrate->bps());
}
+ // The maximum |max_framerate| is currently used for video.
+ const int max_framerate = GetMaxFramerate(encoder_config, layers.size());
// Update the active simulcast layers and configured bitrates.
bool is_highest_layer_max_bitrate_configured = false;
const bool has_scale_resolution_down_by = absl::c_any_of(
@@ -3053,16 +3060,16 @@
NormalizeSimulcastSize(height, encoder_config.number_of_streams);
for (size_t i = 0; i < layers.size(); ++i) {
layers[i].active = encoder_config.simulcast_layers[i].active;
+ if (!is_screenshare_) {
+ // Update simulcast framerates with max configured max framerate.
+ layers[i].max_framerate = max_framerate;
+ }
// Update with configured num temporal layers if supported by codec.
if (encoder_config.simulcast_layers[i].num_temporal_layers &&
IsTemporalLayersSupported(codec_name_)) {
layers[i].num_temporal_layers =
*encoder_config.simulcast_layers[i].num_temporal_layers;
}
- if (encoder_config.simulcast_layers[i].max_framerate > 0) {
- layers[i].max_framerate =
- encoder_config.simulcast_layers[i].max_framerate;
- }
if (has_scale_resolution_down_by) {
const double scale_resolution_down_by = std::max(
encoder_config.simulcast_layers[i].scale_resolution_down_by, 1.0);
diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc
index 5c24454..8870cd6 100644
--- a/media/engine/webrtc_video_engine_unittest.cc
+++ b/media/engine/webrtc_video_engine_unittest.cc
@@ -6566,6 +6566,47 @@
EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, nullptr, nullptr));
}
+TEST_F(WebRtcVideoChannelTest, MaxSimulcastFrameratePropagatedToEncoder) {
+ const size_t kNumSimulcastStreams = 3;
+ FakeVideoSendStream* stream = SetUpSimulcast(true, false);
+
+ // Send a full size frame so all simulcast layers are used when reconfiguring.
+ webrtc::test::FrameForwarder frame_forwarder;
+ VideoOptions options;
+ EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, &options, &frame_forwarder));
+ channel_->SetSend(true);
+ frame_forwarder.IncomingCapturedFrame(frame_source_.GetFrame());
+
+ // Get and set the rtp encoding parameters.
+ // Change the value and set it on the VideoChannel.
+ webrtc::RtpParameters parameters = channel_->GetRtpSendParameters(last_ssrc_);
+ EXPECT_EQ(kNumSimulcastStreams, parameters.encodings.size());
+ parameters.encodings[0].max_framerate = 15;
+ parameters.encodings[1].max_framerate = 25;
+ parameters.encodings[2].max_framerate = 20;
+ EXPECT_TRUE(channel_->SetRtpSendParameters(last_ssrc_, parameters).ok());
+
+ // Verify that the new value propagated down to the encoder.
+ // Check that WebRtcVideoSendStream updates VideoEncoderConfig correctly.
+ EXPECT_EQ(2, stream->num_encoder_reconfigurations());
+ webrtc::VideoEncoderConfig encoder_config = stream->GetEncoderConfig().Copy();
+ EXPECT_EQ(kNumSimulcastStreams, encoder_config.number_of_streams);
+ EXPECT_EQ(kNumSimulcastStreams, encoder_config.simulcast_layers.size());
+ EXPECT_EQ(15, encoder_config.simulcast_layers[0].max_framerate);
+ EXPECT_EQ(25, encoder_config.simulcast_layers[1].max_framerate);
+ EXPECT_EQ(20, encoder_config.simulcast_layers[2].max_framerate);
+
+ // FakeVideoSendStream calls CreateEncoderStreams, test that the vector of
+ // VideoStreams are created appropriately for the simulcast case.
+ // Currently the maximum |max_framerate| is used.
+ EXPECT_EQ(kNumSimulcastStreams, stream->GetVideoStreams().size());
+ EXPECT_EQ(25, stream->GetVideoStreams()[0].max_framerate);
+ EXPECT_EQ(25, stream->GetVideoStreams()[1].max_framerate);
+ EXPECT_EQ(25, stream->GetVideoStreams()[2].max_framerate);
+
+ EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, nullptr, nullptr));
+}
+
TEST_F(WebRtcVideoChannelTest,
DefaultValuePropagatedToEncoderForUnsetFramerate) {
const size_t kNumSimulcastStreams = 3;
@@ -6600,10 +6641,12 @@
// VideoStreams are created appropriately for the simulcast case.
// The maximum |max_framerate| is used, kDefaultVideoMaxFramerate: 60.
EXPECT_EQ(kNumSimulcastStreams, stream->GetVideoStreams().size());
- EXPECT_EQ(15, stream->GetVideoStreams()[0].max_framerate);
+ EXPECT_EQ(kDefaultVideoMaxFramerate,
+ stream->GetVideoStreams()[0].max_framerate);
EXPECT_EQ(kDefaultVideoMaxFramerate,
stream->GetVideoStreams()[1].max_framerate);
- EXPECT_EQ(20, stream->GetVideoStreams()[2].max_framerate);
+ EXPECT_EQ(kDefaultVideoMaxFramerate,
+ stream->GetVideoStreams()[2].max_framerate);
EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, nullptr, nullptr));
}
diff --git a/modules/video_coding/utility/simulcast_test_fixture_impl.cc b/modules/video_coding/utility/simulcast_test_fixture_impl.cc
index f1577341..f091636 100644
--- a/modules/video_coding/utility/simulcast_test_fixture_impl.cc
+++ b/modules/video_coding/utility/simulcast_test_fixture_impl.cc
@@ -43,7 +43,6 @@
const int kMaxBitrates[kNumberOfSimulcastStreams] = {150, 600, 1200};
const int kMinBitrates[kNumberOfSimulcastStreams] = {50, 150, 600};
const int kTargetBitrates[kNumberOfSimulcastStreams] = {100, 450, 1000};
-const float kMaxFramerates[kNumberOfSimulcastStreams] = {30, 30, 30};
const int kDefaultTemporalLayerProfile[3] = {3, 3, 3};
const int kNoTemporalLayerProfile[3] = {0, 0, 0};
@@ -196,7 +195,6 @@
int max_bitrate,
int min_bitrate,
int target_bitrate,
- float max_framerate,
SimulcastStream* stream,
int num_temporal_layers) {
assert(stream);
@@ -205,7 +203,6 @@
stream->maxBitrate = max_bitrate;
stream->minBitrate = min_bitrate;
stream->targetBitrate = target_bitrate;
- stream->maxFramerate = max_framerate;
if (num_temporal_layers >= 0) {
stream->numberOfTemporalLayers = num_temporal_layers;
}
@@ -242,15 +239,15 @@
settings->timing_frame_thresholds = {kDefaultTimingFramesDelayMs,
kDefaultOutlierFrameSizePercent};
ConfigureStream(kDefaultWidth / 4, kDefaultHeight / 4, kMaxBitrates[0],
- kMinBitrates[0], kTargetBitrates[0], kMaxFramerates[0],
+ kMinBitrates[0], kTargetBitrates[0],
&settings->simulcastStream[layer_order[0]],
temporal_layer_profile[0]);
ConfigureStream(kDefaultWidth / 2, kDefaultHeight / 2, kMaxBitrates[1],
- kMinBitrates[1], kTargetBitrates[1], kMaxFramerates[1],
+ kMinBitrates[1], kTargetBitrates[1],
&settings->simulcastStream[layer_order[1]],
temporal_layer_profile[1]);
ConfigureStream(kDefaultWidth, kDefaultHeight, kMaxBitrates[2],
- kMinBitrates[2], kTargetBitrates[2], kMaxFramerates[2],
+ kMinBitrates[2], kTargetBitrates[2],
&settings->simulcastStream[layer_order[2]],
temporal_layer_profile[2]);
if (codec_type == kVideoCodecVP8) {
diff --git a/modules/video_coding/video_codec_initializer.cc b/modules/video_coding/video_codec_initializer.cc
index ea5de23..1ede93b 100644
--- a/modules/video_coding/video_codec_initializer.cc
+++ b/modules/video_coding/video_codec_initializer.cc
@@ -89,13 +89,17 @@
kDefaultOutlierFrameSizePercent};
RTC_DCHECK_LE(streams.size(), kMaxSimulcastStreams);
- int max_framerate = 0;
-
for (size_t i = 0; i < streams.size(); ++i) {
SimulcastStream* sim_stream = &video_codec.simulcastStream[i];
RTC_DCHECK_GT(streams[i].width, 0);
RTC_DCHECK_GT(streams[i].height, 0);
RTC_DCHECK_GT(streams[i].max_framerate, 0);
+ // Different framerates not supported per stream at the moment, unless it's
+ // screenshare where there is an exception and a simulcast encoder adapter,
+ // which supports different framerates, is used instead.
+ if (config.content_type != VideoEncoderConfig::ContentType::kScreen) {
+ RTC_DCHECK_EQ(streams[i].max_framerate, streams[0].max_framerate);
+ }
RTC_DCHECK_GE(streams[i].min_bitrate_bps, 0);
RTC_DCHECK_GE(streams[i].target_bitrate_bps, streams[i].min_bitrate_bps);
RTC_DCHECK_GE(streams[i].max_bitrate_bps, streams[i].target_bitrate_bps);
@@ -122,7 +126,6 @@
video_codec.maxBitrate += streams[i].max_bitrate_bps / 1000;
video_codec.qpMax = std::max(video_codec.qpMax,
static_cast<unsigned int>(streams[i].max_qp));
- max_framerate = std::max(max_framerate, streams[i].max_framerate);
}
if (video_codec.maxBitrate == 0) {
@@ -134,7 +137,8 @@
if (video_codec.maxBitrate < kEncoderMinBitrateKbps)
video_codec.maxBitrate = kEncoderMinBitrateKbps;
- video_codec.maxFramerate = max_framerate;
+ RTC_DCHECK_GT(streams[0].max_framerate, 0);
+ video_codec.maxFramerate = streams[0].max_framerate;
// Set codec specific options
if (config.encoder_specific_settings)
diff --git a/pc/rtp_sender_receiver_unittest.cc b/pc/rtp_sender_receiver_unittest.cc
index fb6bad4..9026cfc 100644
--- a/pc/rtp_sender_receiver_unittest.cc
+++ b/pc/rtp_sender_receiver_unittest.cc
@@ -1309,43 +1309,6 @@
DestroyVideoRtpSender();
}
-TEST_F(RtpSenderReceiverTest, VideoSenderCanSetMaxFramerate) {
- CreateVideoRtpSender();
-
- RtpParameters params = video_rtp_sender_->GetParameters();
- params.encodings[0].max_framerate = 20;
-
- EXPECT_TRUE(video_rtp_sender_->SetParameters(params).ok());
- params = video_rtp_sender_->GetParameters();
- EXPECT_EQ(20., params.encodings[0].max_framerate);
-
- DestroyVideoRtpSender();
-}
-
-TEST_F(RtpSenderReceiverTest, VideoSenderCanSetMaxFramerateZero) {
- CreateVideoRtpSender();
-
- RtpParameters params = video_rtp_sender_->GetParameters();
- params.encodings[0].max_framerate = 0.;
-
- EXPECT_TRUE(video_rtp_sender_->SetParameters(params).ok());
- params = video_rtp_sender_->GetParameters();
- EXPECT_EQ(0., params.encodings[0].max_framerate);
-
- DestroyVideoRtpSender();
-}
-
-TEST_F(RtpSenderReceiverTest, VideoSenderDetectInvalidMaxFramerate) {
- CreateVideoRtpSender();
-
- RtpParameters params = video_rtp_sender_->GetParameters();
- params.encodings[0].max_framerate = -5.;
- RTCError result = video_rtp_sender_->SetParameters(params);
- EXPECT_EQ(RTCErrorType::INVALID_RANGE, result.type());
-
- DestroyVideoRtpSender();
-}
-
TEST_F(RtpSenderReceiverTest,
VideoSenderCantSetUnimplementedEncodingParametersWithSimulcast) {
CreateVideoRtpSenderWithSimulcast();
diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc
index e84ad6e..458f1ed 100644
--- a/video/video_stream_encoder.cc
+++ b/video/video_stream_encoder.cc
@@ -151,8 +151,6 @@
prev_send_codec.simulcastStream[i].width ||
new_send_codec.simulcastStream[i].height !=
prev_send_codec.simulcastStream[i].height ||
- new_send_codec.simulcastStream[i].maxFramerate !=
- prev_send_codec.simulcastStream[i].maxFramerate ||
new_send_codec.simulcastStream[i].numberOfTemporalLayers !=
prev_send_codec.simulcastStream[i].numberOfTemporalLayers ||
new_send_codec.simulcastStream[i].qpMax !=
@@ -815,7 +813,6 @@
<< " min_bps: " << codec.simulcastStream[i].minBitrate
<< " target_bps: " << codec.simulcastStream[i].targetBitrate
<< " max_bps: " << codec.simulcastStream[i].maxBitrate
- << " max_fps: " << codec.simulcastStream[i].maxFramerate
<< " max_qp: " << codec.simulcastStream[i].qpMax
<< " num_tl: " << codec.simulcastStream[i].numberOfTemporalLayers
<< " active: "
