Add nonstandard x-google-per-layer-pli fmtp for enabling per-layer keyFrames in response to PLIs

which needs to be added to the remote codecs a=fmtp:

This also forces SimulcastCastEncoderAdapter to avoid issues with codecs that have native simulcast capability but do require synchronized keyframes.

This parameter allows for large-scale experimentation and A/B testing
whether the new behavior has advantages. It is to be considered
transitional and may be removed again in the future.

BUG=webrtc:10107

Change-Id: I81f496c987b2fed7ff3089efb746e7e89e89c033
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/333560
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41805}

docs/native-code/sdp-ext/fmtp-x-google-per-layer-pli.md

@@ -0,0 +1,36 @@
+# x-google-per-layer-pli FMTP parameter
+
+The x-google-per-layer-pli FMTP parameter is a format specific parameter
+that can be added to a remote description in the `a=fmtp:` line:
+  a=fmtp:96 x-google-per-layer-pli=1
+
+When using simulcast with more than a single SSRC, it will change how the
+simulcast encoder reacts to Picture Loss Indication (PLI) and Full Intra
+Request (FIR) RTCP feedback.
+
+When the parameter value is 1, a PLI requests the generation of a key frame
+for the spatial layer associated with the SSRC of the media source and a
+FIR does the same for the SSRC value of the media sender.
+
+When the value is 0 or the parameter is missing, a keyframe is generated
+on all spatial layers for backward compability.
+
+## Experimentation
+
+This parameter does allow for large-scale A/B testing and opt-in to the
+new behavior. For multiparty calls enabling it should reduce the number of
+keyframes sent by the client and number of key frames received by the
+receivers which results in a better bandwidth utilization.
+
+This parameter is experimental and may be removed again in the future.
+
+## IANA considerations
+
+Since the current behavior of reacting to a PLI for a specific SSRC with
+key frames on all spatial layers can be considered an implementation bug
+this parameter is not registered with the IANA.
+
+If experimentation shows that the current behavior is better for some
+codecs like VP8 which can share encoding parameters with synchronous
+keyframes a standardized variant of this parameter shall be registered
+with the IANA.

media/BUILD.gn

@@ -37,11 +37,15 @@ rtc_library("rtc_sdp_video_format_utils") {
   ]
 
   deps = [
+    ":media_constants",
     "../api/video_codecs:video_codecs_api",
     "../rtc_base:checks",
     "../rtc_base:stringutils",
   ]
-  absl_deps = [ "//third_party/abseil-cpp/absl/types:optional" ]
+  absl_deps = [
+    "//third_party/abseil-cpp/absl/algorithm:container",
+    "//third_party/abseil-cpp/absl/types:optional",
+  ]
 }
 
 rtc_library("rtc_media_base") {
@@ -337,6 +341,7 @@ rtc_library("rtc_simulcast_encoder_adapter") {
   ]
   deps = [
     ":rtc_media_base",
+    ":rtc_sdp_video_format_utils",
     "../api:fec_controller_api",
     "../api:field_trials_view",
     "../api:scoped_refptr",

media/base/media_constants.cc

@@ -96,6 +96,8 @@ const char kCodecParamMaxBitrate[] = "x-google-max-bitrate";
 const char kCodecParamMinBitrate[] = "x-google-min-bitrate";
 const char kCodecParamStartBitrate[] = "x-google-start-bitrate";
 const char kCodecParamMaxQuantization[] = "x-google-max-quantization";
+const char kCodecParamPerLayerPictureLossIndication[] =
+    "x-google-per-layer-pli";
 
 const char kComfortNoiseCodecName[] = "CN";
 

media/base/media_constants.h

@@ -64,6 +64,7 @@ extern const char kCodecParamUseInbandFec[];
 extern const char kCodecParamUseDtx[];
 extern const char kCodecParamMaxAverageBitrate[];
 extern const char kCodecParamMaxPlaybackRate[];
+extern const char kCodecParamPerLayerPictureLossIndication[];
 
 extern const char kParamValueTrue[];
 // Parameters are stored as parameter/value pairs. For parameters who do not

media/base/sdp_video_format_utils.cc

@@ -12,12 +12,15 @@
 
 #include <cstring>
 #include <map>
+#include <string>
 #include <utility>
 
 #include "api/video_codecs/h264_profile_level_id.h"
 #ifdef RTC_ENABLE_H265
 #include "api/video_codecs/h265_profile_tier_level.h"
 #endif
+#include "absl/algorithm/container.h"
+#include "media/base/media_constants.h"
 #include "rtc_base/checks.h"
 #include "rtc_base/string_to_number.h"
 
@@ -177,4 +180,13 @@ absl::optional<int> ParseSdpForVPxMaxFrameSize(
            : absl::nullopt;
 }
 
+bool SupportsPerLayerPictureLossIndication(const CodecParameterMap& params) {
+  return absl::c_find_if(
+             params, [](const std::pair<std::string, std::string>& kv) {
+               return kv.first ==
+                          cricket::kCodecParamPerLayerPictureLossIndication &&
+                      kv.second == "1";
+             }) != params.end();
+}
+
 }  // namespace webrtc

media/base/sdp_video_format_utils.h

@@ -57,6 +57,10 @@ absl::optional<int> ParseSdpForVPxMaxFrameRate(const CodecParameterMap& params);
 // blocks but the returned value is in total number of pixels.
 absl::optional<int> ParseSdpForVPxMaxFrameSize(const CodecParameterMap& params);
 
+// Determines whether the non-standard x-google-per-layer-pli fmtp is present
+// in the parameters and has a value of "1".
+bool SupportsPerLayerPictureLossIndication(const CodecParameterMap& params);
+
 }  // namespace webrtc
 
 #endif  // MEDIA_BASE_SDP_VIDEO_FORMAT_UTILS_H_

media/base/sdp_video_format_utils_unittest.cc

@@ -24,6 +24,9 @@ namespace {
 const char kVPxFmtpMaxFrameRate[] = "max-fr";
 // Max frame size for VP8 and VP9 video.
 const char kVPxFmtpMaxFrameSize[] = "max-fs";
+// Nonstandard per-layer PLI for video.
+const char kCodecParamPerLayerPictureLossIndication[] =
+    "x-google-per-layer-pli";
 }  // namespace
 
 TEST(SdpVideoFormatUtilsTest, TestH264GenerateProfileLevelIdForAnswerEmpty) {
@@ -141,4 +144,15 @@ TEST(SdpVideoFormatUtilsTest, MaxFrameSizeIsSpecified) {
   EXPECT_EQ(ParseSdpForVPxMaxFrameSize(params), 3840 * 2160);
 }
 
+TEST(SdpVideoFormatUtilsTest, PerLayerPictureLossIndication) {
+  CodecParameterMap params;
+  EXPECT_FALSE(SupportsPerLayerPictureLossIndication(params));
+  params[kCodecParamPerLayerPictureLossIndication] = "wrong";
+  EXPECT_FALSE(SupportsPerLayerPictureLossIndication(params));
+  params[kCodecParamPerLayerPictureLossIndication] = "0";
+  EXPECT_FALSE(SupportsPerLayerPictureLossIndication(params));
+  params[kCodecParamPerLayerPictureLossIndication] = "1";
+  EXPECT_TRUE(SupportsPerLayerPictureLossIndication(params));
+}
+
 }  // namespace webrtc

media/engine/simulcast_encoder_adapter.cc

@@ -30,8 +30,8 @@
 #include "api/video_codecs/video_encoder_factory.h"
 #include "api/video_codecs/video_encoder_software_fallback_wrapper.h"
 #include "media/base/media_constants.h"
+#include "media/base/sdp_video_format_utils.h"
 #include "media/base/video_common.h"
-#include "modules/video_coding/include/video_error_codes.h"
 #include "modules/video_coding/utility/simulcast_rate_allocator.h"
 #include "rtc_base/checks.h"
 #include "rtc_base/experiments/rate_control_settings.h"
@@ -268,7 +268,8 @@ SimulcastEncoderAdapter::SimulcastEncoderAdapter(
           RateControlSettings::ParseFromKeyValueConfig(&field_trials)
               .Vp8BoostBaseLayerQuality()),
       prefer_temporal_support_on_base_layer_(field_trials.IsEnabled(
-          "WebRTC-Video-PreferTemporalSupportOnBaseLayer")) {
+          "WebRTC-Video-PreferTemporalSupportOnBaseLayer")),
+      per_layer_pli_(SupportsPerLayerPictureLossIndication(format.parameters)) {
   RTC_DCHECK(primary_factory);
 
   // The adapter is typically created on the worker thread, but operated on
@@ -357,9 +358,12 @@ int SimulcastEncoderAdapter::InitEncode(
   // If we only have a single active layer it is better to create an encoder
   // with only one configured layer than creating it with all-but-one disabled
   // layers because that way we control scaling.
+  // The use of the nonstandard x-google-per-layer-pli fmtp parameter also
+  // forces the use of SEA with separate encoders to support per-layer
+  // handling of PLIs.
   bool separate_encoders_needed =
       !encoder_context->encoder().GetEncoderInfo().supports_simulcast ||
-      active_streams_count == 1;
+      active_streams_count == 1 || per_layer_pli_;
   // Singlecast or simulcast with simulcast-capable underlaying encoder.
   if (total_streams_count_ == 1 || !separate_encoders_needed) {
     int ret = encoder_context->encoder().InitEncode(&codec_, settings);

media/engine/simulcast_encoder_adapter.h

@@ -191,6 +191,7 @@ class RTC_EXPORT SimulcastEncoderAdapter : public VideoEncoder {
   const absl::optional<unsigned int> experimental_boosted_screenshare_qp_;
   const bool boost_base_layer_quality_;
   const bool prefer_temporal_support_on_base_layer_;
+  const bool per_layer_pli_;
 
   const SimulcastEncoderAdapterEncoderInfoSettings encoder_info_override_;
 };

modules/rtp_rtcp/source/rtcp_receiver.cc

@@ -87,7 +87,6 @@ bool ResetTimestampIfExpired(const Timestamp now,
 }  // namespace
 
 constexpr size_t RTCPReceiver::RegisteredSsrcs::kMediaSsrcIndex;
-constexpr size_t RTCPReceiver::RegisteredSsrcs::kMaxSsrcs;
 
 RTCPReceiver::RegisteredSsrcs::RegisteredSsrcs(
     bool disable_sequence_checker,
@@ -105,7 +104,7 @@ RTCPReceiver::RegisteredSsrcs::RegisteredSsrcs(
     }
   }
   // Ensure that the RegisteredSsrcs can inline the SSRCs.
-  RTC_DCHECK_LE(ssrcs_.size(), RTCPReceiver::RegisteredSsrcs::kMaxSsrcs);
+  RTC_DCHECK_LE(ssrcs_.size(), kMaxSimulcastStreams);
 }
 
 bool RTCPReceiver::RegisteredSsrcs::contains(uint32_t ssrc) const {

modules/rtp_rtcp/source/rtcp_receiver.h

@@ -179,7 +179,6 @@ class RTCPReceiver final {
   class RegisteredSsrcs {
    public:
     static constexpr size_t kMediaSsrcIndex = 0;
-    static constexpr size_t kMaxSsrcs = 3;
     // Initializes the set of registered local SSRCS by extracting them from the
     // provided `config`. The `disable_sequence_checker` flag is a workaround
     // to be able to use a sequence checker without breaking downstream
@@ -194,7 +193,7 @@ class RTCPReceiver final {
 
    private:
     RTC_NO_UNIQUE_ADDRESS CustomSequenceChecker packet_sequence_checker_;
-    absl::InlinedVector<uint32_t, kMaxSsrcs> ssrcs_
+    absl::InlinedVector<uint32_t, kMaxSimulcastStreams> ssrcs_
         RTC_GUARDED_BY(packet_sequence_checker_);
   };
 

video/BUILD.gn

@@ -119,6 +119,7 @@ rtc_library("video") {
     "../call:video_stream_api",
     "../common_video",
     "../media:media_constants",
+    "../media:rtc_sdp_video_format_utils",
     "../modules:module_api",
     "../modules:module_api_public",
     "../modules/pacing",

video/encoder_rtcp_feedback.cc

@@ -26,6 +26,7 @@ constexpr int kMinKeyframeSendIntervalMs = 300;
 
 EncoderRtcpFeedback::EncoderRtcpFeedback(
     Clock* clock,
+    bool per_layer_keyframes,
     const std::vector<uint32_t>& ssrcs,
     VideoStreamEncoderInterface* encoder,
     std::function<std::vector<RtpSequenceNumberMap::Info>(
@@ -33,9 +34,11 @@ EncoderRtcpFeedback::EncoderRtcpFeedback(
         const std::vector<uint16_t>& seq_nums)> get_packet_infos)
     : clock_(clock),
       ssrcs_(ssrcs),
+      per_layer_keyframes_(per_layer_keyframes),
       get_packet_infos_(std::move(get_packet_infos)),
       video_stream_encoder_(encoder),
-      time_last_packet_delivery_queue_(Timestamp::Zero()),
+      time_last_packet_delivery_queue_(per_layer_keyframes ? ssrcs.size() : 1,
+                                       Timestamp::Zero()),
       min_keyframe_send_interval_(
           TimeDelta::Millis(KeyframeIntervalSettings::ParseFromFieldTrials()
                                 .MinKeyframeSendIntervalMs()
@@ -49,14 +52,32 @@ void EncoderRtcpFeedback::OnReceivedIntraFrameRequest(uint32_t ssrc) {
   RTC_DCHECK_RUN_ON(&packet_delivery_queue_);
   RTC_DCHECK(std::find(ssrcs_.begin(), ssrcs_.end(), ssrc) != ssrcs_.end());
 
+  auto it = std::find(ssrcs_.begin(), ssrcs_.end(), ssrc);
+  if (it == ssrcs_.end()) {
+    RTC_LOG(LS_WARNING) << "SSRC " << ssrc << " not found.";
+    return;
+  }
+  size_t ssrc_index =
+      per_layer_keyframes_ ? std::distance(ssrcs_.begin(), it) : 0;
+  RTC_CHECK_LE(ssrc_index, time_last_packet_delivery_queue_.size());
   const Timestamp now = clock_->CurrentTime();
-  if (time_last_packet_delivery_queue_ + min_keyframe_send_interval_ > now)
+  if (time_last_packet_delivery_queue_[ssrc_index] +
+          min_keyframe_send_interval_ >
+      now)
     return;
 
-  time_last_packet_delivery_queue_ = now;
+  time_last_packet_delivery_queue_[ssrc_index] = now;
 
-  // Always produce key frame for all streams.
-  video_stream_encoder_->SendKeyFrame();
+  std::vector<VideoFrameType> layers(ssrcs_.size(),
+                                     VideoFrameType::kVideoFrameDelta);
+  if (!per_layer_keyframes_) {
+    // Always produce key frame for all streams.
+    video_stream_encoder_->SendKeyFrame();
+  } else {
+    // Determine on which layer we ask for key frames.
+    layers[ssrc_index] = VideoFrameType::kVideoFrameKey;
+    video_stream_encoder_->SendKeyFrame(layers);
+  }
 }
 
 void EncoderRtcpFeedback::OnReceivedLossNotification(

video/encoder_rtcp_feedback.h

@@ -33,6 +33,7 @@ class EncoderRtcpFeedback : public RtcpIntraFrameObserver,
  public:
   EncoderRtcpFeedback(
       Clock* clock,
+      bool per_layer_keyframes,
       const std::vector<uint32_t>& ssrcs,
       VideoStreamEncoderInterface* encoder,
       std::function<std::vector<RtpSequenceNumberMap::Info>(
@@ -51,6 +52,7 @@ class EncoderRtcpFeedback : public RtcpIntraFrameObserver,
  private:
   Clock* const clock_;
   const std::vector<uint32_t> ssrcs_;
+  const bool per_layer_keyframes_;
   const std::function<std::vector<RtpSequenceNumberMap::Info>(
       uint32_t ssrc,
       const std::vector<uint16_t>& seq_nums)>
@@ -58,7 +60,7 @@ class EncoderRtcpFeedback : public RtcpIntraFrameObserver,
   VideoStreamEncoderInterface* const video_stream_encoder_;
 
   RTC_NO_UNIQUE_ADDRESS SequenceChecker packet_delivery_queue_;
-  Timestamp time_last_packet_delivery_queue_
+  std::vector<Timestamp> time_last_packet_delivery_queue_
       RTC_GUARDED_BY(packet_delivery_queue_);
 
   const TimeDelta min_keyframe_send_interval_;

video/encoder_rtcp_feedback_unittest.cc

@@ -17,34 +17,46 @@
 #include "video/test/mock_video_stream_encoder.h"
 
 using ::testing::_;
+using ::testing::ElementsAre;
 
 namespace webrtc {
 
-class VieKeyRequestTest : public ::testing::Test {
+class VideoEncoderFeedbackKeyframeTestBase : public ::testing::Test {
  public:
-  VieKeyRequestTest()
+  VideoEncoderFeedbackKeyframeTestBase(bool per_layer_pli_handling,
+                                       std::vector<uint32_t> ssrcs)
       : simulated_clock_(123456789),
         encoder_(),
-        encoder_rtcp_feedback_(
-            &simulated_clock_,
-            std::vector<uint32_t>(1, VieKeyRequestTest::kSsrc),
-            &encoder_,
-            nullptr) {}
+        encoder_rtcp_feedback_(&simulated_clock_,
+                               per_layer_pli_handling,
+                               ssrcs,
+                               &encoder_,
+                               nullptr) {}
 
  protected:
-  const uint32_t kSsrc = 1234;
+  static const uint32_t kSsrc = 1234;
+  static const uint32_t kOtherSsrc = 4321;
 
   SimulatedClock simulated_clock_;
   ::testing::StrictMock<MockVideoStreamEncoder> encoder_;
   EncoderRtcpFeedback encoder_rtcp_feedback_;
 };
 
-TEST_F(VieKeyRequestTest, CreateAndTriggerRequests) {
+class VideoEncoderFeedbackKeyframeTest
+    : public VideoEncoderFeedbackKeyframeTestBase {
+ public:
+  VideoEncoderFeedbackKeyframeTest()
+      : VideoEncoderFeedbackKeyframeTestBase(
+            /*per_layer_pli_handling=*/false,
+            {VideoEncoderFeedbackKeyframeTestBase::kSsrc}) {}
+};
+
+TEST_F(VideoEncoderFeedbackKeyframeTest, CreateAndTriggerRequests) {
   EXPECT_CALL(encoder_, SendKeyFrame(_)).Times(1);
   encoder_rtcp_feedback_.OnReceivedIntraFrameRequest(kSsrc);
 }
 
-TEST_F(VieKeyRequestTest, TooManyOnReceivedIntraFrameRequest) {
+TEST_F(VideoEncoderFeedbackKeyframeTest, TooManyOnReceivedIntraFrameRequest) {
   EXPECT_CALL(encoder_, SendKeyFrame(_)).Times(1);
   encoder_rtcp_feedback_.OnReceivedIntraFrameRequest(kSsrc);
   encoder_rtcp_feedback_.OnReceivedIntraFrameRequest(kSsrc);
@@ -58,4 +70,61 @@ TEST_F(VieKeyRequestTest, TooManyOnReceivedIntraFrameRequest) {
   encoder_rtcp_feedback_.OnReceivedIntraFrameRequest(kSsrc);
 }
 
+class VideoEncoderFeedbackKeyframePerLayerPliTest
+    : public VideoEncoderFeedbackKeyframeTestBase {
+ public:
+  VideoEncoderFeedbackKeyframePerLayerPliTest()
+      : VideoEncoderFeedbackKeyframeTestBase(
+            /*per_layer_pli_handling=*/true,
+            {VideoEncoderFeedbackKeyframeTestBase::kSsrc,
+             VideoEncoderFeedbackKeyframeTestBase::kOtherSsrc}) {}
+};
+
+TEST_F(VideoEncoderFeedbackKeyframePerLayerPliTest, CreateAndTriggerRequests) {
+  EXPECT_CALL(encoder_,
+              SendKeyFrame(ElementsAre(VideoFrameType::kVideoFrameKey,
+                                       VideoFrameType::kVideoFrameDelta)))
+      .Times(1);
+  EXPECT_CALL(encoder_,
+              SendKeyFrame(ElementsAre(VideoFrameType::kVideoFrameDelta,
+                                       VideoFrameType::kVideoFrameKey)))
+      .Times(1);
+  encoder_rtcp_feedback_.OnReceivedIntraFrameRequest(kSsrc);
+  encoder_rtcp_feedback_.OnReceivedIntraFrameRequest(kOtherSsrc);
+}
+
+TEST_F(VideoEncoderFeedbackKeyframePerLayerPliTest,
+       TooManyOnReceivedIntraFrameRequest) {
+  EXPECT_CALL(encoder_,
+              SendKeyFrame(ElementsAre(VideoFrameType::kVideoFrameKey,
+                                       VideoFrameType::kVideoFrameDelta)))
+      .Times(1);
+  EXPECT_CALL(encoder_,
+              SendKeyFrame(ElementsAre(VideoFrameType::kVideoFrameDelta,
+                                       VideoFrameType::kVideoFrameKey)))
+      .Times(1);
+  encoder_rtcp_feedback_.OnReceivedIntraFrameRequest(kSsrc);
+  encoder_rtcp_feedback_.OnReceivedIntraFrameRequest(kSsrc);
+  encoder_rtcp_feedback_.OnReceivedIntraFrameRequest(kOtherSsrc);
+  simulated_clock_.AdvanceTimeMilliseconds(10);
+  encoder_rtcp_feedback_.OnReceivedIntraFrameRequest(kSsrc);
+  encoder_rtcp_feedback_.OnReceivedIntraFrameRequest(kOtherSsrc);
+
+  EXPECT_CALL(encoder_,
+              SendKeyFrame(ElementsAre(VideoFrameType::kVideoFrameKey,
+                                       VideoFrameType::kVideoFrameDelta)))
+      .Times(1);
+  EXPECT_CALL(encoder_,
+              SendKeyFrame(ElementsAre(VideoFrameType::kVideoFrameDelta,
+                                       VideoFrameType::kVideoFrameKey)))
+      .Times(1);
+  simulated_clock_.AdvanceTimeMilliseconds(300);
+  encoder_rtcp_feedback_.OnReceivedIntraFrameRequest(kSsrc);
+  encoder_rtcp_feedback_.OnReceivedIntraFrameRequest(kSsrc);
+  encoder_rtcp_feedback_.OnReceivedIntraFrameRequest(kSsrc);
+  encoder_rtcp_feedback_.OnReceivedIntraFrameRequest(kOtherSsrc);
+  encoder_rtcp_feedback_.OnReceivedIntraFrameRequest(kOtherSsrc);
+  encoder_rtcp_feedback_.OnReceivedIntraFrameRequest(kOtherSsrc);
+}
+
 }  // namespace webrtc

video/video_send_stream_impl.cc

@@ -52,6 +52,8 @@
 #include "call/rtp_config.h"
 #include "call/rtp_transport_controller_send_interface.h"
 #include "call/video_send_stream.h"
+#include "media/base/media_constants.h"
+#include "media/base/sdp_video_format_utils.h"
 #include "modules/pacing/pacing_controller.h"
 #include "modules/rtp_rtcp/include/rtp_header_extension_map.h"
 #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
@@ -410,6 +412,8 @@ VideoSendStreamImpl::VideoSendStreamImpl(
                     config_.encoder_selector)),
       encoder_feedback_(
           clock,
+          SupportsPerLayerPictureLossIndication(
+              encoder_config.video_format.parameters),
           config_.rtp.ssrcs,
           video_stream_encoder_.get(),
           [this](uint32_t ssrc, const std::vector<uint16_t>& seq_nums) {