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

Fix LibvpxVp9Encoder simulcast bug.

Browse Source 
As of [1], a single VP9 encoder instance can produce simulcast 4:2:1.
When it does, the EncodedImage has its simulcast index set (0, 1, 2).

The bug is that if you then go back to a single encoder instance,
either because you're doing singlecast or because you're doing
simulcast with scaling factors that are not power of two (not 4:2:1),
then the simulcast index which was previously set to 2 is not reset due
to the old code path never calling SetSimulcastIndex.

Example repro:
1. Send VP9 simulcast {180p, 360p, 720p}, i.e. 4:2.1.
2. Reconfigure to {180p, 360p, 540p}, i.e. no longer 4:2:1.

What should happen: all three layers are sent.
What actually happened: 180p is not sent and the 540p layer flips flops
between 180p and 540p because the EncodedImage says simulcast index is
2 for both encodings[0] and encodings[2].

The fix is a one-line change: `SetSimulcastIndex(std::nullopt)` in the
case that we don't have a `simulcast_to_svc_converter_` that sets it
(0, 1, 2) for us.

[1] https://webrtc-review.googlesource.com/c/src/+/360280

Bug: chromium:370299916
Change-Id: I52bd4428bd12528f0e98869ec61626c06f589b43
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/363941
Reviewed-by: Sergey Silkin <ssilkin@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43109}
This commit is contained in:
Henrik Boström 2024-09-30 14:41:50 +02:00 committed by WebRTC LUCI CQ
parent 0d3dcc4997
commit a6fbb35ac1
2 changed files with 73 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
modules/video_coding/codecs/vp9/libvpx_vp9_encoder.cc
View File

@ -1761,7 +1761,9 @@ void LibvpxVp9Encoder::DeliverBufferedFrame(bool end_of_picture) {
codec_specific_.end_of_picture = end_of_picture;
if (simulcast_to_svc_converter_) {
if (!simulcast_to_svc_converter_) {
encoded_image_.SetSimulcastIndex(std::nullopt);
} else {
simulcast_to_svc_converter_->ConvertFrame(encoded_image_,
codec_specific_);
}

70
pc/peer_connection_encodings_integrationtest.cc
View File

@ -2417,6 +2417,76 @@ TEST_P(PeerConnectionEncodingsIntegrationParameterizedTest,
EXPECT_THAT(*outbound_rtps[2]->scalability_mode, StrEq("L1T3"));
}
// Simulcast starting in 720p 4:2:1 then changing to {180p, 360p, 540p} using
// the `scale_resolution_down_by` API.
TEST_P(PeerConnectionEncodingsIntegrationParameterizedTest,
SimulcastScaleDownByNoLongerPowerOfTwo) {
rtc::scoped_refptr<PeerConnectionTestWrapper> local_pc_wrapper = CreatePc();
if (SkipTestDueToAv1Missing(local_pc_wrapper)) {
return;
}
rtc::scoped_refptr<PeerConnectionTestWrapper> remote_pc_wrapper = CreatePc();
ExchangeIceCandidates(local_pc_wrapper, remote_pc_wrapper);
std::vector<cricket::SimulcastLayer> layers =
CreateLayers({"q", "h", "f"}, /*active=*/true);
rtc::scoped_refptr<RtpTransceiverInterface> transceiver =
AddTransceiverWithSimulcastLayers(local_pc_wrapper, remote_pc_wrapper,
layers);
std::vector<RtpCodecCapability> codecs =
GetCapabilitiesAndRestrictToCodec(remote_pc_wrapper, codec_name_);
transceiver->SetCodecPreferences(codecs);
rtc::scoped_refptr<RtpSenderInterface> sender = transceiver->sender();
// Configure {180p, 360p, 720p}.
RtpParameters parameters = sender->GetParameters();
ASSERT_THAT(parameters.encodings, SizeIs(3));
parameters.encodings[0].scalability_mode = "L1T1";
parameters.encodings[0].scale_resolution_down_by = 4.0;
parameters.encodings[1].scalability_mode = "L1T1";
parameters.encodings[1].scale_resolution_down_by = 2.0;
parameters.encodings[2].scalability_mode = "L1T1";
parameters.encodings[2].scale_resolution_down_by = 1.0;
sender->SetParameters(parameters);
NegotiateWithSimulcastTweaks(local_pc_wrapper, remote_pc_wrapper);
local_pc_wrapper->WaitForConnection();
remote_pc_wrapper->WaitForConnection();
// Wait for media to flow on all layers.
// Needed repro step of https://crbug.com/webrtc/369654168: When the same
// LibvpxVp9Encoder instance was used to first produce simulcast and later for
// a single encoding, the previously used simulcast index (= 2) would still be
// set when producing 180p since non-simulcast config does not reset this,
// resulting in the 180p encoding freezing and the 540p encoding having double
// frame rate and toggling between 180p and 540p in resolution.
ASSERT_TRUE_WAIT(HasOutboundRtpBytesSent(local_pc_wrapper, 3u),
kLongTimeoutForRampingUp.ms());
// Configure {180p, 360p, 540p}.
parameters = sender->GetParameters();
parameters.encodings[0].scale_resolution_down_by = 4.0;
parameters.encodings[1].scale_resolution_down_by = 2.0;
parameters.encodings[2].scale_resolution_down_by = 1.333333;
sender->SetParameters(parameters);
// Wait for the new resolutions to be produced.
ASSERT_TRUE_WAIT(GetEncodingResolution(local_pc_wrapper, "q") ==
Resolution({.width = 320, .height = 180}),
kLongTimeoutForRampingUp.ms());
ASSERT_TRUE_WAIT(GetEncodingResolution(local_pc_wrapper, "h") ==
Resolution({.width = 640, .height = 360}),
kLongTimeoutForRampingUp.ms());
ASSERT_TRUE_WAIT(GetEncodingResolution(local_pc_wrapper, "f") ==
Resolution({.width = 960, .height = 540}),
kLongTimeoutForRampingUp.ms());
// Ensure 180p frames continue to be encoded post reconfiguration.
int frames_encoded = EncodedFrames(local_pc_wrapper, "q");
ASSERT_TRUE_WAIT(EncodedFrames(local_pc_wrapper, "q") > frames_encoded,
kLongTimeoutForRampingUp.ms());
}
// The code path that disables layers based on resolution size should NOT run
// when `requested_resolution` is specified. (It shouldn't run in any case but
// that is an existing legacy code and non-compliance problem that we don't have