Move ownership of PlayoutDelayOracle

Moved from RtpSender to RtpSenderVideo, since currently the
PlayoutDelay extension is used for video only, and configured via
RTPVideoHeader.

Bug: webrtc:7135
Change-Id: Idfcc90cefea83e40a4e79164d7914cdcd50e41fe
Reviewed-on: https://webrtc-review.googlesource.com/c/120357
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26484}

modules/rtp_rtcp/source/rtp_sender.cc

@@ -23,7 +23,6 @@
 #include "modules/remote_bitrate_estimator/test/bwe_test_logging.h"
 #include "modules/rtp_rtcp/include/rtp_cvo.h"
 #include "modules/rtp_rtcp/source/byte_io.h"
-#include "modules/rtp_rtcp/source/playout_delay_oracle.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_to_send.h"
@@ -344,7 +343,6 @@ bool RTPSender::SendOutgoingData(FrameType frame_type,
                                  const RTPVideoHeader* rtp_header,
                                  uint32_t* transport_frame_id_out,
                                  int64_t expected_retransmission_time_ms) {
-  uint32_t ssrc;
   uint16_t sequence_number;
   uint32_t rtp_timestamp;
   {
@@ -352,7 +350,6 @@ bool RTPSender::SendOutgoingData(FrameType frame_type,
     rtc::CritScope lock(&send_critsect_);
     RTC_DCHECK(ssrc_);
 
-    ssrc = *ssrc_;
     sequence_number = sequence_number_;
     rtp_timestamp = timestamp_offset_ + capture_timestamp;
     if (transport_frame_id_out)
@@ -388,20 +385,6 @@ bool RTPSender::SendOutgoingData(FrameType frame_type,
     if (frame_type == kEmptyFrame)
       return true;
 
-    if (rtp_header) {
-      // TODO(nisse): This way of using PlayoutDelayOracle is a bit awkward. The
-      // intended way to use it is to call PlayoutDelayToSend at the place where
-      // the extension is written into the packet, and OnSentPacket later, after
-      // the final allocation of the sequence number. But currently the
-      // extension is set in AllocatePacket, where the RTPVideoHeader isn't
-      // available, so it always calls PlayoutDelayToSend with {-1,-1}. Hence,
-      // we have to use OnSentPacket early, and record both contents of the
-      // extension and the sequence number.
-      playout_delay_oracle_.OnSentPacket(
-          sequence_number,
-          playout_delay_oracle_.PlayoutDelayToSend(rtp_header->playout_delay));
-    }
-
     result = video_->SendVideo(frame_type, payload_type, rtp_timestamp,
                                capture_time_ms, payload_data, payload_size,
                                fragmentation, rtp_header,
@@ -675,8 +658,7 @@ void RTPSender::OnReceivedRtcpReportBlocks(
 
   for (const RTCPReportBlock& report_block : report_blocks) {
     if (ssrc == report_block.source_ssrc) {
-      playout_delay_oracle_.OnReceivedAck(
+      video_->OnReceivedAck(report_block.extended_highest_sequence_number);
-          report_block.extended_highest_sequence_number);
     }
   }
 }
@@ -1075,11 +1057,7 @@ std::unique_ptr<RtpPacketToSend> RTPSender::AllocatePacket() const {
   packet->ReserveExtension<AbsoluteSendTime>();
   packet->ReserveExtension<TransmissionOffset>();
   packet->ReserveExtension<TransportSequenceNumber>();
-  absl::optional<PlayoutDelay> playout_delay =
+
-      playout_delay_oracle_.PlayoutDelayToSend({-1, -1});
-  if (playout_delay) {
-    packet->SetExtension<PlayoutDelayLimits>(*playout_delay);
-  }
   if (!mid_.empty()) {
     // This is a no-op if the MID header extension is not registered.
     packet->SetExtension<RtpMid>(mid_);

modules/rtp_rtcp/source/rtp_sender.h

@@ -25,7 +25,6 @@
 #include "modules/rtp_rtcp/include/flexfec_sender.h"
 #include "modules/rtp_rtcp/include/rtp_header_extension_map.h"
 #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
-#include "modules/rtp_rtcp/source/playout_delay_oracle.h"
 #include "modules/rtp_rtcp/source/rtp_packet_history.h"
 #include "modules/rtp_rtcp/source/rtp_rtcp_config.h"
 #include "rtc_base/constructor_magic.h"
@@ -183,6 +182,7 @@ class RTPSender {
 
   absl::optional<uint32_t> FlexfecSsrc() const;
 
+  // Sends packet to |transport_| or to the pacer, depending on configuration.
   bool SendToNetwork(std::unique_ptr<RtpPacketToSend> packet,
                      StorageType storage,
                      RtpPacketSender::Priority priority);
@@ -239,6 +239,7 @@ class RTPSender {
   std::unique_ptr<RtpPacketToSend> BuildRtxPacket(
       const RtpPacketToSend& packet);
 
+  // Sends packet on to |transport_|, leaving the RTP module.
   bool SendPacketToNetwork(const RtpPacketToSend& packet,
                            const PacketOptions& options,
                            const PacedPacketInfo& pacing_info);
@@ -286,11 +287,6 @@ class RTPSender {
   RtpHeaderExtensionMap rtp_header_extension_map_
       RTC_GUARDED_BY(send_critsect_);
 
-  // Tracks the current request for playout delay limits from application
-  // and decides whether the current RTP frame should include the playout
-  // delay extension on header.
-  PlayoutDelayOracle playout_delay_oracle_;
-
   RtpPacketHistory packet_history_;
   // TODO(brandtr): Remove |flexfec_packet_history_| when the FlexfecSender
   // is hooked up to the PacedSender.

modules/rtp_rtcp/source/rtp_sender_video.cc

@@ -53,6 +53,7 @@ void BuildRedPayload(const RtpPacketToSend& media_packet,
 }
 
 void AddRtpHeaderExtensions(const RTPVideoHeader& video_header,
+                            const absl::optional<PlayoutDelay>& playout_delay,
                             FrameType frame_type,
                             bool set_video_rotation,
                             bool set_color_space,
@@ -78,6 +79,10 @@ void AddRtpHeaderExtensions(const RTPVideoHeader& video_header,
       video_header.video_timing.flags != VideoSendTiming::kInvalid)
     packet->SetExtension<VideoTimingExtension>(video_header.video_timing);
 
+  // If transmitted, add to all packets; ack logic depends on this.
+  if (playout_delay) {
+    packet->SetExtension<PlayoutDelayLimits>(*playout_delay);
+  }
   if (video_header.generic) {
     RtpGenericFrameDescriptor generic_descriptor;
     generic_descriptor.SetFirstPacketInSubFrame(first_packet);
@@ -378,6 +383,9 @@ bool RTPSenderVideo::SendVideo(FrameType frame_type,
   int32_t retransmission_settings;
   bool set_video_rotation;
   bool set_color_space = false;
+
+  const absl::optional<PlayoutDelay> playout_delay =
+      playout_delay_oracle_.PlayoutDelayToSend(video_header->playout_delay);
   {
     rtc::CritScope cs(&crit_);
     // According to
@@ -441,19 +449,18 @@ bool RTPSenderVideo::SendVideo(FrameType frame_type,
   auto middle_packet = absl::make_unique<RtpPacketToSend>(*single_packet);
   auto last_packet = absl::make_unique<RtpPacketToSend>(*single_packet);
   // Simplest way to estimate how much extensions would occupy is to set them.
-  AddRtpHeaderExtensions(*video_header, frame_type, set_video_rotation,
+  AddRtpHeaderExtensions(*video_header, playout_delay, frame_type,
-                         set_color_space, /*first=*/true, /*last=*/true,
+                         set_video_rotation, set_color_space, /*first=*/true,
-                         single_packet.get());
+                         /*last=*/true, single_packet.get());
-  AddRtpHeaderExtensions(*video_header, frame_type, set_video_rotation,
+  AddRtpHeaderExtensions(*video_header, playout_delay, frame_type,
-                         set_color_space, /*first=*/true, /*last=*/false,
+                         set_video_rotation, set_color_space, /*first=*/true,
-                         first_packet.get());
+                         /*last=*/false, first_packet.get());
-  AddRtpHeaderExtensions(*video_header, frame_type, set_video_rotation,
+  AddRtpHeaderExtensions(*video_header, playout_delay, frame_type,
-                         set_color_space, /*first=*/false, /*last=*/false,
+                         set_video_rotation, set_color_space, /*first=*/false,
-                         middle_packet.get());
+                         /*last=*/false, middle_packet.get());
-  AddRtpHeaderExtensions(*video_header, frame_type, set_video_rotation,
+  AddRtpHeaderExtensions(*video_header, playout_delay, frame_type,
-                         set_color_space, /*first=*/false, /*last=*/true,
+                         set_video_rotation, set_color_space, /*first=*/false,
-                         last_packet.get());
+                         /*last=*/true, last_packet.get());
-
   RTC_DCHECK_GT(packet_capacity, single_packet->headers_size());
   RTC_DCHECK_GT(packet_capacity, first_packet->headers_size());
   RTC_DCHECK_GT(packet_capacity, middle_packet->headers_size());
@@ -582,6 +589,10 @@ bool RTPSenderVideo::SendVideo(FrameType frame_type,
       return false;
     packetized_payload_size += packet->payload_size();
 
+    if (i == 0) {
+      playout_delay_oracle_.OnSentPacket(packet->SequenceNumber(),
+                                         playout_delay);
+    }
     // No FEC protection for upper temporal layers, if used.
     bool protect_packet = temporal_id == 0 || temporal_id == kNoTemporalIdx;
 

modules/rtp_rtcp/source/rtp_sender_video.h

@@ -19,6 +19,7 @@
 #include "common_types.h"  // NOLINT(build/include)
 #include "modules/rtp_rtcp/include/flexfec_sender.h"
 #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
+#include "modules/rtp_rtcp/source/playout_delay_oracle.h"
 #include "modules/rtp_rtcp/source/rtp_rtcp_config.h"
 #include "modules/rtp_rtcp/source/rtp_sender.h"
 #include "modules/rtp_rtcp/source/ulpfec_generator.h"
@@ -71,6 +72,10 @@ class RTPSenderVideo {
   uint32_t FecOverheadRate() const;
   uint32_t PacketizationOverheadBps() const;
 
+  void OnReceivedAck(int64_t extended_highest_sequence_number) {
+    playout_delay_oracle_.OnReceivedAck(extended_highest_sequence_number);
+  }
+
  protected:
   static uint8_t GetTemporalId(const RTPVideoHeader& header);
   StorageType GetStorageType(uint8_t temporal_id,
@@ -135,6 +140,10 @@ class RTPSenderVideo {
   VideoRotation last_rotation_ RTC_GUARDED_BY(crit_);
   absl::optional<ColorSpace> last_color_space_ RTC_GUARDED_BY(crit_);
   bool transmit_color_space_next_frame_ RTC_GUARDED_BY(crit_);
+  // Tracks the current request for playout delay limits from application
+  // and decides whether the current RTP frame should include the playout
+  // delay extension on header.
+  PlayoutDelayOracle playout_delay_oracle_;
 
   // RED/ULPFEC.
   int red_payload_type_ RTC_GUARDED_BY(crit_);