Reland "Remove PlayoutDelayOracle and make RtpSenderVideo guarantee delivery"
This is a reland of 4f68f5398d7fa3d47c449e99893c9bea07bf7ca2 Original change's description: > 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} TBR=stefan@webrtc.org Bug: webrtc:11340 Change-Id: I2fdd0004121b13b96497b21e052359e31d0c477a Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/168305 Commit-Queue: Erik Språng <sprang@webrtc.org> Reviewed-by: Sebastian Jansson <srte@webrtc.org> Cr-Commit-Position: refs/heads/master@{#30479}
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