Revert "Delete RtpPayloadParams::SetDependenciesVp8Deprecated as unused"

This reverts commit dc7fe40f497179721e53af1b3ece37c741bb757e.

Reason for revert: speculative revert for breaking downstream projects

Original change's description:
> Delete RtpPayloadParams::SetDependenciesVp8Deprecated as unused
> 
> Bug: webrtc:10242
> Change-Id: Iddad086d8ce3652bd9f0fb12788d5c73b5ebda76
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/161945
> Reviewed-by: Niels Moller <nisse@webrtc.org>
> Reviewed-by: Philip Eliasson <philipel@webrtc.org>
> Reviewed-by: Elad Alon <eladalon@webrtc.org>
> Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#30159}

TBR=danilchap@webrtc.org,eladalon@webrtc.org,nisse@webrtc.org,philipel@webrtc.org

Change-Id: Ie7f875291610a7b676539a5ccc4bac9a08011f42
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:10242
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/165240
Reviewed-by: Qingsi Wang <qingsi@webrtc.org>
Commit-Queue: Qingsi Wang <qingsi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30173}

call/rtp_payload_params.cc

@@ -379,6 +379,71 @@ void RtpPayloadParams::Vp8ToGeneric(const CodecSpecificInfoVP8& vp8_info,
   generic.spatial_index = spatial_index;
   generic.temporal_index = temporal_index;
 
+  if (vp8_info.useExplicitDependencies) {
+    SetDependenciesVp8New(vp8_info, shared_frame_id, is_keyframe,
+                          vp8_header.layerSync, &generic);
+  } else {
+    SetDependenciesVp8Deprecated(vp8_info, shared_frame_id, is_keyframe,
+                                 spatial_index, temporal_index,
+                                 vp8_header.layerSync, &generic);
+  }
+}
+
+void RtpPayloadParams::SetDependenciesVp8Deprecated(
+    const CodecSpecificInfoVP8& vp8_info,
+    int64_t shared_frame_id,
+    bool is_keyframe,
+    int spatial_index,
+    int temporal_index,
+    bool layer_sync,
+    RTPVideoHeader::GenericDescriptorInfo* generic) {
+  RTC_DCHECK(!vp8_info.useExplicitDependencies);
+  RTC_DCHECK(!new_version_used_.has_value() || !new_version_used_.value());
+  new_version_used_ = false;
+
+  if (is_keyframe) {
+    RTC_DCHECK_EQ(temporal_index, 0);
+    last_shared_frame_id_[spatial_index].fill(-1);
+    last_shared_frame_id_[spatial_index][temporal_index] = shared_frame_id;
+    return;
+  }
+
+  if (layer_sync) {
+    int64_t tl0_frame_id = last_shared_frame_id_[spatial_index][0];
+
+    for (int i = 1; i < RtpGenericFrameDescriptor::kMaxTemporalLayers; ++i) {
+      if (last_shared_frame_id_[spatial_index][i] < tl0_frame_id) {
+        last_shared_frame_id_[spatial_index][i] = -1;
+      }
+    }
+
+    RTC_DCHECK_GE(tl0_frame_id, 0);
+    RTC_DCHECK_LT(tl0_frame_id, shared_frame_id);
+    generic->dependencies.push_back(tl0_frame_id);
+  } else {
+    for (int i = 0; i <= temporal_index; ++i) {
+      int64_t frame_id = last_shared_frame_id_[spatial_index][i];
+
+      if (frame_id != -1) {
+        RTC_DCHECK_LT(frame_id, shared_frame_id);
+        generic->dependencies.push_back(frame_id);
+      }
+    }
+  }
+
+  last_shared_frame_id_[spatial_index][temporal_index] = shared_frame_id;
+}
+
+void RtpPayloadParams::SetDependenciesVp8New(
+    const CodecSpecificInfoVP8& vp8_info,
+    int64_t shared_frame_id,
+    bool is_keyframe,
+    bool layer_sync,
+    RTPVideoHeader::GenericDescriptorInfo* generic) {
+  RTC_DCHECK(vp8_info.useExplicitDependencies);
+  RTC_DCHECK(!new_version_used_.has_value() || new_version_used_.value());
+  new_version_used_ = true;
+
   if (is_keyframe) {
     RTC_DCHECK_EQ(vp8_info.referencedBuffersCount, 0u);
     buffer_id_to_frame_id_.fill(shared_frame_id);
@@ -402,10 +467,10 @@ void RtpPayloadParams::Vp8ToGeneric(const CodecSpecificInfoVP8& vp8_info,
     RTC_DCHECK_LT(dependency_frame_id, shared_frame_id);
 
     const bool is_new_dependency =
-        std::find(generic.dependencies.begin(), generic.dependencies.end(),
+        std::find(generic->dependencies.begin(), generic->dependencies.end(),
-                  dependency_frame_id) == generic.dependencies.end();
+                  dependency_frame_id) == generic->dependencies.end();
     if (is_new_dependency) {
-      generic.dependencies.push_back(dependency_frame_id);
+      generic->dependencies.push_back(dependency_frame_id);
     }
   }
 

call/rtp_payload_params.h

@@ -62,6 +62,23 @@ class RtpPayloadParams final {
                         bool is_keyframe,
                         RTPVideoHeader* rtp_video_header);
 
+  // TODO(bugs.webrtc.org/10242): Delete SetDependenciesVp8Deprecated() and move
+  // the logic in SetDependenciesVp8New() into Vp8ToGeneric() once all hardware
+  // wrappers have been updated.
+  void SetDependenciesVp8Deprecated(
+      const CodecSpecificInfoVP8& vp8_info,
+      int64_t shared_frame_id,
+      bool is_keyframe,
+      int spatial_index,
+      int temporal_index,
+      bool layer_sync,
+      RTPVideoHeader::GenericDescriptorInfo* generic);
+  void SetDependenciesVp8New(const CodecSpecificInfoVP8& vp8_info,
+                             int64_t shared_frame_id,
+                             bool is_keyframe,
+                             bool layer_sync,
+                             RTPVideoHeader::GenericDescriptorInfo* generic);
+
   // TODO(bugs.webrtc.org/10242): Remove once all encoder-wrappers are updated.
   // Holds the last shared frame id for a given (spatial, temporal) layer.
   std::array<std::array<int64_t, RtpGenericFrameDescriptor::kMaxTemporalLayers>,
@@ -76,6 +93,12 @@ class RtpPayloadParams final {
   // Maps buffer IDs to the frame-ID stored in them.
   std::array<int64_t, kMaxCodecBuffersCount> buffer_id_to_frame_id_;
 
+  // Until we remove SetDependenciesVp8Deprecated(), we should make sure
+  // that, for a given object, we either always use
+  // SetDependenciesVp8Deprecated(), or always use SetDependenciesVp8New().
+  // TODO(bugs.webrtc.org/10242): Remove.
+  absl::optional<bool> new_version_used_;
+
   const uint32_t ssrc_;
   RtpPayloadState state_;
 

call/rtp_payload_params_unittest.cc

@@ -14,7 +14,6 @@
 
 #include <map>
 #include <set>
-#include <vector>
 
 #include "absl/container/inlined_vector.h"
 #include "absl/types/optional.h"
@@ -30,13 +29,11 @@
 #include "test/gmock.h"
 #include "test/gtest.h"
 
-namespace webrtc {
-namespace {
-
 using ::testing::ElementsAre;
 using ::testing::IsEmpty;
-using ::testing::UnorderedElementsAreArray;
 
+namespace webrtc {
+namespace {
 const uint32_t kSsrc1 = 12345;
 const uint32_t kSsrc2 = 23456;
 const int16_t kPictureId = 123;
@@ -381,32 +378,20 @@ class RtpPayloadParamsVp8ToGenericTest : public ::testing::Test {
 
   void ConvertAndCheck(int temporal_index,
                        int64_t shared_frame_id,
-                       const std::vector<int>& references,
+                       VideoFrameType frame_type,
-                       const std::vector<int>& updates,
                        LayerSync layer_sync,
                        const std::set<int64_t>& expected_deps,
                        uint16_t width = 0,
                        uint16_t height = 0) {
     EncodedImage encoded_image;
-    encoded_image._frameType = references.empty()
+    encoded_image._frameType = frame_type;
-                                   ? VideoFrameType::kVideoFrameKey
-                                   : VideoFrameType::kVideoFrameDelta;
     encoded_image._encodedWidth = width;
     encoded_image._encodedHeight = height;
 
     CodecSpecificInfo codec_info;
     codec_info.codecType = kVideoCodecVP8;
-    auto& vp8 = codec_info.codecSpecific.VP8;
+    codec_info.codecSpecific.VP8.temporalIdx = temporal_index;
-    vp8.temporalIdx = temporal_index;
+    codec_info.codecSpecific.VP8.layerSync = layer_sync == kSync;
-    vp8.layerSync = layer_sync == kSync;
-    vp8.referencedBuffersCount = 0;
-    for (int reference : references) {
-      vp8.referencedBuffers[vp8.referencedBuffersCount++] = reference;
-    }
-    vp8.updatedBuffersCount = 0;
-    for (int update : updates) {
-      vp8.updatedBuffers[vp8.updatedBuffersCount++] = update;
-    }
 
     RTPVideoHeader header =
         params_.GetRtpVideoHeader(encoded_image, &codec_info, shared_frame_id);
@@ -417,8 +402,9 @@ class RtpPayloadParamsVp8ToGenericTest : public ::testing::Test {
 
     EXPECT_EQ(header.generic->frame_id, shared_frame_id);
     EXPECT_EQ(header.generic->temporal_index, temporal_index);
-    EXPECT_THAT(header.generic->dependencies,
+    std::set<int64_t> actual_deps(header.generic->dependencies.begin(),
-                UnorderedElementsAreArray(expected_deps));
+                                  header.generic->dependencies.end());
+    EXPECT_EQ(expected_deps, actual_deps);
 
     EXPECT_EQ(header.width, width);
     EXPECT_EQ(header.height, height);
@@ -431,16 +417,13 @@ class RtpPayloadParamsVp8ToGenericTest : public ::testing::Test {
 };
 
 TEST_F(RtpPayloadParamsVp8ToGenericTest, Keyframe) {
-  ConvertAndCheck(0, 0, /*references=*/{}, /*updates=*/{0}, kNoSync, {}, 480,
+  ConvertAndCheck(0, 0, VideoFrameType::kVideoFrameKey, kNoSync, {}, 480, 360);
-                  360);
+  ConvertAndCheck(0, 1, VideoFrameType::kVideoFrameDelta, kNoSync, {0});
-  ConvertAndCheck(0, 1, /*references=*/{0}, /*updates=*/{0}, kNoSync, {0});
+  ConvertAndCheck(0, 2, VideoFrameType::kVideoFrameKey, kNoSync, {}, 480, 360);
-  ConvertAndCheck(0, 2, /*references=*/{}, /*updates=*/{0}, kNoSync, {}, 480,
-                  360);
 }
 
 TEST_F(RtpPayloadParamsVp8ToGenericTest, TooHighTemporalIndex) {
-  ConvertAndCheck(0, 0, /*references=*/{}, /*updates=*/{0}, kNoSync, {}, 480,
+  ConvertAndCheck(0, 0, VideoFrameType::kVideoFrameKey, kNoSync, {}, 480, 360);
-                  360);
 
   EncodedImage encoded_image;
   encoded_image._frameType = VideoFrameType::kVideoFrameDelta;
@@ -455,35 +438,30 @@ TEST_F(RtpPayloadParamsVp8ToGenericTest, TooHighTemporalIndex) {
   EXPECT_FALSE(header.generic);
 }
 
-TEST_F(RtpPayloadParamsVp8ToGenericTest, Pattern02120212) {
+TEST_F(RtpPayloadParamsVp8ToGenericTest, LayerSync) {
-  ConvertAndCheck(0, 0, /*references=*/{}, /*updates=*/{0}, kNoSync, {}, 480,
+  // 02120212 pattern
-                  360);
+  ConvertAndCheck(0, 0, VideoFrameType::kVideoFrameKey, kNoSync, {}, 480, 360);
-  ConvertAndCheck(2, 1, /*references=*/{0}, /*updates=*/{2}, kNoSync, {0});
+  ConvertAndCheck(2, 1, VideoFrameType::kVideoFrameDelta, kNoSync, {0});
-  ConvertAndCheck(1, 2, /*references=*/{0}, /*updates=*/{1}, kNoSync, {0});
+  ConvertAndCheck(1, 2, VideoFrameType::kVideoFrameDelta, kNoSync, {0});
-  ConvertAndCheck(2, 3, /*references=*/{0, 1, 2}, /*updates=*/{2}, kNoSync,
+  ConvertAndCheck(2, 3, VideoFrameType::kVideoFrameDelta, kNoSync, {0, 1, 2});
-                  {0, 1, 2});
 
-  ConvertAndCheck(0, 4, /*references=*/{0}, /*updates=*/{0}, kNoSync, {0});
+  ConvertAndCheck(0, 4, VideoFrameType::kVideoFrameDelta, kNoSync, {0});
-  ConvertAndCheck(2, 5, /*references=*/{0, 1, 2}, /*updates=*/{2}, kNoSync,
+  ConvertAndCheck(2, 5, VideoFrameType::kVideoFrameDelta, kNoSync, {2, 3, 4});
-                  {2, 3, 4});
+  ConvertAndCheck(1, 6, VideoFrameType::kVideoFrameDelta, kSync,
-  ConvertAndCheck(1, 6, /*references=*/{0}, /*updates=*/{1}, kSync, {4});
+                  {4});  // layer sync
-  ConvertAndCheck(2, 7, /*references=*/{0, 1, 2}, /*updates=*/{2}, kNoSync,
+  ConvertAndCheck(2, 7, VideoFrameType::kVideoFrameDelta, kNoSync, {4, 5, 6});
-                  {4, 5, 6});
 }
 
 TEST_F(RtpPayloadParamsVp8ToGenericTest, FrameIdGaps) {
   // 0101 pattern
-  ConvertAndCheck(0, 0, /*references=*/{}, /*updates=*/{0}, kNoSync, {}, 480,
+  ConvertAndCheck(0, 0, VideoFrameType::kVideoFrameKey, kNoSync, {}, 480, 360);
-                  360);
+  ConvertAndCheck(1, 1, VideoFrameType::kVideoFrameDelta, kNoSync, {0});
-  ConvertAndCheck(1, 1, /*references=*/{0}, /*updates=*/{1}, kNoSync, {0});
 
-  ConvertAndCheck(0, 5, /*references=*/{0}, /*updates=*/{0}, kNoSync, {0});
+  ConvertAndCheck(0, 5, VideoFrameType::kVideoFrameDelta, kNoSync, {0});
-  ConvertAndCheck(1, 10, /*references=*/{0, 1}, /*updates=*/{1}, kNoSync,
+  ConvertAndCheck(1, 10, VideoFrameType::kVideoFrameDelta, kNoSync, {1, 5});
-                  {1, 5});
 
-  ConvertAndCheck(0, 15, /*references=*/{0}, /*updates=*/{0}, kNoSync, {5});
+  ConvertAndCheck(0, 15, VideoFrameType::kVideoFrameDelta, kNoSync, {5});
-  ConvertAndCheck(1, 20, /*references=*/{0, 1}, /*updates=*/{1}, kNoSync,
+  ConvertAndCheck(1, 20, VideoFrameType::kVideoFrameDelta, kNoSync, {10, 15});
-                  {10, 15});
 }
 
 class RtpPayloadParamsH264ToGenericTest : public ::testing::Test {

modules/video_coding/BUILD.gn

@@ -182,7 +182,6 @@ rtc_library("video_codec_interface") {
     "../../api/video_codecs:video_codecs_api",
     "../../common_video",
     "../../common_video/generic_frame_descriptor",
-    "../../rtc_base:deprecation",
     "../../rtc_base/system:rtc_export",
     "//third_party/abseil-cpp/absl/types:optional",
   ]

modules/video_coding/codecs/vp8/default_temporal_layers.cc

@@ -520,6 +520,7 @@ void DefaultTemporalLayers::OnEncodeDone(size_t stream_index,
     }
   }
 
+  vp8_info.useExplicitDependencies = true;
   RTC_DCHECK_EQ(vp8_info.referencedBuffersCount, 0u);
   RTC_DCHECK_EQ(vp8_info.updatedBuffersCount, 0u);
 

modules/video_coding/codecs/vp8/screenshare_layers.cc

@@ -351,6 +351,7 @@ void ScreenshareLayers::OnEncodeDone(size_t stream_index,
       layers_[active_layer_].state = TemporalLayer::State::kNormal;
     }
 
+    vp8_info.useExplicitDependencies = true;
     RTC_DCHECK_EQ(vp8_info.referencedBuffersCount, 0u);
     RTC_DCHECK_EQ(vp8_info.updatedBuffersCount, 0u);
 

modules/video_coding/include/video_codec_interface.h

@@ -22,7 +22,6 @@
 #include "modules/video_coding/codecs/h264/include/h264_globals.h"
 #include "modules/video_coding/codecs/vp9/include/vp9_globals.h"
 #include "modules/video_coding/include/video_error_codes.h"
-#include "rtc_base/deprecation.h"
 #include "rtc_base/system/rtc_export.h"
 
 namespace webrtc {
@@ -42,9 +41,9 @@ struct CodecSpecificInfoVP8 {
   // codec buffers, but the exact mapping (i.e. whether 0 refers to Last,
   // to Golden or to Arf) is not pre-determined.
   // More references may be specified than are strictly necessary, but not less.
-  // TODO(bugs.webrtc.org/10242): Remove |useExplicitDependencies| when not set
+  // TODO(bugs.webrtc.org/10242): Remove |useExplicitDependencies| once all
-  // by downstream projects.
+  // encoder-wrappers are updated.
-  RTC_DEPRECATED bool useExplicitDependencies;
+  bool useExplicitDependencies;
   static constexpr size_t kBuffersCount = 3;
   size_t referencedBuffers[kBuffersCount];
   size_t referencedBuffersCount;