From 2b5f7cb4b34e5d218a3f12e8b04dd537585198c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Bostr=C3=B6m?= Date: Fri, 13 Sep 2024 12:23:04 +0200 Subject: [PATCH] Adjust `requested_resolution` to match frame's aspect ratio. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This API should not modify the aspect ratio of the frame, e.g. if the frame is 1280x720 and requested_resolution is 1280x360, the result should be 640x360, not a streched out 1280x360 frame. The spec version of this API calls this "maxWidth" and "maxHeight" which is the right way to think about it rather than a forced width and height. VideoAdapter continues to be used to apply adaptation restrictions, but we now make sure to calculate the correct frame size BEFORE applying restrictions. Prior to this CL, the VideoAdapter was also used to apply requested_resolution restrictions. This is actually wrong and would cause strange scaling factors in some cases, e.g. f=1280x720 + r=720x405 would result in 640x360 instead of 720x405. Now we make f=720x405 first and only adjust further if restrictions or alignments require us to. Since this is a change in behavior a WebRtcVideoChannelTest is updated. Encodings integration test is also added, both for aspect ratio (new behavior) and orientation agnosticism (old behavior still passing). Bug: webrtc:366067962 Change-Id: I4e8dc27da5a84d73238b8ab74ef197eb5ee8072a Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/362101 Reviewed-by: Ilya Nikolaevskiy Commit-Queue: Henrik Boström Cr-Commit-Position: refs/heads/main@{#43020} --- media/engine/webrtc_video_engine_unittest.cc | 29 ++++++-- ...er_connection_encodings_integrationtest.cc | 74 +++++++++++++++++++ video/config/encoder_stream_factory.cc | 22 +++++- 3 files changed, 118 insertions(+), 7 deletions(-) diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index a495f4a3b2..f6a86237d3 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -9724,7 +9724,7 @@ TEST_F(WebRtcVideoChannelTest, RequestedResolutionSinglecast) { EXPECT_TRUE(send_channel_->SetVideoSend(last_ssrc_, nullptr, nullptr)); } -TEST_F(WebRtcVideoChannelTest, RequestedResolutionSinglecastCropping) { +TEST_F(WebRtcVideoChannelTest, RequestedResolutionSinglecastScaling) { cricket::VideoSenderParameters parameters; parameters.codecs.push_back(GetEngineCodec("VP8")); ASSERT_TRUE(send_channel_->SetSenderParameters(parameters)); @@ -9747,8 +9747,10 @@ TEST_F(WebRtcVideoChannelTest, RequestedResolutionSinglecastCropping) { auto streams = stream->GetVideoStreams(); ASSERT_EQ(streams.size(), 1u); + // The scaling factor is 720/1280 because of orientation, + // scaling the height (720) by this value gets you 405p. EXPECT_EQ(rtc::checked_cast(720), streams[0].width); - EXPECT_EQ(rtc::checked_cast(720), streams[0].height); + EXPECT_EQ(rtc::checked_cast(405), streams[0].height); } { @@ -9762,7 +9764,8 @@ TEST_F(WebRtcVideoChannelTest, RequestedResolutionSinglecastCropping) { auto streams = stream->GetVideoStreams(); ASSERT_EQ(streams.size(), 1u); - EXPECT_EQ(rtc::checked_cast(720), streams[0].width); + // No downscale needed to fit 1280x1280. + EXPECT_EQ(rtc::checked_cast(1280), streams[0].width); EXPECT_EQ(rtc::checked_cast(720), streams[0].height); } @@ -9775,8 +9778,24 @@ TEST_F(WebRtcVideoChannelTest, RequestedResolutionSinglecastCropping) { auto streams = stream->GetVideoStreams(); ASSERT_EQ(streams.size(), 1u); - EXPECT_EQ(rtc::checked_cast(480), streams[0].width); - EXPECT_EQ(rtc::checked_cast(480), streams[0].height); + // The scaling factor is 650/1280 because of orientation, + // scaling the height (720) by this value gets you 365.625 which is rounded. + EXPECT_EQ(rtc::checked_cast(650), streams[0].width); + EXPECT_EQ(rtc::checked_cast(366), streams[0].height); + } + + { + auto rtp_parameters = send_channel_->GetRtpSendParameters(last_ssrc_); + EXPECT_EQ(1UL, rtp_parameters.encodings.size()); + rtp_parameters.encodings[0].requested_resolution = {.width = 2560, + .height = 1440}; + send_channel_->SetRtpSendParameters(last_ssrc_, rtp_parameters); + + auto streams = stream->GetVideoStreams(); + ASSERT_EQ(streams.size(), 1u); + // We don't upscale. + EXPECT_EQ(rtc::checked_cast(1280), streams[0].width); + EXPECT_EQ(rtc::checked_cast(720), streams[0].height); } EXPECT_TRUE(send_channel_->SetVideoSend(last_ssrc_, nullptr, nullptr)); diff --git a/pc/peer_connection_encodings_integrationtest.cc b/pc/peer_connection_encodings_integrationtest.cc index 552df132a1..ed087894e7 100644 --- a/pc/peer_connection_encodings_integrationtest.cc +++ b/pc/peer_connection_encodings_integrationtest.cc @@ -2513,6 +2513,80 @@ TEST_P(PeerConnectionEncodingsIntegrationParameterizedTest, kLongTimeoutForRampingUp.ms()); } +TEST_P(PeerConnectionEncodingsIntegrationParameterizedTest, + RequestedResolutionIsOrientationAgnostic) { + rtc::scoped_refptr local_pc_wrapper = CreatePc(); + if (SkipTestDueToAv1Missing(local_pc_wrapper)) { + return; + } + rtc::scoped_refptr remote_pc_wrapper = CreatePc(); + ExchangeIceCandidates(local_pc_wrapper, remote_pc_wrapper); + + std::vector layers = + CreateLayers({"f"}, /*active=*/true); + + // This transceiver receives a 1280x720 source. + rtc::scoped_refptr transceiver = + AddTransceiverWithSimulcastLayers(local_pc_wrapper, remote_pc_wrapper, + layers); + std::vector codecs = + GetCapabilitiesAndRestrictToCodec(remote_pc_wrapper, codec_name_); + transceiver->SetCodecPreferences(codecs); + + NegotiateWithSimulcastTweaks(local_pc_wrapper, remote_pc_wrapper); + local_pc_wrapper->WaitForConnection(); + remote_pc_wrapper->WaitForConnection(); + + // 360x640 is the same as 640x360 due to orientation agnosticism. + // The orientation is determined by the frame (1280x720): landscape. + rtc::scoped_refptr sender = transceiver->sender(); + RtpParameters parameters = sender->GetParameters(); + ASSERT_THAT(parameters.encodings, SizeIs(1)); + parameters.encodings[0].requested_resolution = {.width = 360, .height = 640}; + sender->SetParameters(parameters); + // Confirm 640x360 is sent. + ASSERT_TRUE_WAIT(GetEncodingResolution(local_pc_wrapper) == + (Resolution{.width = 640, .height = 360}), + kLongTimeoutForRampingUp.ms()); +} + +TEST_P(PeerConnectionEncodingsIntegrationParameterizedTest, + RequestedResolutionMaintainsAspectRatio) { + rtc::scoped_refptr local_pc_wrapper = CreatePc(); + if (SkipTestDueToAv1Missing(local_pc_wrapper)) { + return; + } + rtc::scoped_refptr remote_pc_wrapper = CreatePc(); + ExchangeIceCandidates(local_pc_wrapper, remote_pc_wrapper); + + std::vector layers = + CreateLayers({"f"}, /*active=*/true); + + // This transceiver receives a 1280x720 source. + rtc::scoped_refptr transceiver = + AddTransceiverWithSimulcastLayers(local_pc_wrapper, remote_pc_wrapper, + layers); + std::vector codecs = + GetCapabilitiesAndRestrictToCodec(remote_pc_wrapper, codec_name_); + transceiver->SetCodecPreferences(codecs); + + NegotiateWithSimulcastTweaks(local_pc_wrapper, remote_pc_wrapper); + local_pc_wrapper->WaitForConnection(); + remote_pc_wrapper->WaitForConnection(); + + // Restrict height more than width, the scaling factor needed on height should + // also be applied on the width in order to maintain the frame aspect ratio. + rtc::scoped_refptr sender = transceiver->sender(); + RtpParameters parameters = sender->GetParameters(); + ASSERT_THAT(parameters.encodings, SizeIs(1)); + parameters.encodings[0].requested_resolution = {.width = 1280, .height = 360}; + sender->SetParameters(parameters); + // Confirm 640x360 is sent. + ASSERT_TRUE_WAIT(GetEncodingResolution(local_pc_wrapper) == + (Resolution{.width = 640, .height = 360}), + kLongTimeoutForRampingUp.ms()); +} + INSTANTIATE_TEST_SUITE_P(StandardPath, PeerConnectionEncodingsIntegrationParameterizedTest, ::testing::Values("VP8", diff --git a/video/config/encoder_stream_factory.cc b/video/config/encoder_stream_factory.cc index fc5254d88c..07e9a38cea 100644 --- a/video/config/encoder_stream_factory.cc +++ b/video/config/encoder_stream_factory.cc @@ -472,9 +472,27 @@ EncoderStreamFactory::GetLayerResolutionFromRequestedResolution( int frame_width, int frame_height, webrtc::Resolution requested_resolution) const { + // Make frame and requested resolution have matching orientation. + if ((frame_width < frame_height) != + (requested_resolution.width < requested_resolution.height)) { + requested_resolution = {.width = requested_resolution.height, + .height = requested_resolution.width}; + } + // Downscale by smallest scaling factor, if necessary. + if (frame_width > 0 && frame_height > 0 && + (requested_resolution.width < frame_width || + requested_resolution.height < frame_height)) { + double scale_factor = std::min( + requested_resolution.width / static_cast(frame_width), + requested_resolution.height / static_cast(frame_height)); + frame_width = std::round(frame_width * scale_factor); + frame_height = std::round(frame_height * scale_factor); + } + webrtc::Resolution frame = {.width = frame_width, .height = frame_height}; + + // Maybe adapt further based on restrictions and encoder alignment. VideoAdapter adapter(encoder_info_requested_resolution_alignment_); - adapter.OnOutputFormatRequest(requested_resolution.ToPair(), - requested_resolution.PixelCount(), + adapter.OnOutputFormatRequest(frame.ToPair(), frame.PixelCount(), std::nullopt); if (restrictions_) { rtc::VideoSinkWants wants;