Remove PlayoutDelayOracle and make RtpSenderVideo guarantee delivery
The PlayoutDelayOracle was responsible for making sure the PlayoutDelay header extension was successfully propagated to the receiving side. Once it was determined that the receiver had received a frame with the new delay tag, it's no longer necessary to propagate. The issue with this implementation is that it is based on max extended sequence number reported via RTCP, which makes it often slow to react, could theoretically fail to produce desired outcome (max received > X does not guarantee X was fully received and decoded), and added a lot of code complexity. The guarantee of delivery can in fact be accomplished more reliably and with less code by making sure to tag each frame until an undiscardable frame is sent. This allows containing the logic fully within RTPSenderVideo. Bug: webrtc:11340 Change-Id: I2d1d2b6b67f4f07b8b33336f8fcfcde724243eef Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/168221 Reviewed-by: Stefan Holmer <stefan@webrtc.org> Reviewed-by: Sebastian Jansson <srte@webrtc.org> Commit-Queue: Erik Språng <sprang@webrtc.org> Cr-Commit-Position: refs/heads/master@{#30473}
This commit is contained in:
@ -54,6 +54,7 @@ enum : int { // The first valid value is 1.
|
||||
kVideoRotationExtensionId,
|
||||
kVideoTimingExtensionId,
|
||||
kAbsoluteCaptureTimeExtensionId,
|
||||
kPlayoutDelayExtensionId
|
||||
};
|
||||
|
||||
constexpr int kPayload = 100;
|
||||
@ -87,6 +88,8 @@ class LoopbackTransportTest : public webrtc::Transport {
|
||||
kFrameMarkingExtensionId);
|
||||
receivers_extensions_.Register<AbsoluteCaptureTimeExtension>(
|
||||
kAbsoluteCaptureTimeExtensionId);
|
||||
receivers_extensions_.Register<PlayoutDelayLimits>(
|
||||
kPlayoutDelayExtensionId);
|
||||
}
|
||||
|
||||
bool SendRtp(const uint8_t* data,
|
||||
@ -121,7 +124,6 @@ class TestRtpSenderVideo : public RTPSenderVideo {
|
||||
config.clock = clock;
|
||||
config.rtp_sender = rtp_sender;
|
||||
config.flexfec_sender = flexfec_sender;
|
||||
config.playout_delay_oracle = &playout_delay_oracle_;
|
||||
config.field_trials = &field_trials;
|
||||
return config;
|
||||
}()) {}
|
||||
@ -134,7 +136,6 @@ class TestRtpSenderVideo : public RTPSenderVideo {
|
||||
retransmission_settings,
|
||||
expected_retransmission_time_ms);
|
||||
}
|
||||
PlayoutDelayOracle playout_delay_oracle_;
|
||||
};
|
||||
|
||||
class FieldTrials : public WebRtcKeyValueConfig {
|
||||
@ -792,6 +793,63 @@ TEST_P(RtpSenderVideoTest, AbsoluteCaptureTime) {
|
||||
EXPECT_EQ(packets_with_abs_capture_time, 1);
|
||||
}
|
||||
|
||||
TEST_P(RtpSenderVideoTest, PopulatesPlayoutDelay) {
|
||||
// Single packet frames.
|
||||
constexpr size_t kPacketSize = 123;
|
||||
uint8_t kFrame[kPacketSize];
|
||||
rtp_module_->RegisterRtpHeaderExtension(PlayoutDelayLimits::kUri,
|
||||
kPlayoutDelayExtensionId);
|
||||
const PlayoutDelay kExpectedDelay = {10, 20};
|
||||
|
||||
// Send initial key-frame without playout delay.
|
||||
RTPVideoHeader hdr;
|
||||
hdr.frame_type = VideoFrameType::kVideoFrameKey;
|
||||
hdr.codec = VideoCodecType::kVideoCodecVP8;
|
||||
auto& vp8_header = hdr.video_type_header.emplace<RTPVideoHeaderVP8>();
|
||||
vp8_header.temporalIdx = 0;
|
||||
|
||||
rtp_sender_video_.SendVideo(kPayload, kType, kTimestamp, 0, kFrame, nullptr,
|
||||
hdr, kDefaultExpectedRetransmissionTimeMs);
|
||||
EXPECT_FALSE(
|
||||
transport_.last_sent_packet().HasExtension<PlayoutDelayLimits>());
|
||||
|
||||
// Set playout delay on a discardable frame.
|
||||
hdr.playout_delay = kExpectedDelay;
|
||||
hdr.frame_type = VideoFrameType::kVideoFrameDelta;
|
||||
vp8_header.temporalIdx = 1;
|
||||
rtp_sender_video_.SendVideo(kPayload, kType, kTimestamp, 0, kFrame, nullptr,
|
||||
hdr, kDefaultExpectedRetransmissionTimeMs);
|
||||
PlayoutDelay received_delay = PlayoutDelay::Noop();
|
||||
ASSERT_TRUE(transport_.last_sent_packet().GetExtension<PlayoutDelayLimits>(
|
||||
&received_delay));
|
||||
EXPECT_EQ(received_delay, kExpectedDelay);
|
||||
|
||||
// Set playout delay on a non-discardable frame, the extension should still
|
||||
// be populated since dilvery wasn't guaranteed on the last one.
|
||||
hdr.playout_delay = PlayoutDelay::Noop(); // Inidcates "no change".
|
||||
vp8_header.temporalIdx = 0;
|
||||
rtp_sender_video_.SendVideo(kPayload, kType, kTimestamp, 0, kFrame, nullptr,
|
||||
hdr, kDefaultExpectedRetransmissionTimeMs);
|
||||
ASSERT_TRUE(transport_.last_sent_packet().GetExtension<PlayoutDelayLimits>(
|
||||
&received_delay));
|
||||
EXPECT_EQ(received_delay, kExpectedDelay);
|
||||
|
||||
// The next frame does not need the extensions since it's delivery has
|
||||
// already been guaranteed.
|
||||
rtp_sender_video_.SendVideo(kPayload, kType, kTimestamp, 0, kFrame, nullptr,
|
||||
hdr, kDefaultExpectedRetransmissionTimeMs);
|
||||
EXPECT_FALSE(
|
||||
transport_.last_sent_packet().HasExtension<PlayoutDelayLimits>());
|
||||
|
||||
// Insert key-frame, we need to refresh the state here.
|
||||
hdr.frame_type = VideoFrameType::kVideoFrameKey;
|
||||
rtp_sender_video_.SendVideo(kPayload, kType, kTimestamp, 0, kFrame, nullptr,
|
||||
hdr, kDefaultExpectedRetransmissionTimeMs);
|
||||
ASSERT_TRUE(transport_.last_sent_packet().GetExtension<PlayoutDelayLimits>(
|
||||
&received_delay));
|
||||
EXPECT_EQ(received_delay, kExpectedDelay);
|
||||
}
|
||||
|
||||
INSTANTIATE_TEST_SUITE_P(WithAndWithoutOverhead,
|
||||
RtpSenderVideoTest,
|
||||
::testing::Bool());
|
||||
|
||||
Reference in New Issue
Block a user