From 0deda15c963a72088b7b1271d89e4307fe50dcb7 Mon Sep 17 00:00:00 2001 From: Jonas Oreland Date: Fri, 23 Sep 2022 12:08:57 +0200 Subject: [PATCH] Reland "RtpEncodingParameters::request_resolution patch 1" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit b625101da8d798c936cfd695505a5514644158b0. Reason for revert: Found problem that was specific how configuration is handled for VP9. A 1-line change in webrtc_video_engine.cc line 3715. Thanks Rasmus and great that this was tested! Original change's description: > Revert "RtpEncodingParameters::request_resolution patch 1" > > This reverts commit ef7359e679e579ccb79afacf5c42e8c6020124e2. > > Reason for revert: Breaks downstream test > > Original change's description: > > RtpEncodingParameters::request_resolution patch 1 > > > > This patch adds RtpEncodingParameters::request_resolution > > with documentation and plumming. No behaviour is changed yet. > > > > Bug: webrtc:14451 > > Change-Id: I1f4f83a312ee8c293e3d8f02b950751e62048304 > > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/276262 > > Reviewed-by: Ilya Nikolaevskiy > > Reviewed-by: Henrik Boström > > Reviewed-by: Rasmus Brandt > > Reviewed-by: Tomas Gunnarsson > > Commit-Queue: Jonas Oreland > > Cr-Commit-Position: refs/heads/main@{#38172} > > Bug: webrtc:14451 > Change-Id: I4b9590e23ec38e9e1c2e51a4600ef96b129439f2 > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/276541 > Commit-Queue: Björn Terelius > Bot-Commit: rubber-stamper@appspot.gserviceaccount.com > Reviewed-by: Jonas Oreland > Owners-Override: Björn Terelius > Cr-Commit-Position: refs/heads/main@{#38176} Bug: webrtc:14451 Change-Id: Ica9b74180bce22d09bf289126bb5ac137bf9eb70 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/276543 Reviewed-by: Henrik Boström Reviewed-by: Tomas Gunnarsson Commit-Queue: Jonas Oreland Cr-Commit-Position: refs/heads/main@{#38178} --- api/BUILD.gn | 1 + api/rtp_parameters.h | 22 +++++- api/video/BUILD.gn | 5 ++ api/video/recordable_encoded_frame.h | 1 + api/video/render_resolution.h | 1 + api/video/resolution.h | 32 +++++++++ api/video/video_source_interface.h | 23 +++++++ api/video_codecs/BUILD.gn | 1 + api/video_codecs/video_encoder_config.h | 19 +++++- media/base/media_engine.cc | 7 ++ media/base/video_broadcaster.cc | 21 ++++++ media/base/video_broadcaster_unittest.cc | 68 +++++++++++++++++++ media/engine/webrtc_video_engine.cc | 11 ++- video/video_receive_stream2_unittest.cc | 9 +-- video/video_source_sink_controller.cc | 49 +++++++------ video/video_source_sink_controller.h | 8 +++ .../video_source_sink_controller_unittest.cc | 34 ++++++++++ video/video_stream_encoder.cc | 34 +++++++++- 18 files changed, 309 insertions(+), 37 deletions(-) create mode 100644 api/video/resolution.h diff --git a/api/BUILD.gn b/api/BUILD.gn index 9445e3cee3..5bf73961fb 100644 --- a/api/BUILD.gn +++ b/api/BUILD.gn @@ -454,6 +454,7 @@ rtc_library("rtp_parameters") { "../rtc_base:checks", "../rtc_base:stringutils", "../rtc_base/system:rtc_export", + "video:resolution", "video_codecs:scalability_mode", ] absl_deps = [ diff --git a/api/rtp_parameters.h b/api/rtp_parameters.h index e311577878..0d3c9dfd22 100644 --- a/api/rtp_parameters.h +++ b/api/rtp_parameters.h @@ -23,6 +23,7 @@ #include "api/media_types.h" #include "api/priority.h" #include "api/rtp_transceiver_direction.h" +#include "api/video/resolution.h" #include "api/video_codecs/scalability_mode.h" #include "rtc_base/system/rtc_export.h" @@ -502,6 +503,24 @@ struct RTC_EXPORT RtpEncodingParameters { // https://w3c.github.io/webrtc-svc/#rtcrtpencodingparameters absl::optional scalability_mode; + // Requested encode resolution. + // + // This field provides an alternative to `scale_resolution_down_by` + // that is not dependent on the video source. + // + // When setting requested_resolution it is not necessary to adapt the + // video source using OnOutputFormatRequest, since the VideoStreamEncoder + // will apply downscaling if necessary. requested_resolution will also be + // propagated to the video source, this allows downscaling earlier in the + // pipeline which can be beneficial if the source is consumed by multiple + // encoders, but is not strictly necessary. + // + // The `requested_resolution` is subject to resource adaptation. + // + // It is an error to set both `requested_resolution` and + // `scale_resolution_down_by`. + absl::optional requested_resolution; + // For an RtpSender, set to true to cause this encoding to be encoded and // sent, and false for it not to be encoded and sent. This allows control // across multiple encodings of a sender for turning simulcast layers on and @@ -527,7 +546,8 @@ struct RTC_EXPORT RtpEncodingParameters { num_temporal_layers == o.num_temporal_layers && scale_resolution_down_by == o.scale_resolution_down_by && active == o.active && rid == o.rid && - adaptive_ptime == o.adaptive_ptime; + adaptive_ptime == o.adaptive_ptime && + requested_resolution == o.requested_resolution; } bool operator!=(const RtpEncodingParameters& o) const { return !(*this == o); diff --git a/api/video/BUILD.gn b/api/video/BUILD.gn index 060b9e48b8..ee62abdcc5 100644 --- a/api/video/BUILD.gn +++ b/api/video/BUILD.gn @@ -131,6 +131,11 @@ rtc_source_set("render_resolution") { public = [ "render_resolution.h" ] } +rtc_source_set("resolution") { + visibility = [ "*" ] + public = [ "resolution.h" ] +} + rtc_library("encoded_image") { visibility = [ "*" ] sources = [ diff --git a/api/video/recordable_encoded_frame.h b/api/video/recordable_encoded_frame.h index 702f4d73e6..47ea23f119 100644 --- a/api/video/recordable_encoded_frame.h +++ b/api/video/recordable_encoded_frame.h @@ -24,6 +24,7 @@ namespace webrtc { class RecordableEncodedFrame { public: // Encoded resolution in pixels + // TODO(bugs.webrtc.org/12114) : remove in favor of Resolution. struct EncodedResolution { bool empty() const { return width == 0 && height == 0; } diff --git a/api/video/render_resolution.h b/api/video/render_resolution.h index edcf8f8ee5..fcf4f122d6 100644 --- a/api/video/render_resolution.h +++ b/api/video/render_resolution.h @@ -13,6 +13,7 @@ namespace webrtc { +// TODO(bugs.webrtc.org/12114) : remove in favor of Resolution. class RenderResolution { public: constexpr RenderResolution() = default; diff --git a/api/video/resolution.h b/api/video/resolution.h new file mode 100644 index 0000000000..99cb62299b --- /dev/null +++ b/api/video/resolution.h @@ -0,0 +1,32 @@ +/* + * Copyright (c) 2022 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#ifndef API_VIDEO_RESOLUTION_H_ +#define API_VIDEO_RESOLUTION_H_ + +namespace webrtc { + +// A struct representing a video resolution in pixels. +struct Resolution { + int width = 0; + int height = 0; +}; + +inline bool operator==(const Resolution& lhs, const Resolution& rhs) { + return lhs.width == rhs.width && lhs.height == rhs.height; +} + +inline bool operator!=(const Resolution& lhs, const Resolution& rhs) { + return !(lhs == rhs); +} + +} // namespace webrtc + +#endif // API_VIDEO_RESOLUTION_H_ diff --git a/api/video/video_source_interface.h b/api/video/video_source_interface.h index 5eb4ebfd75..72937c7c80 100644 --- a/api/video/video_source_interface.h +++ b/api/video/video_source_interface.h @@ -80,6 +80,24 @@ struct RTC_EXPORT VideoSinkWants { // Note that the `resolutions` can change while frames are in flight and // should only be used as a hint when constructing the webrtc::VideoFrame. std::vector resolutions; + + // This is the resolution requested by the user using RtpEncodingParameters. + absl::optional requested_resolution; + + // `active` : is (any) of the layers/sink(s) active. + bool is_active = false; + + // This sub-struct contains information computed by VideoBroadcaster + // that aggregates several VideoSinkWants (and sends them to + // AdaptedVideoTrackSource). + struct Aggregates { + // `active_without_requested_resolution` is set by VideoBroadcaster + // when aggregating sink wants if there exists any sink (encoder) that is + // active but has not set the `requested_resolution`, i.e is relying on + // OnOutputFormatRequest to handle encode resolution. + bool any_active_without_requested_resolution = false; + }; + absl::optional aggregates; }; inline bool operator==(const VideoSinkWants::FrameSize& a, @@ -87,6 +105,11 @@ inline bool operator==(const VideoSinkWants::FrameSize& a, return a.width == b.width && a.height == b.height; } +inline bool operator!=(const VideoSinkWants::FrameSize& a, + const VideoSinkWants::FrameSize& b) { + return !(a == b); +} + template class VideoSourceInterface { public: diff --git a/api/video_codecs/BUILD.gn b/api/video_codecs/BUILD.gn index 3f933b9c74..d6b7392b7f 100644 --- a/api/video_codecs/BUILD.gn +++ b/api/video_codecs/BUILD.gn @@ -84,6 +84,7 @@ rtc_library("video_codecs_api") { "../units:data_rate", "../video:encoded_image", "../video:render_resolution", + "../video:resolution", "../video:video_bitrate_allocation", "../video:video_codec_constants", "../video:video_frame", diff --git a/api/video_codecs/video_encoder_config.h b/api/video_codecs/video_encoder_config.h index 86d89d53da..3d1b176738 100644 --- a/api/video_codecs/video_encoder_config.h +++ b/api/video_codecs/video_encoder_config.h @@ -18,6 +18,7 @@ #include "absl/types/optional.h" #include "api/scoped_refptr.h" +#include "api/video/resolution.h" #include "api/video_codecs/scalability_mode.h" #include "api/video_codecs/sdp_video_format.h" #include "api/video_codecs/video_codec.h" @@ -32,10 +33,11 @@ struct VideoStream { VideoStream(const VideoStream& other); std::string ToString() const; - // Width in pixels. + // Width/Height in pixels. + // This is the actual width and height used to configure encoder, + // which might be less than `requested_resolution` due to adaptation + // or due to the source providing smaller frames than requested. size_t width; - - // Height in pixels. size_t height; // Frame rate in fps. @@ -69,6 +71,17 @@ struct VideoStream { // If this stream is enabled by the user, or not. bool active; + + // An optional user supplied max_frame_resolution + // than can be set independently of (adapted) VideoSource. + // This value is set from RtpEncodingParameters::requested_resolution + // (i.e. used for signaling app-level settings). + // + // The actual encode resolution is in `width` and `height`, + // which can be lower than requested_resolution, + // e.g. if source only provides lower resolution or + // if resource adaptation is active. + absl::optional requested_resolution; }; class VideoEncoderConfig { diff --git a/media/base/media_engine.cc b/media/base/media_engine.cc index 813657eb00..694e690056 100644 --- a/media/base/media_engine.cc +++ b/media/base/media_engine.cc @@ -106,6 +106,13 @@ webrtc::RTCError CheckRtpParametersValues( "num_temporal_layers to an invalid number."); } } + + if (rtp_parameters.encodings[i].requested_resolution && + rtp_parameters.encodings[i].scale_resolution_down_by) { + LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_RANGE, + "Attempted to set scale_resolution_down_by and " + "requested_resolution simultaniously."); + } } return webrtc::RTCError::OK(); diff --git a/media/base/video_broadcaster.cc b/media/base/video_broadcaster.cc index 1167d7fb32..60c0e1ce06 100644 --- a/media/base/video_broadcaster.cc +++ b/media/base/video_broadcaster.cc @@ -10,6 +10,7 @@ #include "media/base/video_broadcaster.h" +#include #include #include "absl/types/optional.h" @@ -123,6 +124,7 @@ void VideoBroadcaster::UpdateWants() { VideoSinkWants wants; wants.rotation_applied = false; wants.resolution_alignment = 1; + wants.aggregates.emplace(VideoSinkWants::Aggregates()); for (auto& sink : sink_pairs()) { // wants.rotation_applied == ANY(sink.wants.rotation_applied) if (sink.wants.rotation_applied) { @@ -147,6 +149,25 @@ void VideoBroadcaster::UpdateWants() { } wants.resolution_alignment = cricket::LeastCommonMultiple( wants.resolution_alignment, sink.wants.resolution_alignment); + + // Pick MAX(requested_resolution) since the actual can be downscaled + // in encoder instead. + if (sink.wants.requested_resolution) { + if (!wants.requested_resolution) { + wants.requested_resolution = sink.wants.requested_resolution; + } else { + wants.requested_resolution->width = + std::max(wants.requested_resolution->width, + sink.wants.requested_resolution->width); + wants.requested_resolution->height = + std::max(wants.requested_resolution->height, + sink.wants.requested_resolution->height); + } + } else if (sink.wants.is_active) { + wants.aggregates->any_active_without_requested_resolution = true; + } + + wants.is_active |= sink.wants.is_active; } if (wants.target_pixel_count && diff --git a/media/base/video_broadcaster_unittest.cc b/media/base/video_broadcaster_unittest.cc index fe1a1cb725..52f015cf6e 100644 --- a/media/base/video_broadcaster_unittest.cc +++ b/media/base/video_broadcaster_unittest.cc @@ -24,6 +24,7 @@ using cricket::FakeVideoRenderer; using rtc::VideoBroadcaster; using rtc::VideoSinkWants; +using FrameSize = rtc::VideoSinkWants::FrameSize; using ::testing::AllOf; using ::testing::Eq; @@ -330,3 +331,70 @@ TEST(VideoBroadcasterTest, ForwardsConstraintsToSink) { Field(&webrtc::VideoTrackSourceConstraints::max_fps, Optional(3))))); broadcaster.ProcessConstraints(webrtc::VideoTrackSourceConstraints{2, 3}); } + +TEST(VideoBroadcasterTest, AppliesMaxOfSinkWantsRequestedResolution) { + VideoBroadcaster broadcaster; + + FakeVideoRenderer sink1; + VideoSinkWants wants1; + wants1.requested_resolution = FrameSize(640, 360); + + broadcaster.AddOrUpdateSink(&sink1, wants1); + EXPECT_EQ(FrameSize(640, 360), *broadcaster.wants().requested_resolution); + + FakeVideoRenderer sink2; + VideoSinkWants wants2; + wants2.requested_resolution = FrameSize(650, 350); + broadcaster.AddOrUpdateSink(&sink2, wants2); + EXPECT_EQ(FrameSize(650, 360), *broadcaster.wants().requested_resolution); + + broadcaster.RemoveSink(&sink2); + EXPECT_EQ(FrameSize(640, 360), *broadcaster.wants().requested_resolution); +} + +TEST(VideoBroadcasterTest, AnyActive) { + VideoBroadcaster broadcaster; + + FakeVideoRenderer sink1; + VideoSinkWants wants1; + wants1.is_active = false; + + broadcaster.AddOrUpdateSink(&sink1, wants1); + EXPECT_EQ(false, broadcaster.wants().is_active); + + FakeVideoRenderer sink2; + VideoSinkWants wants2; + wants2.is_active = true; + broadcaster.AddOrUpdateSink(&sink2, wants2); + EXPECT_EQ(true, broadcaster.wants().is_active); + + broadcaster.RemoveSink(&sink2); + EXPECT_EQ(false, broadcaster.wants().is_active); +} + +TEST(VideoBroadcasterTest, AnyActiveWithoutRequestedResolution) { + VideoBroadcaster broadcaster; + + FakeVideoRenderer sink1; + VideoSinkWants wants1; + wants1.is_active = true; + wants1.requested_resolution = FrameSize(640, 360); + + broadcaster.AddOrUpdateSink(&sink1, wants1); + EXPECT_EQ( + false, + broadcaster.wants().aggregates->any_active_without_requested_resolution); + + FakeVideoRenderer sink2; + VideoSinkWants wants2; + wants2.is_active = true; + broadcaster.AddOrUpdateSink(&sink2, wants2); + EXPECT_EQ( + true, + broadcaster.wants().aggregates->any_active_without_requested_resolution); + + broadcaster.RemoveSink(&sink2); + EXPECT_EQ( + false, + broadcaster.wants().aggregates->any_active_without_requested_resolution); +} diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index 5651f5944c..feaa78044a 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -2350,7 +2350,9 @@ webrtc::RTCError WebRtcVideoChannel::WebRtcVideoSendStream::SetRtpParameters( (new_parameters.encodings[i].scale_resolution_down_by != rtp_parameters_.encodings[i].scale_resolution_down_by) || (new_parameters.encodings[i].num_temporal_layers != - rtp_parameters_.encodings[i].num_temporal_layers)) { + rtp_parameters_.encodings[i].num_temporal_layers) || + (new_parameters.encodings[i].requested_resolution != + rtp_parameters_.encodings[i].requested_resolution)) { new_param = true; break; } @@ -2562,6 +2564,8 @@ WebRtcVideoChannel::WebRtcVideoSendStream::CreateVideoEncoderConfig( encoder_config.simulcast_layers[i].num_temporal_layers = *rtp_parameters_.encodings[i].num_temporal_layers; } + encoder_config.simulcast_layers[i].requested_resolution = + rtp_parameters_.encodings[i].requested_resolution; } encoder_config.legacy_conference_mode = parameters_.conference_mode; @@ -3705,6 +3709,11 @@ EncoderStreamFactory::CreateDefaultVideoStreams( layer.width = width; layer.height = height; layer.max_framerate = max_framerate; + // Note: VP9 seems to have be sending if any layer is active, + // (see `UpdateSendState`) and still use parameters only from + // encoder_config.simulcast_layers[0]. + layer.active = absl::c_any_of(encoder_config.simulcast_layers, + [](const auto& layer) { return layer.active; }); if (encoder_config.simulcast_layers[0].scale_resolution_down_by > 1.) { layer.width = ScaleDownResolution( diff --git a/video/video_receive_stream2_unittest.cc b/video/video_receive_stream2_unittest.cc index 79c680f1d1..a05524f9e8 100644 --- a/video/video_receive_stream2_unittest.cc +++ b/video/video_receive_stream2_unittest.cc @@ -158,7 +158,7 @@ class FakeVideoRenderer : public rtc::VideoSinkInterface { TimeController* const time_controller_; }; -MATCHER_P2(Resolution, w, h, "") { +MATCHER_P2(MatchResolution, w, h, "") { return arg.resolution().width == w && arg.resolution().height == h; } @@ -740,8 +740,9 @@ TEST_P(VideoReceiveStream2Test, /*generate_key_frame=*/false); InSequence s; - EXPECT_CALL(callback, Call(Resolution(test::FakeDecoder::kDefaultWidth, - test::FakeDecoder::kDefaultHeight))); + EXPECT_CALL(callback, + Call(MatchResolution(test::FakeDecoder::kDefaultWidth, + test::FakeDecoder::kDefaultHeight))); EXPECT_CALL(callback, Call); video_receive_stream_->OnCompleteFrame( @@ -763,7 +764,7 @@ TEST_P(VideoReceiveStream2Test, /*generate_key_frame=*/false); InSequence s; - EXPECT_CALL(callback, Call(Resolution(1080u, 720u))); + EXPECT_CALL(callback, Call(MatchResolution(1080u, 720u))); EXPECT_CALL(callback, Call); video_receive_stream_->OnCompleteFrame( diff --git a/video/video_source_sink_controller.cc b/video/video_source_sink_controller.cc index cf3b649f70..2f7b37585d 100644 --- a/video/video_source_sink_controller.cc +++ b/video/video_source_sink_controller.cc @@ -20,29 +20,6 @@ namespace webrtc { -namespace { - -std::string WantsToString(const rtc::VideoSinkWants& wants) { - rtc::StringBuilder ss; - - ss << "max_fps=" << wants.max_framerate_fps - << " max_pixel_count=" << wants.max_pixel_count << " target_pixel_count=" - << (wants.target_pixel_count.has_value() - ? std::to_string(wants.target_pixel_count.value()) - : "null") - << " resolutions={"; - for (size_t i = 0; i < wants.resolutions.size(); ++i) { - if (i != 0) - ss << ","; - ss << wants.resolutions[i].width << "x" << wants.resolutions[i].height; - } - ss << "}"; - - return ss.Release(); -} - -} // namespace - VideoSourceSinkController::VideoSourceSinkController( rtc::VideoSinkInterface* sink, rtc::VideoSourceInterface* source) @@ -86,8 +63,6 @@ void VideoSourceSinkController::PushSourceSinkSettings() { if (!source_) return; rtc::VideoSinkWants wants = CurrentSettingsToSinkWants(); - RTC_LOG(LS_INFO) << "Pushing SourceSink restrictions: " - << WantsToString(wants); source_->AddOrUpdateSink(sink_, wants); } @@ -124,6 +99,17 @@ VideoSourceSinkController::resolutions() const { return resolutions_; } +bool VideoSourceSinkController::active() const { + RTC_DCHECK_RUN_ON(&sequence_checker_); + return active_; +} + +absl::optional +VideoSourceSinkController::requested_resolution() const { + RTC_DCHECK_RUN_ON(&sequence_checker_); + return requested_resolution_; +} + void VideoSourceSinkController::SetRestrictions( VideoSourceRestrictions restrictions) { RTC_DCHECK_RUN_ON(&sequence_checker_); @@ -159,6 +145,17 @@ void VideoSourceSinkController::SetResolutions( resolutions_ = std::move(resolutions); } +void VideoSourceSinkController::SetActive(bool active) { + RTC_DCHECK_RUN_ON(&sequence_checker_); + active_ = active; +} + +void VideoSourceSinkController::SetRequestedResolution( + absl::optional requested_resolution) { + RTC_DCHECK_RUN_ON(&sequence_checker_); + requested_resolution_ = std::move(requested_resolution); +} + // RTC_EXCLUSIVE_LOCKS_REQUIRED(sequence_checker_) rtc::VideoSinkWants VideoSourceSinkController::CurrentSettingsToSinkWants() const { @@ -188,6 +185,8 @@ rtc::VideoSinkWants VideoSourceSinkController::CurrentSettingsToSinkWants() ? static_cast(frame_rate_upper_limit_.value()) : std::numeric_limits::max()); wants.resolutions = resolutions_; + wants.is_active = active_; + wants.requested_resolution = requested_resolution_; return wants; } diff --git a/video/video_source_sink_controller.h b/video/video_source_sink_controller.h index e2a7eb7c78..1bb6ef61bf 100644 --- a/video/video_source_sink_controller.h +++ b/video/video_source_sink_controller.h @@ -51,6 +51,8 @@ class VideoSourceSinkController { bool rotation_applied() const; int resolution_alignment() const; const std::vector& resolutions() const; + bool active() const; + absl::optional requested_resolution() const; // Updates the settings stored internally. In order for these settings to be // applied to the sink, PushSourceSinkSettings() must subsequently be called. @@ -61,6 +63,9 @@ class VideoSourceSinkController { void SetRotationApplied(bool rotation_applied); void SetResolutionAlignment(int resolution_alignment); void SetResolutions(std::vector resolutions); + void SetActive(bool active); + void SetRequestedResolution( + absl::optional requested_resolution); private: rtc::VideoSinkWants CurrentSettingsToSinkWants() const @@ -87,6 +92,9 @@ class VideoSourceSinkController { int resolution_alignment_ RTC_GUARDED_BY(&sequence_checker_) = 1; std::vector resolutions_ RTC_GUARDED_BY(&sequence_checker_); + bool active_ RTC_GUARDED_BY(&sequence_checker_) = true; + absl::optional requested_resolution_ + RTC_GUARDED_BY(&sequence_checker_); }; } // namespace webrtc diff --git a/video/video_source_sink_controller_unittest.cc b/video/video_source_sink_controller_unittest.cc index 2db7b5663f..75cc52bdaf 100644 --- a/video/video_source_sink_controller_unittest.cc +++ b/video/video_source_sink_controller_unittest.cc @@ -24,6 +24,7 @@ namespace webrtc { namespace { +using FrameSize = rtc::VideoSinkWants::FrameSize; constexpr int kIntUnconstrained = std::numeric_limits::max(); class MockVideoSinkWithVideoFrame : public rtc::VideoSinkInterface { @@ -61,6 +62,7 @@ TEST(VideoSourceSinkControllerTest, UnconstrainedByDefault) { EXPECT_FALSE(controller.pixels_per_frame_upper_limit().has_value()); EXPECT_FALSE(controller.frame_rate_upper_limit().has_value()); EXPECT_FALSE(controller.rotation_applied()); + EXPECT_FALSE(controller.requested_resolution().has_value()); EXPECT_EQ(controller.resolution_alignment(), 1); EXPECT_CALL(source, AddOrUpdateSink(_, _)) @@ -71,6 +73,7 @@ TEST(VideoSourceSinkControllerTest, UnconstrainedByDefault) { EXPECT_EQ(wants.target_pixel_count, absl::nullopt); EXPECT_EQ(wants.max_framerate_fps, kIntUnconstrained); EXPECT_EQ(wants.resolution_alignment, 1); + EXPECT_FALSE(wants.requested_resolution.has_value()); }); controller.PushSourceSinkSettings(); } @@ -162,4 +165,35 @@ TEST(VideoSourceSinkControllerTest, VideoSourceSinkController controller(&sink, nullptr); controller.RequestRefreshFrame(); } + +TEST(VideoSourceSinkControllerTest, RequestedResolutionPropagatesToWants) { + MockVideoSinkWithVideoFrame sink; + MockVideoSourceWithVideoFrame source; + VideoSourceSinkController controller(&sink, &source); + controller.SetRequestedResolution(FrameSize(640, 360)); + EXPECT_TRUE(controller.requested_resolution().has_value()); + + EXPECT_CALL(source, AddOrUpdateSink(_, _)) + .WillOnce([](rtc::VideoSinkInterface* sink, + const rtc::VideoSinkWants& wants) { + EXPECT_EQ(*wants.requested_resolution, FrameSize(640, 360)); + }); + controller.PushSourceSinkSettings(); +} + +TEST(VideoSourceSinkControllerTest, ActivePropagatesToWants) { + MockVideoSinkWithVideoFrame sink; + MockVideoSourceWithVideoFrame source; + VideoSourceSinkController controller(&sink, &source); + controller.SetActive(true); + EXPECT_TRUE(controller.active()); + + EXPECT_CALL(source, AddOrUpdateSink(_, _)) + .WillOnce([](rtc::VideoSinkInterface* sink, + const rtc::VideoSinkWants& wants) { + EXPECT_TRUE(wants.is_active); + }); + controller.PushSourceSinkSettings(); +} + } // namespace webrtc diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index e320e437cd..1018df4ea8 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -1130,10 +1130,30 @@ void VideoStreamEncoder::ReconfigureEncoder() { RTC_DCHECK_LE(codec.startBitrate, 1000000); max_framerate_ = codec.maxFramerate; - // Inform source about max configured framerate. + // Inform source about max configured framerate, + // requested_resolution and which layers are active. int max_framerate = 0; + // Is any layer active. + bool active = false; + // The max requested_resolution. + absl::optional requested_resolution; for (const auto& stream : streams) { max_framerate = std::max(stream.max_framerate, max_framerate); + active |= stream.active; + // Note: we propagate the highest requested_resolution regardless + // if layer is active or not. + if (stream.requested_resolution) { + if (!requested_resolution) { + requested_resolution.emplace(stream.requested_resolution->width, + stream.requested_resolution->height); + } else { + requested_resolution.emplace( + std::max(stream.requested_resolution->width, + requested_resolution->width), + std::max(stream.requested_resolution->height, + requested_resolution->height)); + } + } } // The resolutions that we're actually encoding with. @@ -1146,20 +1166,28 @@ void VideoStreamEncoder::ReconfigureEncoder() { encoder_resolutions.emplace_back(simulcastStream.width, simulcastStream.height); } + worker_queue_->PostTask(SafeTask( task_safety_.flag(), [this, max_framerate, alignment, - encoder_resolutions = std::move(encoder_resolutions)]() { + encoder_resolutions = std::move(encoder_resolutions), + requested_resolution = std::move(requested_resolution), active]() { RTC_DCHECK_RUN_ON(worker_queue_); if (max_framerate != video_source_sink_controller_.frame_rate_upper_limit() || alignment != video_source_sink_controller_.resolution_alignment() || encoder_resolutions != - video_source_sink_controller_.resolutions()) { + video_source_sink_controller_.resolutions() || + (video_source_sink_controller_.requested_resolution() != + requested_resolution) || + (video_source_sink_controller_.active() != active)) { video_source_sink_controller_.SetFrameRateUpperLimit(max_framerate); video_source_sink_controller_.SetResolutionAlignment(alignment); video_source_sink_controller_.SetResolutions( std::move(encoder_resolutions)); + video_source_sink_controller_.SetRequestedResolution( + requested_resolution); + video_source_sink_controller_.SetActive(active); video_source_sink_controller_.PushSourceSinkSettings(); } }));