Reland "Add support for RtpEncodingParameters::max_framerate" Perf test failure was fixed separately. TBR=steveanton@webrtc.org,sprang@webrtc.org,asapersson@webrtc.org Original change's description: > 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} Bug: webrtc:11117 Change-Id: I9c1daf7887c2024c6669dc79bff89d737417458c Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/161445 Reviewed-by: Florent Castelli <orphis@webrtc.org> Commit-Queue: Florent Castelli <orphis@webrtc.org> Cr-Commit-Position: refs/heads/master@{#30030}
diff --git a/api/rtp_parameters.h b/api/rtp_parameters.h index 124abc9..342ef0f 100644 --- a/api/rtp_parameters.h +++ b/api/rtp_parameters.h
@@ -414,10 +414,7 @@ absl::optional<int> min_bitrate_bps; // Specifies the maximum framerate in fps for video. - // 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; + absl::optional<double> 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 a710243..d03082b 100644 --- a/api/video_codecs/video_codec.cc +++ b/api/video_codecs/video_codec.cc
@@ -58,6 +58,7 @@ 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 bf5e959..44ca3a9 100644 --- a/media/base/media_engine.cc +++ b/media/base/media_engine.cc
@@ -70,7 +70,13 @@ LOG_AND_RETURN_ERROR( RTCErrorType::INVALID_RANGE, "Attempted to set RtpParameters scale_resolution_down_by to an " - "invalid number. scale_resolution_down_by must be >= 1.0"); + "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"); } 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 a5afcb3..e3ac88b 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc
@@ -146,18 +146,6 @@ : 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); @@ -308,6 +296,12 @@ 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 @@ -2049,8 +2043,9 @@ // allocator and the video bitrate allocator. bool new_send_state = false; for (size_t i = 0; i < rtp_parameters_.encodings.size(); ++i) { - if (new_parameters.encodings[i].active != - rtp_parameters_.encodings[i].active) { + bool new_active = IsLayerActive(new_parameters.encodings[i]); + bool old_active = IsLayerActive(rtp_parameters_.encodings[i]); + if (new_active != old_active) { new_send_state = true; } } @@ -2098,7 +2093,7 @@ } std::vector<bool> active_layers(num_layers); for (size_t i = 0; i < num_layers; ++i) { - active_layers[i] = rtp_parameters_.encodings[i].active; + active_layers[i] = IsLayerActive(rtp_parameters_.encodings[i]); } // This updates what simulcast layers are sending, and possibly starts // or stops the VideoSendStream. @@ -3119,8 +3114,6 @@ 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( @@ -3133,16 +3126,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 0270355..2dd6c54 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc
@@ -6718,47 +6718,6 @@ 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; @@ -6793,12 +6752,10 @@ // VideoStreams are created appropriately for the simulcast case. // The maximum |max_framerate| is used, kDefaultVideoMaxFramerate: 60. EXPECT_EQ(kNumSimulcastStreams, stream->GetVideoStreams().size()); - EXPECT_EQ(kDefaultVideoMaxFramerate, - stream->GetVideoStreams()[0].max_framerate); + EXPECT_EQ(15, stream->GetVideoStreams()[0].max_framerate); EXPECT_EQ(kDefaultVideoMaxFramerate, stream->GetVideoStreams()[1].max_framerate); - EXPECT_EQ(kDefaultVideoMaxFramerate, - stream->GetVideoStreams()[2].max_framerate); + EXPECT_EQ(20, 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 f091636..f1577341 100644 --- a/modules/video_coding/utility/simulcast_test_fixture_impl.cc +++ b/modules/video_coding/utility/simulcast_test_fixture_impl.cc
@@ -43,6 +43,7 @@ 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}; @@ -195,6 +196,7 @@ int max_bitrate, int min_bitrate, int target_bitrate, + float max_framerate, SimulcastStream* stream, int num_temporal_layers) { assert(stream); @@ -203,6 +205,7 @@ 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; } @@ -239,15 +242,15 @@ settings->timing_frame_thresholds = {kDefaultTimingFramesDelayMs, kDefaultOutlierFrameSizePercent}; ConfigureStream(kDefaultWidth / 4, kDefaultHeight / 4, kMaxBitrates[0], - kMinBitrates[0], kTargetBitrates[0], + kMinBitrates[0], kTargetBitrates[0], kMaxFramerates[0], &settings->simulcastStream[layer_order[0]], temporal_layer_profile[0]); ConfigureStream(kDefaultWidth / 2, kDefaultHeight / 2, kMaxBitrates[1], - kMinBitrates[1], kTargetBitrates[1], + kMinBitrates[1], kTargetBitrates[1], kMaxFramerates[1], &settings->simulcastStream[layer_order[1]], temporal_layer_profile[1]); ConfigureStream(kDefaultWidth, kDefaultHeight, kMaxBitrates[2], - kMinBitrates[2], kTargetBitrates[2], + kMinBitrates[2], kTargetBitrates[2], kMaxFramerates[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 1ede93b..ea5de23 100644 --- a/modules/video_coding/video_codec_initializer.cc +++ b/modules/video_coding/video_codec_initializer.cc
@@ -89,17 +89,13 @@ 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); @@ -126,6 +122,7 @@ 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) { @@ -137,8 +134,7 @@ if (video_codec.maxBitrate < kEncoderMinBitrateKbps) video_codec.maxBitrate = kEncoderMinBitrateKbps; - RTC_DCHECK_GT(streams[0].max_framerate, 0); - video_codec.maxFramerate = streams[0].max_framerate; + video_codec.maxFramerate = 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 2795e6b..9736f18 100644 --- a/pc/rtp_sender_receiver_unittest.cc +++ b/pc/rtp_sender_receiver_unittest.cc
@@ -1229,6 +1229,43 @@ 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(); +} + // A video sender can have multiple simulcast layers, in which case it will // contain multiple RtpEncodingParameters. This tests that if this is the case // (simulcast), then we can't set the bitrate_priority, or max_bitrate_bps
diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index 7879522..f9a17ca 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc
@@ -154,6 +154,8 @@ 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 != @@ -842,6 +844,7 @@ << " 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: "