admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Update when/how requested_resolution throws for invalid parameters.

Browse Source 
This CL makes `requested_resolution`, which is the C++ name for what
the spec calls scaleResolutionDownTo, align with the latest PR[1].

The PR says to ignore scaleResolutionDownBy when scaleResolutionDownTo
is specified as to be backwards compatible with scaleResolutionDownBy's
default scaling factors (e.g. 4:2:1). Ignoring is different than what
the code does today which is to throw an InvalidModificationError.

We don't want to throw or else get+setParameters() would throw by
default due to 4:2:1 defaults so the app would have to remember to
delete these attributes every time even though it never specified them
(Chrome has a bug here but fixing that would expose this problem, see
https://crbug.com/344943229).

[1] https://github.com/w3c/webrtc-extensions/pull/221

Bug: none
Change-Id: I21165c9b9f9ee7259d88b89f9ae58b862ea4521e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/362260
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43002}
This commit is contained in:
Henrik Boström 2024-09-11 10:32:08 +02:00 committed by WebRTC LUCI CQ
parent 1bd331f102
commit 254bd32188
2 changed files with 111 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
media/base/media_engine.cc
View File

@ -145,6 +145,7 @@ webrtc::RTCError CheckRtpParametersValues(
const webrtc::FieldTrialsView& field_trials) {
using webrtc::RTCErrorType;
bool has_requested_resolution = false;
for (size_t i = 0; i < rtp_parameters.encodings.size(); ++i) {
if (rtp_parameters.encodings[i].bitrate_priority <= 0) {
LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_RANGE,
@ -183,11 +184,8 @@ webrtc::RTCError CheckRtpParametersValues(
}
}
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.");
if (rtp_parameters.encodings[i].requested_resolution.has_value()) {
has_requested_resolution = true;
}
if (!field_trials.IsEnabled("WebRTC-MixedCodecSimulcast")) {
@ -200,6 +198,16 @@ webrtc::RTCError CheckRtpParametersValues(
}
}
if (has_requested_resolution &&
absl::c_any_of(rtp_parameters.encodings,
[](const webrtc::RtpEncodingParameters& encoding) {
return !encoding.requested_resolution.has_value();
})) {
LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_MODIFICATION,
"If a resolution is specified on any encoding then "
"it must be specified on all encodings.");
}
return CheckScalabilityModeValues(rtp_parameters, send_codecs, send_codec);
}

98
pc/peer_connection_encodings_integrationtest.cc
View File

@ -187,6 +187,24 @@ class PeerConnectionEncodingsIntegrationTest : public ::testing::Test {
local_pc_wrapper.get(), &PeerConnectionTestWrapper::AddIceCandidate);
}
// Negotiate without any tweaks (does not work for simulcast loopback).
void Negotiate(
rtc::scoped_refptr<PeerConnectionTestWrapper> local_pc_wrapper,
rtc::scoped_refptr<PeerConnectionTestWrapper> remote_pc_wrapper) {
std::unique_ptr<SessionDescriptionInterface> offer =
CreateOffer(local_pc_wrapper);
rtc::scoped_refptr<MockSetSessionDescriptionObserver> p1 =
SetLocalDescription(local_pc_wrapper, offer.get());
rtc::scoped_refptr<MockSetSessionDescriptionObserver> p2 =
SetRemoteDescription(remote_pc_wrapper, offer.get());
EXPECT_TRUE(Await({p1, p2}));
std::unique_ptr<SessionDescriptionInterface> answer =
CreateAnswer(remote_pc_wrapper);
p1 = SetLocalDescription(remote_pc_wrapper, answer.get());
p2 = SetRemoteDescription(local_pc_wrapper, answer.get());
EXPECT_TRUE(Await({p1, p2}));
}
void NegotiateWithSimulcastTweaks(
rtc::scoped_refptr<PeerConnectionTestWrapper> local_pc_wrapper,
rtc::scoped_refptr<PeerConnectionTestWrapper> remote_pc_wrapper) {
@ -2098,6 +2116,86 @@ TEST_F(PeerConnectionEncodingsIntegrationTest,
ASSERT_TRUE(transceiver_or_error.ok());
}
TEST_F(PeerConnectionEncodingsIntegrationTest,
RequestedResolutionParameterChecking) {
rtc::scoped_refptr<PeerConnectionTestWrapper> pc_wrapper = CreatePc();
// AddTransceiver: If `requested_resolution` is specified on any encoding it
// must be specified on all encodings.
RtpTransceiverInit init;
RtpEncodingParameters encoding;
encoding.requested_resolution = std::nullopt;
init.send_encodings.push_back(encoding);
encoding.requested_resolution = {.width = 1280, .height = 720};
init.send_encodings.push_back(encoding);
auto transceiver_or_error =
pc_wrapper->pc()->AddTransceiver(cricket::MEDIA_TYPE_VIDEO, init);
EXPECT_FALSE(transceiver_or_error.ok());
EXPECT_EQ(transceiver_or_error.error().type(),
RTCErrorType::UNSUPPORTED_OPERATION);
// AddTransceiver: Specifying both `requested_resolution` and
// `scale_resolution_down_by` is allowed (the latter is ignored).
init.send_encodings[0].requested_resolution = {.width = 640, .height = 480};
init.send_encodings[0].scale_resolution_down_by = 1.0;
init.send_encodings[1].requested_resolution = {.width = 1280, .height = 720};
init.send_encodings[1].scale_resolution_down_by = 2.0;
transceiver_or_error =
pc_wrapper->pc()->AddTransceiver(cricket::MEDIA_TYPE_VIDEO, init);
ASSERT_TRUE(transceiver_or_error.ok());
// SetParameters: If `requested_resolution` is specified on any encoding it
// must be specified on all encodings.
auto sender = transceiver_or_error.value()->sender();
auto parameters = sender->GetParameters();
parameters.encodings[0].requested_resolution = {.width = 640, .height = 480};
parameters.encodings[1].requested_resolution = std::nullopt;
auto error = sender->SetParameters(parameters);
EXPECT_FALSE(error.ok());
EXPECT_EQ(error.type(), RTCErrorType::INVALID_MODIFICATION);
// SetParameters: Specifying both `requested_resolution` and
// `scale_resolution_down_by` is allowed (the latter is ignored).
parameters = sender->GetParameters();
parameters.encodings[0].requested_resolution = {.width = 640, .height = 480};
parameters.encodings[0].scale_resolution_down_by = 2.0;
parameters.encodings[1].requested_resolution = {.width = 1280, .height = 720};
parameters.encodings[1].scale_resolution_down_by = 1.0;
error = sender->SetParameters(parameters);
EXPECT_TRUE(error.ok());
}
TEST_F(PeerConnectionEncodingsIntegrationTest,
ScaleResolutionDownByIsIgnoredWhenRequestedResolutionIsSpecified) {
rtc::scoped_refptr<PeerConnectionTestWrapper> local_pc_wrapper = CreatePc();
rtc::scoped_refptr<PeerConnectionTestWrapper> remote_pc_wrapper = CreatePc();
rtc::scoped_refptr<MediaStreamInterface> stream =
local_pc_wrapper->GetUserMedia(
/*audio=*/false, {}, /*video=*/true, {.width = 640, .height = 360});
rtc::scoped_refptr<VideoTrackInterface> track = stream->GetVideoTracks()[0];
// Configure contradicting scaling factors (180p vs 360p).
RtpTransceiverInit init;
RtpEncodingParameters encoding;
encoding.scale_resolution_down_by = 2.0;
encoding.requested_resolution = {.width = 640, .height = 360};
init.send_encodings.push_back(encoding);
auto transceiver_or_error =
local_pc_wrapper->pc()->AddTransceiver(track, init);
// Negotiate singlecast.
ExchangeIceCandidates(local_pc_wrapper, remote_pc_wrapper);
Negotiate(local_pc_wrapper, remote_pc_wrapper);
// Confirm 640x360 is sent.
// If `scale_resolution_down_by` was not ignored we would never ramp up to
// full resolution.
ASSERT_TRUE_WAIT(GetEncodingResolution(local_pc_wrapper) ==
(Resolution{.width = 640, .height = 360}),
kLongTimeoutForRampingUp.ms());
}
// Tests that use the standard path (specifying both `scalability_mode` and
// `scale_resolution_down_by` or `requested_resolution`) should pass for all
// codecs.