diff --git a/api/rtp_parameters.h b/api/rtp_parameters.h index 4af0833c9c..77db960c4c 100644 --- a/api/rtp_parameters.h +++ b/api/rtp_parameters.h @@ -447,7 +447,10 @@ struct RTC_EXPORT RtpEncodingParameters { absl::optional min_bitrate_bps; // Specifies the maximum framerate in fps for video. - absl::optional 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 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 15a3a6662a..b8415753cf 100644 --- a/api/video_codecs/video_codec.cc +++ b/api/video_codecs/video_codec.cc @@ -48,7 +48,6 @@ bool VideoCodecH264::operator==(const VideoCodecH264& other) const { 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 44ca3a9528..bf5e959f81 100644 --- a/media/base/media_engine.cc +++ b/media/base/media_engine.cc @@ -70,13 +70,7 @@ webrtc::RTCError CheckRtpParametersValues( 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 f36314fdd3..cab8e294ee 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -146,6 +146,18 @@ std::vector AssignPayloadTypesAndDefaultCodecs( : std::vector(); } +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 @@ int MinPositive(int a, int b) { 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 @@ webrtc::RTCError WebRtcVideoChannel::WebRtcVideoSendStream::SetRtpParameters( // 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 @@ void WebRtcVideoChannel::WebRtcVideoSendStream::UpdateSendState() { } std::vector 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 @@ std::vector EncoderStreamFactory::CreateEncoderStreams( layers[0].min_bitrate_bps = rtc::saturated_cast(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 @@ std::vector EncoderStreamFactory::CreateEncoderStreams( 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 5c24454914..8870cd66b0 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -6566,6 +6566,47 @@ TEST_F(WebRtcVideoChannelTest, 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 @@ TEST_F(WebRtcVideoChannelTest, // 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 f157734192..f091636aed 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 kColorV = 33; 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 @@ void ConfigureStream(int width, int max_bitrate, int min_bitrate, int target_bitrate, - float max_framerate, SimulcastStream* stream, int num_temporal_layers) { assert(stream); @@ -205,7 +203,6 @@ void ConfigureStream(int width, 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 @@ void SimulcastTestFixtureImpl::DefaultSettings( 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 ea5de23a8f..1ede93b679 100644 --- a/modules/video_coding/video_codec_initializer.cc +++ b/modules/video_coding/video_codec_initializer.cc @@ -89,13 +89,17 @@ VideoCodec VideoCodecInitializer::VideoEncoderConfigToVideoCodec( 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 @@ VideoCodec VideoCodecInitializer::VideoEncoderConfigToVideoCodec( video_codec.maxBitrate += streams[i].max_bitrate_bps / 1000; video_codec.qpMax = std::max(video_codec.qpMax, static_cast(streams[i].max_qp)); - max_framerate = std::max(max_framerate, streams[i].max_framerate); } if (video_codec.maxBitrate == 0) { @@ -134,7 +137,8 @@ VideoCodec VideoCodecInitializer::VideoEncoderConfigToVideoCodec( 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 fb6bad4fce..9026cfc201 100644 --- a/pc/rtp_sender_receiver_unittest.cc +++ b/pc/rtp_sender_receiver_unittest.cc @@ -1309,43 +1309,6 @@ TEST_F(RtpSenderReceiverTest, VideoSenderDetectInvalidScaleResolutionDownBy) { 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 e84ad6e9d3..458f1ed728 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -151,8 +151,6 @@ bool RequiresEncoderReset(const VideoCodec& prev_send_codec, 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 @@ void VideoStreamEncoder::ReconfigureEncoder() { << " 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: "