From be7a0ec2e61c0ca8245add2d97d694389ba5e88e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20M=C3=B6ller?= Date: Thu, 25 Apr 2019 10:02:52 +0200 Subject: [PATCH] Change vcm::VideoReceiver::IncomingPacket to not use WebRtcRTPHeader Bug: webrtc:10397 Change-Id: Id549516faab1b1047ef52dd8229a73eeb48c5fe2 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/134162 Reviewed-by: Philip Eliasson Commit-Queue: Niels Moller Cr-Commit-Position: refs/heads/master@{#27761} --- modules/video_coding/include/video_coding.h | 9 +- modules/video_coding/video_coding_impl.cc | 11 +- modules/video_coding/video_coding_impl.h | 3 +- modules/video_coding/video_receiver.cc | 12 +- .../video_coding/video_receiver_unittest.cc | 106 ++++++++++-------- 5 files changed, 86 insertions(+), 55 deletions(-) diff --git a/modules/video_coding/include/video_coding.h b/modules/video_coding/include/video_coding.h index 2fa5e39e15..5e0741764f 100644 --- a/modules/video_coding/include/video_coding.h +++ b/modules/video_coding/include/video_coding.h @@ -127,10 +127,17 @@ class VideoCodingModule : public Module { // Input: // - incomingPayload : Payload of the packet. // - payloadLength : Length of the payload. - // - rtpInfo : The parsed header. + // - rtp_header : The parsed RTP header. + // - video_header : The relevant extensions and payload header. // // Return value : VCM_OK, on success. // < 0, on error. + virtual int32_t IncomingPacket(const uint8_t* incomingPayload, + size_t payloadLength, + const RTPHeader& rtp_header, + const RTPVideoHeader& video_header) = 0; + + // DEPRECATED virtual int32_t IncomingPacket(const uint8_t* incomingPayload, size_t payloadLength, const WebRtcRTPHeader& rtpInfo) = 0; diff --git a/modules/video_coding/video_coding_impl.cc b/modules/video_coding/video_coding_impl.cc index 41a8eac716..d2741eae32 100644 --- a/modules/video_coding/video_coding_impl.cc +++ b/modules/video_coding/video_coding_impl.cc @@ -93,7 +93,16 @@ class VideoCodingModuleImpl : public VideoCodingModule { int32_t IncomingPacket(const uint8_t* incomingPayload, size_t payloadLength, const WebRtcRTPHeader& rtpInfo) override { - return receiver_.IncomingPacket(incomingPayload, payloadLength, rtpInfo); + return IncomingPacket(incomingPayload, payloadLength, rtpInfo.header, + rtpInfo.video_header()); + } + + int32_t IncomingPacket(const uint8_t* incomingPayload, + size_t payloadLength, + const RTPHeader& rtp_header, + const RTPVideoHeader& video_header) override { + return receiver_.IncomingPacket(incomingPayload, payloadLength, rtp_header, + video_header); } int SetReceiverRobustnessMode(ReceiverRobustness robustnessMode) override { diff --git a/modules/video_coding/video_coding_impl.h b/modules/video_coding/video_coding_impl.h index c348874dbd..2784a840e8 100644 --- a/modules/video_coding/video_coding_impl.h +++ b/modules/video_coding/video_coding_impl.h @@ -76,7 +76,8 @@ class VideoReceiver : public Module { int32_t IncomingPacket(const uint8_t* incomingPayload, size_t payloadLength, - const WebRtcRTPHeader& rtpInfo); + const RTPHeader& rtp_header, + const RTPVideoHeader& video_header); void SetNackSettings(size_t max_nack_list_size, int max_packet_age_to_nack, diff --git a/modules/video_coding/video_receiver.cc b/modules/video_coding/video_receiver.cc index 797f9c1031..8f9e849de1 100644 --- a/modules/video_coding/video_receiver.cc +++ b/modules/video_coding/video_receiver.cc @@ -323,11 +323,12 @@ int32_t VideoReceiver::RegisterReceiveCodec(const VideoCodec* receiveCodec, // Incoming packet from network parsed and ready for decode, non blocking. int32_t VideoReceiver::IncomingPacket(const uint8_t* incomingPayload, size_t payloadLength, - const WebRtcRTPHeader& rtpInfo) { + const RTPHeader& rtp_header, + const RTPVideoHeader& video_header) { RTC_DCHECK_RUN_ON(&module_thread_checker_); - if (rtpInfo.frameType == VideoFrameType::kVideoFrameKey) { + if (video_header.frame_type == VideoFrameType::kVideoFrameKey) { TRACE_EVENT1("webrtc", "VCM::PacketKeyFrame", "seqnum", - rtpInfo.header.sequenceNumber); + rtp_header.sequenceNumber); } if (incomingPayload == nullptr) { // The jitter buffer doesn't handle non-zero payload lengths for packets @@ -335,8 +336,9 @@ int32_t VideoReceiver::IncomingPacket(const uint8_t* incomingPayload, // TODO(holmer): We should fix this in the jitter buffer. payloadLength = 0; } - const VCMPacket packet(incomingPayload, payloadLength, rtpInfo.header, - rtpInfo.video_header(), rtpInfo.ntp_time_ms); + // Callers don't provide any ntp time. + const VCMPacket packet(incomingPayload, payloadLength, rtp_header, + video_header, /*ntp_time_ms=*/0); int32_t ret = _receiver.InsertPacket(packet); // TODO(holmer): Investigate if this somehow should use the key frame diff --git a/modules/video_coding/video_receiver_unittest.cc b/modules/video_coding/video_receiver_unittest.cc index 1c6bd43f6d..7526691587 100644 --- a/modules/video_coding/video_receiver_unittest.cc +++ b/modules/video_coding/video_receiver_unittest.cc @@ -55,23 +55,29 @@ class TestVideoReceiver : public ::testing::Test { receiver_.RegisterReceiveCallback(&receive_callback_); } - WebRtcRTPHeader GetDefaultVp8Header() const { - WebRtcRTPHeader header = {}; - header.frameType = VideoFrameType::kEmptyFrame; - header.header.markerBit = false; - header.header.payloadType = kUnusedPayloadType; - header.header.ssrc = 1; - header.header.headerLength = 12; - header.video_header().codec = kVideoCodecVP8; + RTPHeader GetDefaultRTPHeader() const { + RTPHeader header; + header.markerBit = false; + header.payloadType = kUnusedPayloadType; + header.ssrc = 1; + header.headerLength = 12; return header; } + RTPVideoHeader GetDefaultVp8Header() const { + RTPVideoHeader video_header = {}; + video_header.frame_type = VideoFrameType::kEmptyFrame; + video_header.codec = kVideoCodecVP8; + return video_header; + } + void InsertAndVerifyPaddingFrame(const uint8_t* payload, - WebRtcRTPHeader* header) { + RTPHeader* header, + const RTPVideoHeader& video_header) { for (int j = 0; j < 5; ++j) { // Padding only packets are passed to the VCM with payload size 0. - EXPECT_EQ(0, receiver_.IncomingPacket(payload, 0, *header)); - ++header->header.sequenceNumber; + EXPECT_EQ(0, receiver_.IncomingPacket(payload, 0, *header, video_header)); + ++header->sequenceNumber; } receiver_.Process(); EXPECT_CALL(decoder_, Decode(_, _, _)).Times(0); @@ -80,9 +86,11 @@ class TestVideoReceiver : public ::testing::Test { void InsertAndVerifyDecodableFrame(const uint8_t* payload, size_t length, - WebRtcRTPHeader* header) { - EXPECT_EQ(0, receiver_.IncomingPacket(payload, length, *header)); - ++header->header.sequenceNumber; + RTPHeader* header, + const RTPVideoHeader& video_header) { + EXPECT_EQ(0, + receiver_.IncomingPacket(payload, length, *header, video_header)); + ++header->sequenceNumber; EXPECT_CALL(packet_request_callback_, ResendPackets(_, _)).Times(0); receiver_.Process(); @@ -102,13 +110,14 @@ class TestVideoReceiver : public ::testing::Test { TEST_F(TestVideoReceiver, PaddingOnlyFrames) { const size_t kPaddingSize = 220; const uint8_t kPayload[kPaddingSize] = {0}; - WebRtcRTPHeader header = GetDefaultVp8Header(); - header.header.paddingLength = kPaddingSize; + RTPHeader header = GetDefaultRTPHeader(); + RTPVideoHeader video_header = GetDefaultVp8Header(); + header.paddingLength = kPaddingSize; for (int i = 0; i < 10; ++i) { EXPECT_CALL(packet_request_callback_, ResendPackets(_, _)).Times(0); - InsertAndVerifyPaddingFrame(kPayload, &header); + InsertAndVerifyPaddingFrame(kPayload, &header, video_header); clock_.AdvanceTimeMilliseconds(33); - header.header.timestamp += 3000; + header.timestamp += 3000; } } @@ -116,30 +125,31 @@ TEST_F(TestVideoReceiver, PaddingOnlyFramesWithLosses) { const size_t kFrameSize = 1200; const size_t kPaddingSize = 220; const uint8_t kPayload[kFrameSize] = {0}; - WebRtcRTPHeader header = GetDefaultVp8Header(); - header.header.paddingLength = kPaddingSize; - header.video_header().video_type_header.emplace(); + RTPHeader header = GetDefaultRTPHeader(); + RTPVideoHeader video_header = GetDefaultVp8Header(); + header.paddingLength = kPaddingSize; + video_header.video_type_header.emplace(); // Insert one video frame to get one frame decoded. - header.video_header().frame_type = VideoFrameType::kVideoFrameKey; - header.video_header().is_first_packet_in_frame = true; - header.header.markerBit = true; - InsertAndVerifyDecodableFrame(kPayload, kFrameSize, &header); + video_header.frame_type = VideoFrameType::kVideoFrameKey; + video_header.is_first_packet_in_frame = true; + header.markerBit = true; + InsertAndVerifyDecodableFrame(kPayload, kFrameSize, &header, video_header); clock_.AdvanceTimeMilliseconds(33); - header.header.timestamp += 3000; - header.video_header().frame_type = VideoFrameType::kEmptyFrame; - header.video_header().is_first_packet_in_frame = false; - header.header.markerBit = false; + header.timestamp += 3000; + video_header.frame_type = VideoFrameType::kEmptyFrame; + video_header.is_first_packet_in_frame = false; + header.markerBit = false; // Insert padding frames. for (int i = 0; i < 10; ++i) { // Lose one packet from the 6th frame. if (i == 5) { - ++header.header.sequenceNumber; + ++header.sequenceNumber; } // Lose the 4th frame. if (i == 3) { - header.header.sequenceNumber += 5; + header.sequenceNumber += 5; } else { if (i > 3 && i < 5) { EXPECT_CALL(packet_request_callback_, ResendPackets(_, 5)).Times(1); @@ -148,10 +158,10 @@ TEST_F(TestVideoReceiver, PaddingOnlyFramesWithLosses) { } else { EXPECT_CALL(packet_request_callback_, ResendPackets(_, _)).Times(0); } - InsertAndVerifyPaddingFrame(kPayload, &header); + InsertAndVerifyPaddingFrame(kPayload, &header, video_header); } clock_.AdvanceTimeMilliseconds(33); - header.header.timestamp += 3000; + header.timestamp += 3000; } } @@ -159,11 +169,12 @@ TEST_F(TestVideoReceiver, PaddingOnlyAndVideo) { const size_t kFrameSize = 1200; const size_t kPaddingSize = 220; const uint8_t kPayload[kFrameSize] = {0}; - WebRtcRTPHeader header = GetDefaultVp8Header(); - header.video_header().is_first_packet_in_frame = false; - header.header.paddingLength = kPaddingSize; + RTPHeader header = GetDefaultRTPHeader(); + RTPVideoHeader video_header = GetDefaultVp8Header(); + video_header.is_first_packet_in_frame = false; + header.paddingLength = kPaddingSize; auto& vp8_header = - header.video.video_type_header.emplace(); + video_header.video_type_header.emplace(); vp8_header.pictureId = -1; vp8_header.tl0PicIdx = -1; @@ -171,24 +182,25 @@ TEST_F(TestVideoReceiver, PaddingOnlyAndVideo) { // Insert 2 video frames. for (int j = 0; j < 2; ++j) { if (i == 0 && j == 0) // First frame should be a key frame. - header.video_header().frame_type = VideoFrameType::kVideoFrameKey; + video_header.frame_type = VideoFrameType::kVideoFrameKey; else - header.video_header().frame_type = VideoFrameType::kVideoFrameDelta; - header.video_header().is_first_packet_in_frame = true; - header.header.markerBit = true; - InsertAndVerifyDecodableFrame(kPayload, kFrameSize, &header); + video_header.frame_type = VideoFrameType::kVideoFrameDelta; + video_header.is_first_packet_in_frame = true; + header.markerBit = true; + InsertAndVerifyDecodableFrame(kPayload, kFrameSize, &header, + video_header); clock_.AdvanceTimeMilliseconds(33); - header.header.timestamp += 3000; + header.timestamp += 3000; } // Insert 2 padding only frames. - header.video_header().frame_type = VideoFrameType::kEmptyFrame; - header.video_header().is_first_packet_in_frame = false; - header.header.markerBit = false; + video_header.frame_type = VideoFrameType::kEmptyFrame; + video_header.is_first_packet_in_frame = false; + header.markerBit = false; for (int j = 0; j < 2; ++j) { // InsertAndVerifyPaddingFrame(kPayload, &header); clock_.AdvanceTimeMilliseconds(33); - header.header.timestamp += 3000; + header.timestamp += 3000; } } }