Fix requested_resolution orientation assumption in OnSinkWants().
The VideoAdapter is used to configure encoding resolutions based on requested_resolution in an orientation agnostic way[1]. This means that if you request 1280x720 and the input frame is 720x1280, there is no downscale happening. However in the same file there is one instance of VideoAdapter::OnSinkWants() where requested_resolution is assumed to be expressed in landscape mode. This breaks the case where the 720x1280 is requested but the frame is 1280x720 which causes inconsistent behavior and breaks symmetry. This would also break simulcast since this code path is only applied with the top layer's requested resolution while the lower layers are still scaled in an agnostic way. A new test is added to verify the fix. Prior to the fix, the first half of the test was passing, after the fix both parts of the test pass. [1] https://source.chromium.org/chromium/chromium/src/+/main:third_party/webrtc/media/base/video_adapter.h;l=76;drc=02b5b024b66755a851a752b7851b124ba03f6cb6 Bug: webrtc:363019836 Change-Id: I564068e98c93cab89eb38a10b0f8378899438e5b Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/361160 Commit-Queue: Henrik Boström <hbos@webrtc.org> Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org> Cr-Commit-Position: refs/heads/main@{#42923}
This commit is contained in:
parent
d34f3b8e04
commit
843a3173f2
@ -379,6 +379,11 @@ void VideoAdapter::OnSinkWants(const rtc::VideoSinkWants& sink_wants) {
|
||||
}
|
||||
|
||||
auto res = *sink_wants.requested_resolution;
|
||||
if (res.width < res.height) {
|
||||
// Adjust `res` to landscape mode.
|
||||
res.width = sink_wants.requested_resolution->height;
|
||||
res.height = sink_wants.requested_resolution->width;
|
||||
}
|
||||
auto pixel_count = res.width * res.height;
|
||||
output_format_request_.target_landscape_aspect_ratio =
|
||||
std::make_pair(res.width, res.height);
|
||||
|
||||
@ -1281,6 +1281,31 @@ TEST_P(VideoAdapterTest, UseRequestedResolutionInsteadOfOnOutputFormatRequest) {
|
||||
}
|
||||
}
|
||||
|
||||
TEST_P(VideoAdapterTest, RequestedResolutionIsOrientationAgnostic) {
|
||||
// Request 1280x720 when frame is 720x1280.
|
||||
{
|
||||
adapter_.OnSinkWants(
|
||||
BuildSinkWants(Resolution{.width = 1280, .height = 720},
|
||||
/* any_active_without_requested_resolution= */ false));
|
||||
|
||||
EXPECT_THAT(
|
||||
AdaptFrameResolution(/* input frame */ {.width = 720, .height = 1280})
|
||||
.first,
|
||||
Eq(Resolution{.width = 720, .height = 1280}));
|
||||
}
|
||||
// Request 720x1280 when frame is 1280x720.
|
||||
{
|
||||
adapter_.OnSinkWants(
|
||||
BuildSinkWants(Resolution{.width = 720, .height = 1280},
|
||||
/* any_active_without_requested_resolution= */ false));
|
||||
|
||||
EXPECT_THAT(
|
||||
AdaptFrameResolution(/* input frame */ {.width = 1280, .height = 720})
|
||||
.first,
|
||||
Eq(Resolution{.width = 1280, .height = 720}));
|
||||
}
|
||||
}
|
||||
|
||||
class VideoAdapterWithSourceAlignmentTest : public VideoAdapterTest {
|
||||
protected:
|
||||
static constexpr int kSourceResolutionAlignment = 7;
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user