Skip to content

Commit

Permalink
Bug 1924098 - Vendor libwebrtc from 254bd32188
Browse files Browse the repository at this point in the history 
Upstream commit: https://webrtc.googlesource.com/src/+/254bd32188e654b2c03b7d9fc29851d5197200b6
    Update when/how `requested_resolution` throws for invalid parameters.

    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] w3c/webrtc-extensions#221

    Bug: none
    Change-Id: I21165c9b9f9ee7259d88b89f9ae58b862ea4521e
    Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/362260
    Commit-Queue: Henrik Boström <[email protected]>
    Reviewed-by: Ilya Nikolaevskiy <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#43002}
  • Loading branch information
@mfromanmoz
mfromanmoz committed Oct 15, 2024
1 parent fcc9a7a commit 770ef0a
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 5 deletions.
3 changes: 3 additions & 0 deletions third_party/libwebrtc/README.moz-ff-commit
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32793,3 +32793,6 @@ e184c56bef
# MOZ_LIBWEBRTC_SRC=/home/mfroman/mozilla/elm/.moz-fast-forward/moz-libwebrtc MOZ_LIBWEBRTC_BRANCH=mozpatches bash dom/media/webrtc/third_party_build/fast-forward-libwebrtc.sh
# base of lastest vendoring
1bd331f102
# MOZ_LIBWEBRTC_SRC=/home/mfroman/mozilla/elm/.moz-fast-forward/moz-libwebrtc MOZ_LIBWEBRTC_BRANCH=mozpatches bash dom/media/webrtc/third_party_build/fast-forward-libwebrtc.sh
# base of lastest vendoring
254bd32188
2 changes: 2 additions & 0 deletions third_party/libwebrtc/README.mozilla
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21886,3 +21886,5 @@ libwebrtc updated from /home/mfroman/mozilla/elm/.moz-fast-forward/moz-libwebrtc
libwebrtc updated from /home/mfroman/mozilla/elm/.moz-fast-forward/moz-libwebrtc commit mozpatches on 2024-10-15T23:49:54.669234.
# ./mach python dom/media/webrtc/third_party_build/vendor-libwebrtc.py --from-local /home/mfroman/mozilla/elm/.moz-fast-forward/moz-libwebrtc --commit mozpatches libwebrtc
libwebrtc updated from /home/mfroman/mozilla/elm/.moz-fast-forward/moz-libwebrtc commit mozpatches on 2024-10-15T23:50:51.433136.
# ./mach python dom/media/webrtc/third_party_build/vendor-libwebrtc.py --from-local /home/mfroman/mozilla/elm/.moz-fast-forward/moz-libwebrtc --commit mozpatches libwebrtc
libwebrtc updated from /home/mfroman/mozilla/elm/.moz-fast-forward/moz-libwebrtc commit mozpatches on 2024-10-15T23:52:28.075620.
18 changes: 13 additions & 5 deletions third_party/libwebrtc/media/base/media_engine.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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")) {
Expand All @@ -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);
}

Expand Down
98 changes: 98 additions & 0 deletions third_party/libwebrtc/pc/peer_connection_encodings_integrationtest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 770ef0a

Please sign in to comment.