Revert "Do not propagate generic descriptor on receiving frame"

This reverts commit abf73de8eae90e9ac7e88ce1d52728e8102e824f.

Reason for revert: breaks downstream tests

Original change's description:
> Do not propagate generic descriptor on receiving frame
> 
> It was used only for the frame decryptor.
> Decryptor needs only raw representation that it can recreate
> in a way compatible with the new version of the descriptor.
> 
> Bug: webrtc:10342
> Change-Id: Ie098235ebb87c6f5e2af42d0022d2365cd6bfa29
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/166163
> Reviewed-by: Philip Eliasson <philipel@webrtc.org>
> Reviewed-by: Erik Språng <sprang@webrtc.org>
> Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#30501}

TBR=danilchap@webrtc.org,sprang@webrtc.org,philipel@webrtc.org

Change-Id: I6634df06ee75aa8cdfda614994ab11f7a5845c70
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:10342
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/168488
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30502}

modules/rtp_rtcp/BUILD.gn

@@ -165,8 +165,6 @@ rtc_library("rtp_rtcp") {
     "source/rtcp_receiver.h",
     "source/rtcp_sender.cc",
     "source/rtcp_sender.h",
-    "source/rtp_descriptor_authentication.cc",
-    "source/rtp_descriptor_authentication.h",
     "source/rtp_format.cc",
     "source/rtp_format.h",
     "source/rtp_format_h264.cc",

modules/rtp_rtcp/source/rtp_descriptor_authentication.cc

@@ -1,49 +0,0 @@
-/*
- *  Copyright (c) 2020 The WebRTC project authors. All Rights Reserved.
- *
- *  Use of this source code is governed by a BSD-style license
- *  that can be found in the LICENSE file in the root of the source
- *  tree. An additional intellectual property rights grant can be found
- *  in the file PATENTS.  All contributing project authors may
- *  be found in the AUTHORS file in the root of the source tree.
- */
-
-#include "modules/rtp_rtcp/source/rtp_descriptor_authentication.h"
-
-#include <cstdint>
-#include <vector>
-
-#include "modules/rtp_rtcp/source/rtp_generic_frame_descriptor.h"
-#include "modules/rtp_rtcp/source/rtp_generic_frame_descriptor_extension.h"
-#include "modules/rtp_rtcp/source/rtp_video_header.h"
-
-namespace webrtc {
-
-std::vector<uint8_t> RtpDescriptorAuthentication(
-    const RTPVideoHeader::GenericDescriptorInfo& descriptor) {
-  // Default way of creating additional data for an encrypted frame.
-  if (descriptor.spatial_index < 0 || descriptor.temporal_index < 0 ||
-      descriptor.spatial_index >=
-          RtpGenericFrameDescriptor::kMaxSpatialLayers ||
-      descriptor.temporal_index >=
-          RtpGenericFrameDescriptor::kMaxTemporalLayers ||
-      descriptor.dependencies.size() >
-          RtpGenericFrameDescriptor::kMaxNumFrameDependencies) {
-    return {};
-  }
-  RtpGenericFrameDescriptor frame_descriptor;
-  frame_descriptor.SetFirstPacketInSubFrame(true);
-  frame_descriptor.SetLastPacketInSubFrame(false);
-  frame_descriptor.SetTemporalLayer(descriptor.temporal_index);
-  frame_descriptor.SetSpatialLayersBitmask(1 << descriptor.spatial_index);
-  frame_descriptor.SetFrameId(descriptor.frame_id & 0xFFFF);
-  for (int64_t dependency : descriptor.dependencies) {
-    frame_descriptor.AddFrameDependencyDiff(descriptor.frame_id - dependency);
-  }
-  std::vector<uint8_t> result(
-      RtpGenericFrameDescriptorExtension00::ValueSize(frame_descriptor));
-  RtpGenericFrameDescriptorExtension00::Write(result, frame_descriptor);
-  return result;
-}
-
-}  // namespace webrtc

modules/rtp_rtcp/source/rtp_descriptor_authentication.h

@@ -1,27 +0,0 @@
-/*
- *  Copyright (c) 2020 The WebRTC project authors. All Rights Reserved.
- *
- *  Use of this source code is governed by a BSD-style license
- *  that can be found in the LICENSE file in the root of the source
- *  tree. An additional intellectual property rights grant can be found
- *  in the file PATENTS.  All contributing project authors may
- *  be found in the AUTHORS file in the root of the source tree.
- */
-
-#ifndef MODULES_RTP_RTCP_SOURCE_RTP_DESCRIPTOR_AUTHENTICATION_H_
-#define MODULES_RTP_RTCP_SOURCE_RTP_DESCRIPTOR_AUTHENTICATION_H_
-
-#include <cstdint>
-#include <vector>
-
-#include "modules/rtp_rtcp/source/rtp_video_header.h"
-
-namespace webrtc {
-
-// Converts frame dependencies into array of bytes for authentication.
-std::vector<uint8_t> RtpDescriptorAuthentication(
-    const RTPVideoHeader::GenericDescriptorInfo& descriptor);
-
-}  // namespace webrtc
-
-#endif  // MODULES_RTP_RTCP_SOURCE_RTP_DESCRIPTOR_AUTHENTICATION_H_

modules/rtp_rtcp/source/rtp_generic_frame_descriptor.cc

@@ -97,4 +97,22 @@ bool RtpGenericFrameDescriptor::AddFrameDependencyDiff(uint16_t fdiff) {
   return true;
 }
 
+void RtpGenericFrameDescriptor::SetByteRepresentation(
+    rtc::ArrayView<const uint8_t> byte_representation) {
+  RTC_CHECK(!byte_representation.empty());
+  byte_representation_.assign(byte_representation.begin(),
+                              byte_representation.end());
+  // Clear end_of_subframe bit.
+  // Because ByteRepresentation is used for frame authentication, bit describing
+  // position of the packet in the frame shouldn't be part of it.
+  // This match RtpVideoSender where descriptor is passed for authentication
+  // before end_of_subframe bit is decided and set, i.e. it is always 0.
+  byte_representation_[0] &= ~0x40;
+}
+
+rtc::ArrayView<const uint8_t>
+RtpGenericFrameDescriptor::GetByteRepresentation() {
+  return byte_representation_;
+}
+
 }  // namespace webrtc

modules/rtp_rtcp/source/rtp_generic_frame_descriptor.h

@@ -70,6 +70,9 @@ class RtpGenericFrameDescriptor {
   // Returns false on failure, i.e. number of dependencies is too large.
   bool AddFrameDependencyDiff(uint16_t fdiff);
 
+  void SetByteRepresentation(rtc::ArrayView<const uint8_t> representation);
+  rtc::ArrayView<const uint8_t> GetByteRepresentation();
+
  private:
   bool beginning_of_subframe_ = false;
   bool end_of_subframe_ = false;
@@ -83,6 +86,8 @@ class RtpGenericFrameDescriptor {
   uint16_t frame_deps_id_diffs_[kMaxNumFrameDependencies];
   int width_ = 0;
   int height_ = 0;
+
+  std::vector<uint8_t> byte_representation_;
 };
 
 }  // namespace webrtc

modules/video_coding/frame_object.cc

@@ -37,6 +37,7 @@ RtpFrameObject::RtpFrameObject(
     VideoContentType content_type,
     const RTPVideoHeader& video_header,
     const absl::optional<webrtc::ColorSpace>& color_space,
+    const absl::optional<RtpGenericFrameDescriptor>& generic_descriptor,
     RtpPacketInfos packet_infos,
     rtc::scoped_refptr<EncodedImageBuffer> image_buffer)
     : first_seq_num_(first_seq_num),
@@ -44,6 +45,7 @@ RtpFrameObject::RtpFrameObject(
       last_packet_received_time_(last_packet_received_time),
       times_nacked_(times_nacked) {
   rtp_video_header_ = video_header;
+  rtp_generic_frame_descriptor_ = generic_descriptor;
 
   // EncodedFrame members
   codec_type_ = codec;
@@ -129,6 +131,11 @@ const RTPVideoHeader& RtpFrameObject::GetRtpVideoHeader() const {
   return rtp_video_header_;
 }
 
+const absl::optional<RtpGenericFrameDescriptor>&
+RtpFrameObject::GetGenericFrameDescriptor() const {
+  return rtp_generic_frame_descriptor_;
+}
+
 const FrameMarking& RtpFrameObject::GetFrameMarking() const {
   return rtp_video_header_.frame_marking;
 }

modules/video_coding/frame_object.h

@@ -13,13 +13,15 @@
 
 #include "absl/types/optional.h"
 #include "api/video/encoded_frame.h"
+#include "modules/rtp_rtcp/source/rtp_generic_frame_descriptor.h"
 
 namespace webrtc {
 namespace video_coding {
 
 class RtpFrameObject : public EncodedFrame {
  public:
-  RtpFrameObject(uint16_t first_seq_num,
+  RtpFrameObject(
+      uint16_t first_seq_num,
       uint16_t last_seq_num,
       bool markerBit,
       int times_nacked,
@@ -34,6 +36,7 @@ class RtpFrameObject : public EncodedFrame {
       VideoContentType content_type,
       const RTPVideoHeader& video_header,
       const absl::optional<webrtc::ColorSpace>& color_space,
+      const absl::optional<RtpGenericFrameDescriptor>& generic_descriptor,
       RtpPacketInfos packet_infos,
       rtc::scoped_refptr<EncodedImageBuffer> image_buffer);
 
@@ -47,10 +50,13 @@ class RtpFrameObject : public EncodedFrame {
   int64_t RenderTime() const override;
   bool delayed_by_retransmission() const override;
   const RTPVideoHeader& GetRtpVideoHeader() const;
+  const absl::optional<RtpGenericFrameDescriptor>& GetGenericFrameDescriptor()
+      const;
   const FrameMarking& GetFrameMarking() const;
 
  private:
   RTPVideoHeader rtp_video_header_;
+  absl::optional<RtpGenericFrameDescriptor> rtp_generic_frame_descriptor_;
   VideoCodecType codec_type_;
   uint16_t first_seq_num_;
   uint16_t last_seq_num_;

modules/video_coding/packet_buffer.cc

@@ -473,6 +473,7 @@ std::unique_ptr<RtpFrameObject> PacketBuffer::AssembleFrame(
       last_packet.video_header.content_type,    //
       first_packet.video_header,                //
       last_packet.video_header.color_space,     //
+      first_packet.generic_descriptor,          //
       RtpPacketInfos(std::move(packet_infos)),  //
       std::move(bitstream));
 }

modules/video_coding/packet_buffer.h

@@ -19,6 +19,7 @@
 #include "absl/base/attributes.h"
 #include "api/rtp_packet_info.h"
 #include "api/video/encoded_image.h"
+#include "modules/rtp_rtcp/source/rtp_generic_frame_descriptor.h"
 #include "modules/rtp_rtcp/source/rtp_packet_received.h"
 #include "modules/rtp_rtcp/source/rtp_video_header.h"
 #include "modules/video_coding/frame_object.h"
@@ -66,6 +67,7 @@ class PacketBuffer {
 
     rtc::CopyOnWriteBuffer video_payload;
     RTPVideoHeader video_header;
+    absl::optional<RtpGenericFrameDescriptor> generic_descriptor;
 
     RtpPacketInfo packet_info;
   };

modules/video_coding/rtp_frame_reference_finder_unittest.cc

@@ -57,6 +57,7 @@ std::unique_ptr<RtpFrameObject> CreateFrame(
       VideoContentType::UNSPECIFIED,
       video_header,
       /*color_space=*/absl::nullopt,
+      /*generic_descriptor=*/absl::nullopt,
       RtpPacketInfos(),
       EncodedImageBuffer::Create(/*size=*/0));
   // clang-format on

test/fuzzers/rtp_frame_reference_finder_fuzzer.cc

@@ -63,31 +63,25 @@ class NullCallback : public video_coding::OnCompleteFrameCallback {
       std::unique_ptr<video_coding::EncodedFrame> frame) override {}
 };
 
-absl::optional<RTPVideoHeader::GenericDescriptorInfo>
+RtpGenericFrameDescriptor GenerateRtpGenericFrameDescriptor(
-GenerateGenericFrameDependencies(DataReader* reader) {
+    DataReader* reader) {
-  absl::optional<RTPVideoHeader::GenericDescriptorInfo> result;
+  RtpGenericFrameDescriptor res;
-  uint8_t flags = reader->GetNum<uint8_t>();
+  res.SetFirstPacketInSubFrame(true);
-  if (flags & 0b1000'0000) {
+  res.SetFrameId(reader->GetNum<uint16_t>());
-    // i.e. with 50% chance there are no generic dependencies.
-    // in such case codec-specfic code path of the RtpFrameReferenceFinder will
-    // be validated.
-    return result;
-  }
 
-  result.emplace();
+  int spatial_layer =
-  result->frame_id = reader->GetNum<int64_t>();
+      reader->GetNum<uint8_t>() % RtpGenericFrameDescriptor::kMaxSpatialLayers;
-  result->spatial_index = (flags & 0b0111'0000) >> 4;
+  res.SetSpatialLayersBitmask(1 << spatial_layer);
-  result->temporal_index = (flags & 0b0000'1110) >> 1;
+  res.SetTemporalLayer(reader->GetNum<uint8_t>() %
-  result->discardable = (flags & 0b0000'0001);
+                       RtpGenericFrameDescriptor::kMaxTemporalLayers);
 
-  // Larger than supported by the RtpFrameReferenceFinder.
+  int num_diffs = (reader->GetNum<uint8_t>() %
-  int num_diffs = (reader->GetNum<uint8_t>() % 16);
+                   RtpGenericFrameDescriptor::kMaxNumFrameDependencies);
   for (int i = 0; i < num_diffs; ++i) {
-    result->dependencies.push_back(result->frame_id -
+    res.AddFrameDependencyDiff(reader->GetNum<uint16_t>() % (1 << 14));
-                                   (reader->GetNum<uint16_t>() % (1 << 14)));
   }
 
-  return result;
+  return res;
 }
 }  // namespace
 
@@ -96,7 +90,7 @@ void FuzzOneInput(const uint8_t* data, size_t size) {
   NullCallback cb;
   video_coding::RtpFrameReferenceFinder reference_finder(&cb);
 
-  auto codec = static_cast<VideoCodecType>(reader.GetNum<uint8_t>() % 5);
+  auto codec = static_cast<VideoCodecType>(reader.GetNum<uint8_t>() % 4);
 
   while (reader.MoreToRead()) {
     uint16_t first_seq_num = reader.GetNum<uint16_t>();
@@ -134,7 +128,6 @@ void FuzzOneInput(const uint8_t* data, size_t size) {
     }
 
     reader.CopyTo(&video_header.frame_marking);
-    video_header.generic = GenerateGenericFrameDependencies(&reader);
 
     // clang-format off
     auto frame = std::make_unique<video_coding::RtpFrameObject>(
@@ -153,6 +146,7 @@ void FuzzOneInput(const uint8_t* data, size_t size) {
         VideoContentType::UNSPECIFIED,
         video_header,
         /*color_space=*/absl::nullopt,
+        GenerateRtpGenericFrameDescriptor(&reader),
         RtpPacketInfos(),
         EncodedImageBuffer::Create(/*size=*/0));
     // clang-format on

video/buffered_frame_decryptor.cc

@@ -11,10 +11,7 @@
 #include "video/buffered_frame_decryptor.h"
 
 #include <utility>
-#include <vector>
 
-#include "modules/rtp_rtcp/source/rtp_descriptor_authentication.h"
-#include "modules/video_coding/frame_object.h"
 #include "rtc_base/logging.h"
 #include "system_wrappers/include/field_trial.h"
 
@@ -63,8 +60,8 @@ BufferedFrameDecryptor::FrameDecision BufferedFrameDecryptor::DecryptFrame(
     return FrameDecision::kStash;
   }
   // When using encryption we expect the frame to have the generic descriptor.
-  const absl::optional<RTPVideoHeader::GenericDescriptorInfo>& descriptor =
+  absl::optional<RtpGenericFrameDescriptor> descriptor =
-      frame->GetRtpVideoHeader().generic;
+      frame->GetGenericFrameDescriptor();
   if (!descriptor) {
     RTC_LOG(LS_ERROR) << "No generic frame descriptor found dropping frame.";
     return FrameDecision::kDrop;
@@ -79,9 +76,9 @@ BufferedFrameDecryptor::FrameDecision BufferedFrameDecryptor::DecryptFrame(
                                                      max_plaintext_byte_size);
 
   // Only enable authenticating the header if the field trial is enabled.
-  std::vector<uint8_t> additional_data;
+  rtc::ArrayView<const uint8_t> additional_data;
   if (generic_descriptor_auth_experiment_) {
-    additional_data = RtpDescriptorAuthentication(*descriptor);
+    additional_data = descriptor->GetByteRepresentation();
   }
 
   // Attempt to decrypt the video frame.

video/buffered_frame_decryptor_unittest.cc

@@ -57,8 +57,6 @@ class BufferedFrameDecryptorTest : public ::testing::Test,
   std::unique_ptr<video_coding::RtpFrameObject> CreateRtpFrameObject(
       bool key_frame) {
     seq_num_++;
-    RTPVideoHeader rtp_video_header;
-    rtp_video_header.generic.emplace();
 
     // clang-format off
     return std::make_unique<video_coding::RtpFrameObject>(
@@ -75,8 +73,9 @@ class BufferedFrameDecryptorTest : public ::testing::Test,
         kVideoCodecGeneric,
         kVideoRotation_0,
         VideoContentType::UNSPECIFIED,
-        rtp_video_header,
+        RTPVideoHeader(),
         /*color_space=*/absl::nullopt,
+        RtpGenericFrameDescriptor(),
         RtpPacketInfos(),
         EncodedImageBuffer::Create(/*size=*/0));
     // clang-format on

video/rtp_video_stream_receiver.cc

@@ -28,7 +28,6 @@
 #include "modules/rtp_rtcp/include/ulpfec_receiver.h"
 #include "modules/rtp_rtcp/source/create_video_rtp_depacketizer.h"
 #include "modules/rtp_rtcp/source/rtp_format.h"
-#include "modules/rtp_rtcp/source/rtp_generic_frame_descriptor.h"
 #include "modules/rtp_rtcp/source/rtp_generic_frame_descriptor_extension.h"
 #include "modules/rtp_rtcp/source/rtp_header_extensions.h"
 #include "modules/rtp_rtcp/source/rtp_packet_received.h"
@@ -368,43 +367,51 @@ void RtpVideoStreamReceiver::OnReceivedPayloadData(
   rtp_packet.GetExtension<PlayoutDelayLimits>(&video_header.playout_delay);
   rtp_packet.GetExtension<FrameMarkingExtension>(&video_header.frame_marking);
 
-  if (rtp_packet.HasExtension<RtpGenericFrameDescriptorExtension00>() &&
+  RtpGenericFrameDescriptor& generic_descriptor =
-      rtp_packet.HasExtension<RtpGenericFrameDescriptorExtension01>()) {
+      packet->generic_descriptor.emplace();
+  if (rtp_packet.GetExtension<RtpGenericFrameDescriptorExtension01>(
+          &generic_descriptor)) {
+    if (rtp_packet.HasExtension<RtpGenericFrameDescriptorExtension00>()) {
       RTC_LOG(LS_WARNING) << "RTP packet had two different GFD versions.";
       return;
     }
-
+    generic_descriptor.SetByteRepresentation(
-  RtpGenericFrameDescriptor generic_descriptor;
+        rtp_packet.GetRawExtension<RtpGenericFrameDescriptorExtension01>());
-  bool has_generic_descriptor =
+  } else if ((rtp_packet.GetExtension<RtpGenericFrameDescriptorExtension00>(
-      rtp_packet.GetExtension<RtpGenericFrameDescriptorExtension01>(
+                 &generic_descriptor))) {
-          &generic_descriptor) ||
+    generic_descriptor.SetByteRepresentation(
-      rtp_packet.GetExtension<RtpGenericFrameDescriptorExtension00>(
+        rtp_packet.GetRawExtension<RtpGenericFrameDescriptorExtension00>());
-          &generic_descriptor);
+  } else {
-  if (has_generic_descriptor) {
+    packet->generic_descriptor = absl::nullopt;
+  }
+  if (packet->generic_descriptor != absl::nullopt) {
     video_header.is_first_packet_in_frame =
-        generic_descriptor.FirstPacketInSubFrame();
+        packet->generic_descriptor->FirstPacketInSubFrame();
     video_header.is_last_packet_in_frame =
-        generic_descriptor.LastPacketInSubFrame();
+        packet->generic_descriptor->LastPacketInSubFrame();
 
-    if (generic_descriptor.FirstPacketInSubFrame()) {
+    if (packet->generic_descriptor->FirstPacketInSubFrame()) {
       video_header.frame_type =
-          generic_descriptor.FrameDependenciesDiffs().empty()
+          packet->generic_descriptor->FrameDependenciesDiffs().empty()
               ? VideoFrameType::kVideoFrameKey
               : VideoFrameType::kVideoFrameDelta;
 
       auto& descriptor = video_header.generic.emplace();
       int64_t frame_id =
-          frame_id_unwrapper_.Unwrap(generic_descriptor.FrameId());
+          frame_id_unwrapper_.Unwrap(packet->generic_descriptor->FrameId());
       descriptor.frame_id = frame_id;
-      descriptor.spatial_index = generic_descriptor.SpatialLayer();
+      descriptor.spatial_index = packet->generic_descriptor->SpatialLayer();
-      descriptor.temporal_index = generic_descriptor.TemporalLayer();
+      descriptor.temporal_index = packet->generic_descriptor->TemporalLayer();
-      descriptor.discardable = generic_descriptor.Discardable().value_or(false);
+      descriptor.discardable =
-      for (uint16_t fdiff : generic_descriptor.FrameDependenciesDiffs()) {
+          packet->generic_descriptor->Discardable().value_or(false);
+      for (uint16_t fdiff :
+           packet->generic_descriptor->FrameDependenciesDiffs()) {
         descriptor.dependencies.push_back(frame_id - fdiff);
       }
     }
-    video_header.width = generic_descriptor.Width();
+
-    video_header.height = generic_descriptor.Height();
+    video_header.width = packet->generic_descriptor->Width();
+    video_header.height = packet->generic_descriptor->Height();
   }
 
   // Color space should only be transmitted in the last packet of a frame,
@@ -428,7 +435,7 @@ void RtpVideoStreamReceiver::OnReceivedPayloadData(
       // TODO(bugs.webrtc.org/10336): Implement support for reordering.
       RTC_LOG(LS_INFO)
           << "LossNotificationController does not support reordering.";
-    } else if (!has_generic_descriptor) {
+    } else if (!packet->generic_descriptor) {
       RTC_LOG(LS_WARNING) << "LossNotificationController requires generic "
                              "frame descriptor, but it is missing.";
     } else {