diff --git a/modules/video_coding/codecs/vp9/libvpx_vp9_encoder.cc b/modules/video_coding/codecs/vp9/libvpx_vp9_encoder.cc index c2a17c59d4..2da0918062 100644 --- a/modules/video_coding/codecs/vp9/libvpx_vp9_encoder.cc +++ b/modules/video_coding/codecs/vp9/libvpx_vp9_encoder.cc @@ -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_); } diff --git a/pc/peer_connection_encodings_integrationtest.cc b/pc/peer_connection_encodings_integrationtest.cc index b8b7e18e58..8058379666 100644 --- a/pc/peer_connection_encodings_integrationtest.cc +++ b/pc/peer_connection_encodings_integrationtest.cc @@ -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 local_pc_wrapper = CreatePc(); + if (SkipTestDueToAv1Missing(local_pc_wrapper)) { + return; + } + rtc::scoped_refptr remote_pc_wrapper = CreatePc(); + ExchangeIceCandidates(local_pc_wrapper, remote_pc_wrapper); + + std::vector layers = + CreateLayers({"q", "h", "f"}, /*active=*/true); + rtc::scoped_refptr transceiver = + AddTransceiverWithSimulcastLayers(local_pc_wrapper, remote_pc_wrapper, + layers); + std::vector codecs = + GetCapabilitiesAndRestrictToCodec(remote_pc_wrapper, codec_name_); + transceiver->SetCodecPreferences(codecs); + rtc::scoped_refptr 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