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}diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc
index b2b6e7c..7f42a62 100644
--- a/modules/rtp_rtcp/source/rtp_sender.cc
+++ b/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 @@
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 @@
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 @@
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 @@
for (const RTCPReportBlock& report_block : report_blocks) {
if (ssrc == report_block.source_ssrc) {
- playout_delay_oracle_.OnReceivedAck(
- report_block.extended_highest_sequence_number);
+ video_->OnReceivedAck(report_block.extended_highest_sequence_number);
}
}
}
@@ -1075,11 +1057,7 @@
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_);
diff --git a/modules/rtp_rtcp/source/rtp_sender.h b/modules/rtp_rtcp/source/rtp_sender.h
index d0bc1c3..401a8ff 100644
--- a/modules/rtp_rtcp/source/rtp_sender.h
+++ b/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 @@
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 @@
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 @@
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.
diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc
index 60ffd32..92805f3 100644
--- a/modules/rtp_rtcp/source/rtp_sender_video.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_video.cc
@@ -53,6 +53,7 @@
}
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 @@
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 @@
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 @@
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,
- set_color_space, /*first=*/true, /*last=*/true,
- single_packet.get());
- AddRtpHeaderExtensions(*video_header, frame_type, set_video_rotation,
- set_color_space, /*first=*/true, /*last=*/false,
- first_packet.get());
- AddRtpHeaderExtensions(*video_header, frame_type, set_video_rotation,
- set_color_space, /*first=*/false, /*last=*/false,
- middle_packet.get());
- AddRtpHeaderExtensions(*video_header, frame_type, set_video_rotation,
- set_color_space, /*first=*/false, /*last=*/true,
- last_packet.get());
-
+ AddRtpHeaderExtensions(*video_header, playout_delay, frame_type,
+ set_video_rotation, set_color_space, /*first=*/true,
+ /*last=*/true, single_packet.get());
+ AddRtpHeaderExtensions(*video_header, playout_delay, frame_type,
+ set_video_rotation, set_color_space, /*first=*/true,
+ /*last=*/false, first_packet.get());
+ AddRtpHeaderExtensions(*video_header, playout_delay, frame_type,
+ set_video_rotation, set_color_space, /*first=*/false,
+ /*last=*/false, middle_packet.get());
+ AddRtpHeaderExtensions(*video_header, playout_delay, frame_type,
+ set_video_rotation, set_color_space, /*first=*/false,
+ /*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 @@
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;
diff --git a/modules/rtp_rtcp/source/rtp_sender_video.h b/modules/rtp_rtcp/source/rtp_sender_video.h
index ea5865b..bba549b 100644
--- a/modules/rtp_rtcp/source/rtp_sender_video.h
+++ b/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 @@
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 @@
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_);
