From 1d3452f31ba2970791bc3fe22a44eeaa6019e70b Mon Sep 17 00:00:00 2001 From: Jonas Oreland Date: Fri, 12 May 2023 16:36:41 +0200 Subject: [PATCH] RequestedResolution - Bug fix Change default value of is_active to false, this means that VideoRenderer or other VideoSinks added with default rtc::VideoSinkWants() does not block usage of RequestedResolution, e.g JNI_VideoTrack_AddSink. This problem occurs when attaching a VideoRenderer directly to the sending VideoTrack (which is a great solution!). But the VideoRenderer is "passive" and should not block adaptations from RequestedResolution. Bug: webrtc:14451 Change-Id: I2ab02596245c7b82bf94fe86f8788f458c7ea286 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/305024 Reviewed-by: Ilya Nikolaevskiy Commit-Queue: Jonas Oreland Reviewed-by: Rasmus Brandt Cr-Commit-Position: refs/heads/main@{#40105} --- api/video/video_source_interface.h | 9 +++++++-- media/base/video_broadcaster_unittest.cc | 2 ++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/api/video/video_source_interface.h b/api/video/video_source_interface.h index 38d0041718..c636c2ff95 100644 --- a/api/video/video_source_interface.h +++ b/api/video/video_source_interface.h @@ -35,6 +35,7 @@ struct RTC_EXPORT VideoSinkWants { VideoSinkWants(); VideoSinkWants(const VideoSinkWants&); ~VideoSinkWants(); + // Tells the source whether the sink wants frames with rotation applied. // By default, any rotation must be applied by the sink. bool rotation_applied = false; @@ -84,8 +85,12 @@ struct RTC_EXPORT VideoSinkWants { // 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 = true; + // `is_active` : Is this VideoSinkWants from an encoder that is encoding any + // layer. IF YES, it will affect how the VideoAdapter will choose to + // prioritize the OnOutputFormatRequest vs. requested_resolution. IF NO, + // VideoAdapter consider this VideoSinkWants as a passive listener (e.g a + // VideoRenderer or a VideoEncoder that is not currently actively encoding). + bool is_active = false; // This sub-struct contains information computed by VideoBroadcaster // that aggregates several VideoSinkWants (and sends them to diff --git a/media/base/video_broadcaster_unittest.cc b/media/base/video_broadcaster_unittest.cc index c5dc24353c..bb80c11930 100644 --- a/media/base/video_broadcaster_unittest.cc +++ b/media/base/video_broadcaster_unittest.cc @@ -337,6 +337,7 @@ TEST(VideoBroadcasterTest, AppliesMaxOfSinkWantsRequestedResolution) { FakeVideoRenderer sink1; VideoSinkWants wants1; + wants1.is_active = true; wants1.requested_resolution = FrameSize(640, 360); broadcaster.AddOrUpdateSink(&sink1, wants1); @@ -344,6 +345,7 @@ TEST(VideoBroadcasterTest, AppliesMaxOfSinkWantsRequestedResolution) { FakeVideoRenderer sink2; VideoSinkWants wants2; + wants2.is_active = true; wants2.requested_resolution = FrameSize(650, 350); broadcaster.AddOrUpdateSink(&sink2, wants2); EXPECT_EQ(FrameSize(650, 360), *broadcaster.wants().requested_resolution);