VideoAdapter: Interpret requested resolution as max restriction.
The `requested_resolution` API must not change aspect ratio, example: - Frame is 60x30 - Requested is 30x30 - We expect 30x15 (not 30x30!) as to maintain aspect ratio. This bug was previously fixed by making VideoAdapter unaware of the requested resolution behind a flag: this seemed OK since the VideoStreamEncoder ultimately decides the resolution, whether or not the incoming frame is adapted. But this is not desired for some non-Chrome use cases. This CL attempts to make both Chrome and non-Chrome use cases happy by implementing the aspect ratio preserving restriction inside VideoAdapter too. This allows us to get rid of the "use_standard_requested_resolution" flag and change the "VideoStreamEncoderResolutionTest" TEST_P to TEST_F. Bug: webrtc:366067962, webrtc:366284861 Change-Id: I1dfd10963274c5fdfd18d0f4443b2f209d2e9a4b Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/362720 Reviewed-by: Jonas Oreland <jonaso@webrtc.org> Commit-Queue: Henrik Boström <hbos@webrtc.org> Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org> Reviewed-by: Henrik Andreassson <henrika@webrtc.org> Cr-Commit-Position: refs/heads/main@{#43037}
This commit is contained in:
parent
52ea2c3d2a
commit
825e4f19ce
@ -57,25 +57,6 @@ struct VideoStreamEncoderSettings {
|
||||
// Enables the frame instrumentation generator that is required for automatic
|
||||
// corruption detection.
|
||||
bool enable_frame_instrumentation_generator = false;
|
||||
|
||||
// According to spec, `requested_resolution` (called scaleResolutionDownTo in
|
||||
// the web API) MUST NOT modify the aspect ratio of the frame, e.g. a 1280x720
|
||||
// frame being restricted to maxWidth by maxHeight 720x720 should result in
|
||||
// 720x405. In order for this to work, the video source must not adapt the
|
||||
// input frame to the value of `requested_resolution`, as that would result in
|
||||
// stretched 720x720.
|
||||
//
|
||||
// In order not to break backwards compatibility with C++ usage of this API,
|
||||
// when `use_standard_requested_resolution` is false, the
|
||||
// `requested_resolution` is signaled back to the video source. This works as
|
||||
// long as the aspect ratio is the same, but breaks the web API use case.
|
||||
//
|
||||
// Spec:
|
||||
// https://w3c.github.io/webrtc-extensions/#dom-rtcrtpencodingparameters-scaleresolutiondownto
|
||||
//
|
||||
// TODO(https://crbug.com/webrtc/366284861): Change the default to true,
|
||||
// delete this flag and any code handling the legacy behavior.
|
||||
bool use_standard_requested_resolution = false;
|
||||
};
|
||||
|
||||
} // namespace webrtc
|
||||
|
||||
@ -92,6 +92,7 @@ rtc_library("rtc_media_base") {
|
||||
"../api/transport/rtp:rtp_source",
|
||||
"../api/units:time_delta",
|
||||
"../api/video:recordable_encoded_frame",
|
||||
"../api/video:resolution",
|
||||
"../api/video:video_bitrate_allocation",
|
||||
"../api/video:video_bitrate_allocator_factory",
|
||||
"../api/video:video_frame",
|
||||
@ -159,6 +160,7 @@ rtc_library("video_adapter") {
|
||||
]
|
||||
deps = [
|
||||
":video_common",
|
||||
"../api/video:resolution",
|
||||
"../api/video:video_frame",
|
||||
"../common_video",
|
||||
"../rtc_base:checks",
|
||||
|
||||
@ -211,6 +211,28 @@ bool VideoAdapter::AdaptFrameResolution(int in_width,
|
||||
return false;
|
||||
}
|
||||
|
||||
// Adjust input width and height without modifying aspect ratio as to be less
|
||||
// than or equal to the `requested_resolution_`.
|
||||
if (requested_resolution_.has_value()) {
|
||||
// Make frame and requested resolution have matching orientation.
|
||||
webrtc::Resolution requested_resolution = requested_resolution_.value();
|
||||
if ((in_width < in_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 (in_width > 0 && in_height > 0 &&
|
||||
(requested_resolution.width < in_width ||
|
||||
requested_resolution.height < in_height)) {
|
||||
double scale_factor = std::min(
|
||||
requested_resolution.width / static_cast<double>(in_width),
|
||||
requested_resolution.height / static_cast<double>(in_height));
|
||||
in_width = std::round(in_width * scale_factor);
|
||||
in_height = std::round(in_height * scale_factor);
|
||||
}
|
||||
}
|
||||
|
||||
// Calculate how the input should be cropped.
|
||||
if (!target_aspect_ratio || target_aspect_ratio->first <= 0 ||
|
||||
target_aspect_ratio->second <= 0) {
|
||||
@ -341,6 +363,13 @@ void VideoAdapter::OnSinkWants(const rtc::VideoSinkWants& sink_wants) {
|
||||
max_framerate_request_ = sink_wants.max_framerate_fps;
|
||||
resolution_alignment_ = cricket::LeastCommonMultiple(
|
||||
source_resolution_alignment_, sink_wants.resolution_alignment);
|
||||
// Convert from std::optional<rtc::VideoSinkWants::FrameSize> to
|
||||
// std::optional<webrtc::Resolution>. Both are {int,int}.
|
||||
requested_resolution_ = std::nullopt;
|
||||
if (sink_wants.requested_resolution.has_value()) {
|
||||
requested_resolution_ = {.width = sink_wants.requested_resolution->width,
|
||||
.height = sink_wants.requested_resolution->height};
|
||||
}
|
||||
|
||||
// If requested_resolution is used, and there are no active encoders
|
||||
// that are NOT using requested_resolution (aka newapi), then override
|
||||
@ -371,7 +400,7 @@ void VideoAdapter::OnSinkWants(const rtc::VideoSinkWants& sink_wants) {
|
||||
}
|
||||
|
||||
if (!stashed_output_format_request_) {
|
||||
// The active output format request is about to be rewritten by
|
||||
// The active output format request is about to be cleared due to
|
||||
// request_resolution. We need to save it for later use in case the encoder
|
||||
// which doesn't use request_resolution logic become active in the future.
|
||||
stashed_output_format_request_ = output_format_request_;
|
||||
@ -379,22 +408,9 @@ void VideoAdapter::OnSinkWants(const rtc::VideoSinkWants& sink_wants) {
|
||||
<< stashed_output_format_request_->ToString();
|
||||
}
|
||||
|
||||
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);
|
||||
output_format_request_.max_landscape_pixel_count = pixel_count;
|
||||
output_format_request_.target_portrait_aspect_ratio =
|
||||
std::make_pair(res.height, res.width);
|
||||
output_format_request_.max_portrait_pixel_count = pixel_count;
|
||||
output_format_request_.max_fps = max_framerate_request_;
|
||||
RTC_LOG(LS_INFO) << "Setting output_format_request_ based on sink_wants: "
|
||||
<< output_format_request_.ToString();
|
||||
// Clear the output format request. Requested resolution will be applied
|
||||
// instead which happens inside AdaptFrameResolution().
|
||||
output_format_request_ = {};
|
||||
}
|
||||
|
||||
int VideoAdapter::GetTargetPixels() const {
|
||||
|
||||
@ -17,6 +17,7 @@
|
||||
#include <string>
|
||||
#include <utility>
|
||||
|
||||
#include "api/video/resolution.h"
|
||||
#include "api/video/video_source_interface.h"
|
||||
#include "common_video/framerate_controller.h"
|
||||
#include "media/base/video_common.h"
|
||||
@ -148,6 +149,8 @@ class RTC_EXPORT VideoAdapter {
|
||||
int resolution_request_target_pixel_count_ RTC_GUARDED_BY(mutex_);
|
||||
int resolution_request_max_pixel_count_ RTC_GUARDED_BY(mutex_);
|
||||
int max_framerate_request_ RTC_GUARDED_BY(mutex_);
|
||||
std::optional<webrtc::Resolution> requested_resolution_
|
||||
RTC_GUARDED_BY(mutex_);
|
||||
|
||||
// Stashed OutputFormatRequest that is used to save value of
|
||||
// OnOutputFormatRequest in case all active encoders are using
|
||||
|
||||
@ -59,13 +59,8 @@ rtc::VideoSinkWants BuildSinkWants(
|
||||
wants.resolution_alignment = 1;
|
||||
wants.is_active = true;
|
||||
if (requested_resolution) {
|
||||
wants.target_pixel_count = requested_resolution->PixelCount();
|
||||
wants.max_pixel_count = requested_resolution->PixelCount();
|
||||
wants.requested_resolution.emplace(rtc::VideoSinkWants::FrameSize(
|
||||
requested_resolution->width, requested_resolution->height));
|
||||
} else {
|
||||
wants.target_pixel_count = kWidth * kHeight;
|
||||
wants.max_pixel_count = kWidth * kHeight;
|
||||
}
|
||||
wants.aggregates.emplace(rtc::VideoSinkWants::Aggregates());
|
||||
wants.aggregates->any_active_without_requested_resolution =
|
||||
@ -1306,6 +1301,19 @@ TEST_P(VideoAdapterTest, RequestedResolutionIsOrientationAgnostic) {
|
||||
}
|
||||
}
|
||||
|
||||
TEST_P(VideoAdapterTest, RequestedResolutionMaintainsAspectRatio) {
|
||||
// Request 720x720.
|
||||
adapter_.OnSinkWants(
|
||||
BuildSinkWants(Resolution{.width = 720, .height = 720},
|
||||
/* any_active_without_requested_resolution= */ false));
|
||||
|
||||
// A 1280x720 frame restricted to 720x720 produces 720x405.
|
||||
EXPECT_THAT(
|
||||
AdaptFrameResolution(/* input frame */ {.width = 1280, .height = 720})
|
||||
.first,
|
||||
Eq(Resolution{.width = 720, .height = 405}));
|
||||
}
|
||||
|
||||
class VideoAdapterWithSourceAlignmentTest : public VideoAdapterTest {
|
||||
protected:
|
||||
static constexpr int kSourceResolutionAlignment = 7;
|
||||
|
||||
@ -891,18 +891,12 @@ void VideoStreamEncoder::ConfigureEncoder(VideoEncoderConfig config,
|
||||
}
|
||||
}
|
||||
}
|
||||
const bool signal_requested_resolution =
|
||||
!settings_.use_standard_requested_resolution;
|
||||
if ((signal_requested_resolution &&
|
||||
requested_resolution !=
|
||||
video_source_sink_controller_.requested_resolution()) ||
|
||||
if (requested_resolution !=
|
||||
video_source_sink_controller_.requested_resolution() ||
|
||||
active != video_source_sink_controller_.active() ||
|
||||
max_framerate !=
|
||||
video_source_sink_controller_.frame_rate_upper_limit().value_or(-1)) {
|
||||
if (signal_requested_resolution) {
|
||||
video_source_sink_controller_.SetRequestedResolution(
|
||||
requested_resolution);
|
||||
}
|
||||
video_source_sink_controller_.SetRequestedResolution(requested_resolution);
|
||||
if (max_framerate >= 0) {
|
||||
video_source_sink_controller_.SetFrameRateUpperLimit(max_framerate);
|
||||
} else {
|
||||
|
||||
@ -2530,56 +2530,14 @@ TEST_F(VideoStreamEncoderTest, FrameRateLimitCanBeReset) {
|
||||
video_stream_encoder_->Stop();
|
||||
}
|
||||
|
||||
// Tests both the standard and non-standard version of `requested_resolution`
|
||||
// (also known as scaleResolutionDownTo) which behave differently with regards
|
||||
// to signaling `requested_resolution` back to the source.
|
||||
enum class RequestedResolutionVersion {
|
||||
kLegacyRequestedResolution,
|
||||
kStandardRequestedResolution,
|
||||
};
|
||||
class VideoStreamEncoderResolutionTest
|
||||
: public VideoStreamEncoderTest,
|
||||
public ::testing::WithParamInterface<RequestedResolutionVersion> {
|
||||
public:
|
||||
VideoStreamEncoderResolutionTest()
|
||||
: VideoStreamEncoderTest(), requested_resolution_version_(GetParam()) {}
|
||||
TEST_F(VideoStreamEncoderTest, RequestInSinkWantsBeforeFirstFrame) {
|
||||
// Use a real video stream factory or else `requested_resolution` is not
|
||||
// applied correctly.
|
||||
video_encoder_config_.video_stream_factory = nullptr;
|
||||
ConfigureEncoder(video_encoder_config_.Copy());
|
||||
video_stream_encoder_->OnBitrateUpdatedAndWaitForManagedResources(
|
||||
kTargetBitrate, kTargetBitrate, kTargetBitrate, 0, 0, 0);
|
||||
|
||||
void SetUp() override {
|
||||
VideoStreamEncoderTest::SetUp();
|
||||
// Configure `use_standard_requested_resolution` based on test param.
|
||||
switch (requested_resolution_version_) {
|
||||
case RequestedResolutionVersion::kLegacyRequestedResolution:
|
||||
video_send_config_.encoder_settings.use_standard_requested_resolution =
|
||||
false;
|
||||
break;
|
||||
case RequestedResolutionVersion::kStandardRequestedResolution:
|
||||
video_send_config_.encoder_settings.use_standard_requested_resolution =
|
||||
true;
|
||||
break;
|
||||
}
|
||||
|
||||
// Use a real video stream factory or else `requested_resolution` is not
|
||||
// applied correctly.
|
||||
video_encoder_config_.video_stream_factory = nullptr;
|
||||
|
||||
ConfigureEncoder(video_encoder_config_.Copy());
|
||||
|
||||
video_stream_encoder_->OnBitrateUpdatedAndWaitForManagedResources(
|
||||
kTargetBitrate, kTargetBitrate, kTargetBitrate, 0, 0, 0);
|
||||
}
|
||||
|
||||
protected:
|
||||
const RequestedResolutionVersion requested_resolution_version_;
|
||||
};
|
||||
|
||||
INSTANTIATE_TEST_SUITE_P(
|
||||
VideoStreamEncoderResolutionTest,
|
||||
VideoStreamEncoderResolutionTest,
|
||||
::testing::Values(
|
||||
RequestedResolutionVersion::kLegacyRequestedResolution,
|
||||
RequestedResolutionVersion::kStandardRequestedResolution));
|
||||
|
||||
TEST_P(VideoStreamEncoderResolutionTest, RequestInSinkWantsBeforeFirstFrame) {
|
||||
ASSERT_THAT(video_encoder_config_.simulcast_layers, SizeIs(1));
|
||||
video_encoder_config_.simulcast_layers[0].requested_resolution.emplace(
|
||||
Resolution({.width = 320, .height = 160}));
|
||||
@ -2587,37 +2545,28 @@ TEST_P(VideoStreamEncoderResolutionTest, RequestInSinkWantsBeforeFirstFrame) {
|
||||
video_stream_encoder_->ConfigureEncoder(video_encoder_config_.Copy(),
|
||||
kMaxPayloadLength);
|
||||
|
||||
// The legacy path signals `requested_resolution` back to the source. The
|
||||
// standard path does not.
|
||||
switch (requested_resolution_version_) {
|
||||
case RequestedResolutionVersion::kLegacyRequestedResolution:
|
||||
EXPECT_EQ(video_source_.sink_wants().requested_resolution,
|
||||
rtc::VideoSinkWants::FrameSize(320, 160));
|
||||
break;
|
||||
case RequestedResolutionVersion::kStandardRequestedResolution:
|
||||
EXPECT_FALSE(video_source_.sink_wants().requested_resolution.has_value());
|
||||
break;
|
||||
}
|
||||
EXPECT_EQ(video_source_.sink_wants().requested_resolution,
|
||||
rtc::VideoSinkWants::FrameSize(320, 160));
|
||||
|
||||
video_encoder_config_.simulcast_layers[0].requested_resolution->height = 320;
|
||||
video_encoder_config_.simulcast_layers[0].requested_resolution->width = 640;
|
||||
video_stream_encoder_->ConfigureEncoder(video_encoder_config_.Copy(),
|
||||
kMaxPayloadLength);
|
||||
|
||||
switch (requested_resolution_version_) {
|
||||
case RequestedResolutionVersion::kLegacyRequestedResolution:
|
||||
EXPECT_EQ(video_source_.sink_wants().requested_resolution,
|
||||
rtc::VideoSinkWants::FrameSize(640, 320));
|
||||
break;
|
||||
case RequestedResolutionVersion::kStandardRequestedResolution:
|
||||
EXPECT_FALSE(video_source_.sink_wants().requested_resolution.has_value());
|
||||
break;
|
||||
}
|
||||
EXPECT_EQ(video_source_.sink_wants().requested_resolution,
|
||||
rtc::VideoSinkWants::FrameSize(640, 320));
|
||||
|
||||
video_stream_encoder_->Stop();
|
||||
}
|
||||
|
||||
TEST_P(VideoStreamEncoderResolutionTest, RequestInWrongAspectRatioWithAdapter) {
|
||||
TEST_F(VideoStreamEncoderTest, RequestInWrongAspectRatioWithAdapter) {
|
||||
// Use a real video stream factory or else `requested_resolution` is not
|
||||
// applied correctly.
|
||||
video_encoder_config_.video_stream_factory = nullptr;
|
||||
ConfigureEncoder(video_encoder_config_.Copy());
|
||||
video_stream_encoder_->OnBitrateUpdatedAndWaitForManagedResources(
|
||||
kTargetBitrate, kTargetBitrate, kTargetBitrate, 0, 0, 0);
|
||||
|
||||
// Use a source that adapts resolution based on OnSinkWants.
|
||||
AdaptingFrameForwarder source(&time_controller_);
|
||||
source.set_adaptation_enabled(true);
|
||||
@ -2633,16 +2582,8 @@ TEST_P(VideoStreamEncoderResolutionTest, RequestInWrongAspectRatioWithAdapter) {
|
||||
// Capture a 60x30 frame.
|
||||
source.IncomingCapturedFrame(CreateFrame(1, 60, 30));
|
||||
// The 60x30 frame does not fit inside the 30x30 restrictions.
|
||||
// - The non-standard path produces 30x30 due to OnSinkWants() signaling.
|
||||
// - The standard path produces 30x15, maintaining aspect ratio.
|
||||
switch (requested_resolution_version_) {
|
||||
case RequestedResolutionVersion::kLegacyRequestedResolution:
|
||||
WaitForEncodedFrame(30, 30);
|
||||
break;
|
||||
case RequestedResolutionVersion::kStandardRequestedResolution:
|
||||
WaitForEncodedFrame(30, 15);
|
||||
break;
|
||||
}
|
||||
// Expect 30x15, maintaining aspect ratio.
|
||||
WaitForEncodedFrame(30, 15);
|
||||
|
||||
video_stream_encoder_->Stop();
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user