From a32d7e2a2f346fb102daf8436693d4f78f689bf6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20M=C3=B6ller?= Date: Fri, 16 Nov 2018 12:35:26 +0100 Subject: [PATCH] Add default values for PlayoutDelay in RTPVideoHeader. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There have been several bugs where the members of PlayoutDelay were zero initialized when handling RTP packets without the corresponding extensions. Initializing to {-1, -1} (meaning not provided) is less brittle. Bug: None Change-Id: I196850377128d5e67a19bdaf9298403b2e9f5a6e Reviewed-on: https://webrtc-review.googlesource.com/c/111181 Reviewed-by: Erik Språng Reviewed-by: Danil Chapovalov Commit-Queue: Niels Moller Cr-Commit-Position: refs/heads/master@{#25670} --- modules/rtp_rtcp/source/rtp_video_header.cc | 2 +- modules/rtp_rtcp/source/rtp_video_header.h | 2 +- video/rtp_video_stream_receiver.cc | 2 -- video/rtp_video_stream_receiver_unittest.cc | 9 +++++++++ 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/modules/rtp_rtcp/source/rtp_video_header.cc b/modules/rtp_rtcp/source/rtp_video_header.cc index a3ee8baa83..bb9413ddd5 100644 --- a/modules/rtp_rtcp/source/rtp_video_header.cc +++ b/modules/rtp_rtcp/source/rtp_video_header.cc @@ -12,7 +12,7 @@ namespace webrtc { -RTPVideoHeader::RTPVideoHeader() : playout_delay(), video_timing() {} +RTPVideoHeader::RTPVideoHeader() : video_timing() {} RTPVideoHeader::RTPVideoHeader(const RTPVideoHeader& other) = default; RTPVideoHeader::~RTPVideoHeader() = default; diff --git a/modules/rtp_rtcp/source/rtp_video_header.h b/modules/rtp_rtcp/source/rtp_video_header.h index 4426c41895..ddf5942e39 100644 --- a/modules/rtp_rtcp/source/rtp_video_header.h +++ b/modules/rtp_rtcp/source/rtp_video_header.h @@ -59,7 +59,7 @@ struct RTPVideoHeader { uint8_t simulcastIdx = 0; VideoCodecType codec = VideoCodecType::kVideoCodecGeneric; - PlayoutDelay playout_delay; + PlayoutDelay playout_delay = {-1, -1}; VideoSendTiming video_timing; FrameMarking frame_marking; RTPVideoTypeHeader video_type_header; diff --git a/video/rtp_video_stream_receiver.cc b/video/rtp_video_stream_receiver.cc index 8383c07372..f85ae2d59e 100644 --- a/video/rtp_video_stream_receiver.cc +++ b/video/rtp_video_stream_receiver.cc @@ -530,8 +530,6 @@ void RtpVideoStreamReceiver::ReceivePacket(const RtpPacketReceived& packet) { webrtc_rtp_header.video_header().content_type = VideoContentType::UNSPECIFIED; webrtc_rtp_header.video_header().video_timing.flags = VideoSendTiming::kInvalid; - webrtc_rtp_header.video_header().playout_delay.min_ms = -1; - webrtc_rtp_header.video_header().playout_delay.max_ms = -1; webrtc_rtp_header.video_header().is_last_packet_in_frame = webrtc_rtp_header.header.markerBit; diff --git a/video/rtp_video_stream_receiver_unittest.cc b/video/rtp_video_stream_receiver_unittest.cc index 81342b826a..10c5e7eebc 100644 --- a/video/rtp_video_stream_receiver_unittest.cc +++ b/video/rtp_video_stream_receiver_unittest.cc @@ -626,4 +626,13 @@ TEST_F(RtpVideoStreamReceiverTest, RepeatedSecondarySinkDisallowed) { } #endif +// Initialization of WebRtcRTPHeader is a bit convoluted, with some fields +// zero-initialized. RtpVideoStreamReceiver depends on proper default values for +// the playout delay. +TEST(WebRtcRTPHeader, DefaultPlayoutDelayIsUnspecified) { + WebRtcRTPHeader webrtc_rtp_header = {}; + EXPECT_EQ(webrtc_rtp_header.video_header().playout_delay.min_ms, -1); + EXPECT_EQ(webrtc_rtp_header.video_header().playout_delay.max_ms, -1); +} + } // namespace webrtc