From 125b5d6ffb98a1c4dfada7bf0418d4db2cced579 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20M=C3=B6ller?= Date: Mon, 11 Mar 2019 16:11:07 +0100 Subject: [PATCH] Refactor RtpVideoStreamReceiver::OnReceivedPayloadData without WebRtcRTPHeader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: None Change-Id: I7f96d9a7435a12a0390149e7854100f7fb7e7431 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/126160 Commit-Queue: Niels Moller Reviewed-by: Philip Eliasson Reviewed-by: Åsa Persson Cr-Commit-Position: refs/heads/master@{#27062} --- video/rtp_video_stream_receiver.cc | 92 ++++------ video/rtp_video_stream_receiver.h | 9 +- video/rtp_video_stream_receiver_unittest.cc | 181 ++++++++++---------- 3 files changed, 133 insertions(+), 149 deletions(-) diff --git a/video/rtp_video_stream_receiver.cc b/video/rtp_video_stream_receiver.cc index de8e2dcb46..0cb0c35110 100644 --- a/video/rtp_video_stream_receiver.cc +++ b/video/rtp_video_stream_receiver.cc @@ -226,31 +226,21 @@ absl::optional RtpVideoStreamReceiver::GetSyncInfo() const { int32_t RtpVideoStreamReceiver::OnReceivedPayloadData( const uint8_t* payload_data, size_t payload_size, - const WebRtcRTPHeader* rtp_header) { - return OnReceivedPayloadData(payload_data, payload_size, rtp_header, - absl::nullopt, false); -} - -int32_t RtpVideoStreamReceiver::OnReceivedPayloadData( - const uint8_t* payload_data, - size_t payload_size, - const WebRtcRTPHeader* rtp_header, + const RTPHeader& rtp_header, + const RTPVideoHeader& video_header, + FrameType frame_type, const absl::optional& generic_descriptor, bool is_recovered) { - WebRtcRTPHeader rtp_header_with_ntp = *rtp_header; - rtp_header_with_ntp.ntp_time_ms = - ntp_estimator_.Estimate(rtp_header->header.timestamp); - - VCMPacket packet(payload_data, payload_size, rtp_header_with_ntp); + VCMPacket packet(payload_data, payload_size, rtp_header, video_header, + frame_type, ntp_estimator_.Estimate(rtp_header.timestamp)); packet.generic_descriptor = generic_descriptor; if (nack_module_) { const bool is_keyframe = - rtp_header->video_header().is_first_packet_in_frame && - rtp_header->frameType == kVideoFrameKey; + video_header.is_first_packet_in_frame && frame_type == kVideoFrameKey; packet.timesNacked = nack_module_->OnReceivedPacket( - rtp_header->header.sequenceNumber, is_keyframe, is_recovered); + rtp_header.sequenceNumber, is_keyframe, is_recovered); } else { packet.timesNacked = -1; @@ -528,49 +518,36 @@ void RtpVideoStreamReceiver::ReceivePacket(const RtpPacketReceived& packet) { return; } - WebRtcRTPHeader webrtc_rtp_header = {}; - packet.GetHeader(&webrtc_rtp_header.header); - - webrtc_rtp_header.frameType = parsed_payload.frame_type; - webrtc_rtp_header.video_header() = parsed_payload.video_header(); - webrtc_rtp_header.video_header().rotation = kVideoRotation_0; - webrtc_rtp_header.video_header().content_type = VideoContentType::UNSPECIFIED; - webrtc_rtp_header.video_header().video_timing.flags = - VideoSendTiming::kInvalid; - webrtc_rtp_header.video_header().is_last_packet_in_frame = - webrtc_rtp_header.header.markerBit; - webrtc_rtp_header.video_header().frame_marking.temporal_id = kNoTemporalIdx; + RTPHeader rtp_header; + packet.GetHeader(&rtp_header); + RTPVideoHeader video_header = parsed_payload.video_header(); + video_header.rotation = kVideoRotation_0; + video_header.content_type = VideoContentType::UNSPECIFIED; + video_header.video_timing.flags = VideoSendTiming::kInvalid; + video_header.is_last_packet_in_frame = rtp_header.markerBit; + video_header.frame_marking.temporal_id = kNoTemporalIdx; if (parsed_payload.video_header().codec == kVideoCodecVP9) { const RTPVideoHeaderVP9& codec_header = absl::get( parsed_payload.video_header().video_type_header); - webrtc_rtp_header.video_header().is_last_packet_in_frame |= - codec_header.end_of_frame; - webrtc_rtp_header.video_header().is_first_packet_in_frame |= - codec_header.beginning_of_frame; + video_header.is_last_packet_in_frame |= codec_header.end_of_frame; + video_header.is_first_packet_in_frame |= codec_header.beginning_of_frame; } - packet.GetExtension( - &webrtc_rtp_header.video_header().rotation); - packet.GetExtension( - &webrtc_rtp_header.video_header().content_type); - packet.GetExtension( - &webrtc_rtp_header.video_header().video_timing); - packet.GetExtension( - &webrtc_rtp_header.video_header().playout_delay); - packet.GetExtension( - &webrtc_rtp_header.video_header().frame_marking); + packet.GetExtension(&video_header.rotation); + packet.GetExtension(&video_header.content_type); + packet.GetExtension(&video_header.video_timing); + packet.GetExtension(&video_header.playout_delay); + packet.GetExtension(&video_header.frame_marking); - webrtc_rtp_header.video_header().color_space = - packet.GetExtension(); - if (webrtc_rtp_header.video_header().color_space || - webrtc_rtp_header.frameType == kVideoFrameKey) { + video_header.color_space = packet.GetExtension(); + if (video_header.color_space || parsed_payload.frame_type == kVideoFrameKey) { // Store color space since it's only transmitted when changed or for key // frames. Color space will be cleared if a key frame is transmitted without // color space information. - last_color_space_ = webrtc_rtp_header.video_header().color_space; + last_color_space_ = video_header.color_space; } else if (last_color_space_) { - webrtc_rtp_header.video_header().color_space = last_color_space_; + video_header.color_space = last_color_space_; } absl::optional generic_descriptor_wire; @@ -595,28 +572,27 @@ void RtpVideoStreamReceiver::ReceivePacket(const RtpPacketReceived& packet) { packet.GetRawExtension()); } - webrtc_rtp_header.video_header().is_first_packet_in_frame = + video_header.is_first_packet_in_frame = generic_descriptor_wire->FirstPacketInSubFrame(); - webrtc_rtp_header.video_header().is_last_packet_in_frame = - webrtc_rtp_header.header.markerBit || - generic_descriptor_wire->LastPacketInSubFrame(); + video_header.is_last_packet_in_frame = + rtp_header.markerBit || generic_descriptor_wire->LastPacketInSubFrame(); if (generic_descriptor_wire->FirstPacketInSubFrame()) { - webrtc_rtp_header.frameType = + parsed_payload.frame_type = generic_descriptor_wire->FrameDependenciesDiffs().empty() ? kVideoFrameKey : kVideoFrameDelta; } - webrtc_rtp_header.video_header().width = generic_descriptor_wire->Width(); - webrtc_rtp_header.video_header().height = generic_descriptor_wire->Height(); + video_header.width = generic_descriptor_wire->Width(); + video_header.height = generic_descriptor_wire->Height(); } else { generic_descriptor_wire.reset(); } OnReceivedPayloadData(parsed_payload.payload, parsed_payload.payload_length, - &webrtc_rtp_header, generic_descriptor_wire, - packet.recovered()); + rtp_header, video_header, parsed_payload.frame_type, + generic_descriptor_wire, packet.recovered()); } void RtpVideoStreamReceiver::ParseAndHandleEncapsulatingHeader( diff --git a/video/rtp_video_stream_receiver.h b/video/rtp_video_stream_receiver.h index 3824680fca..e8f78db25e 100644 --- a/video/rtp_video_stream_receiver.h +++ b/video/rtp_video_stream_receiver.h @@ -105,14 +105,13 @@ class RtpVideoStreamReceiver : public LossNotificationSender, void OnRtpPacket(const RtpPacketReceived& packet) override; // TODO(philipel): Stop using VCMPacket in the new jitter buffer and then - // remove this function. - int32_t OnReceivedPayloadData(const uint8_t* payload_data, - size_t payload_size, - const WebRtcRTPHeader* rtp_header); + // remove this function. Public only for tests. int32_t OnReceivedPayloadData( const uint8_t* payload_data, size_t payload_size, - const WebRtcRTPHeader* rtp_header, + const RTPHeader& rtp_header, + const RTPVideoHeader& video_header, + FrameType frame_type, const absl::optional& generic_descriptor, bool is_recovered); diff --git a/video/rtp_video_stream_receiver_unittest.cc b/video/rtp_video_stream_receiver_unittest.cc index 8521732e0e..b39e459451 100644 --- a/video/rtp_video_stream_receiver_unittest.cc +++ b/video/rtp_video_stream_receiver_unittest.cc @@ -136,16 +136,16 @@ class RtpVideoStreamReceiverTest : public testing::Test { &mock_on_complete_frame_callback_, nullptr); } - WebRtcRTPHeader GetDefaultPacket() { - WebRtcRTPHeader packet = {}; - packet.video_header().codec = kVideoCodecH264; - packet.video_header().video_type_header.emplace(); - return packet; + RTPVideoHeader GetDefaultH264VideoHeader() { + RTPVideoHeader video_header; + video_header.codec = kVideoCodecH264; + video_header.video_type_header.emplace(); + return video_header; } // TODO(Johan): refactor h264_sps_pps_tracker_unittests.cc to avoid duplicate // code. - void AddSps(WebRtcRTPHeader* packet, + void AddSps(RTPVideoHeader* video_header, uint8_t sps_id, std::vector* data) { NaluInfo info; @@ -154,12 +154,11 @@ class RtpVideoStreamReceiverTest : public testing::Test { info.pps_id = -1; data->push_back(H264::NaluType::kSps); data->push_back(sps_id); - auto& h264 = - absl::get(packet->video_header().video_type_header); + auto& h264 = absl::get(video_header->video_type_header); h264.nalus[h264.nalus_length++] = info; } - void AddPps(WebRtcRTPHeader* packet, + void AddPps(RTPVideoHeader* video_header, uint8_t sps_id, uint8_t pps_id, std::vector* data) { @@ -169,18 +168,16 @@ class RtpVideoStreamReceiverTest : public testing::Test { info.pps_id = pps_id; data->push_back(H264::NaluType::kPps); data->push_back(pps_id); - auto& h264 = - absl::get(packet->video_header().video_type_header); + auto& h264 = absl::get(video_header->video_type_header); h264.nalus[h264.nalus_length++] = info; } - void AddIdr(WebRtcRTPHeader* packet, int pps_id) { + void AddIdr(RTPVideoHeader* video_header, int pps_id) { NaluInfo info; info.type = H264::NaluType::kIdr; info.sps_id = -1; info.pps_id = pps_id; - auto& h264 = - absl::get(packet->video_header().video_type_header); + auto& h264 = absl::get(video_header->video_type_header); h264.nalus[h264.nalus_length++] = info; } @@ -205,18 +202,19 @@ class RtpVideoStreamReceiverTest : public testing::Test { }; TEST_F(RtpVideoStreamReceiverTest, GenericKeyFrame) { - WebRtcRTPHeader rtp_header = {}; + RTPHeader rtp_header; + RTPVideoHeader video_header; const std::vector data({1, 2, 3, 4}); - rtp_header.header.sequenceNumber = 1; - rtp_header.video_header().is_first_packet_in_frame = true; - rtp_header.video_header().is_last_packet_in_frame = true; - rtp_header.frameType = kVideoFrameKey; - rtp_header.video_header().codec = kVideoCodecGeneric; + rtp_header.sequenceNumber = 1; + video_header.is_first_packet_in_frame = true; + video_header.is_last_packet_in_frame = true; + video_header.codec = kVideoCodecGeneric; mock_on_complete_frame_callback_.AppendExpectedBitstream(data.data(), data.size()); EXPECT_CALL(mock_on_complete_frame_callback_, DoOnCompleteFrame(_)); - rtp_video_stream_receiver_->OnReceivedPayloadData(data.data(), data.size(), - &rtp_header); + rtp_video_stream_receiver_->OnReceivedPayloadData( + data.data(), data.size(), rtp_header, video_header, kVideoFrameKey, + absl::nullopt, false); } TEST_F(RtpVideoStreamReceiverTest, NoInfiniteRecursionOnEncapsulatedRedPacket) { @@ -262,20 +260,21 @@ TEST_F(RtpVideoStreamReceiverTest, } TEST_F(RtpVideoStreamReceiverTest, GenericKeyFrameBitstreamError) { - WebRtcRTPHeader rtp_header = {}; + RTPHeader rtp_header; + RTPVideoHeader video_header; const std::vector data({1, 2, 3, 4}); - rtp_header.header.sequenceNumber = 1; - rtp_header.video_header().is_first_packet_in_frame = true; - rtp_header.video_header().is_last_packet_in_frame = true; - rtp_header.frameType = kVideoFrameKey; - rtp_header.video_header().codec = kVideoCodecGeneric; + rtp_header.sequenceNumber = 1; + video_header.is_first_packet_in_frame = true; + video_header.is_last_packet_in_frame = true; + video_header.codec = kVideoCodecGeneric; constexpr uint8_t expected_bitsteam[] = {1, 2, 3, 0xff}; mock_on_complete_frame_callback_.AppendExpectedBitstream( expected_bitsteam, sizeof(expected_bitsteam)); EXPECT_CALL(mock_on_complete_frame_callback_, DoOnCompleteFrameFailBitstream(_)); - rtp_video_stream_receiver_->OnReceivedPayloadData(data.data(), data.size(), - &rtp_header); + rtp_video_stream_receiver_->OnReceivedPayloadData( + data.data(), data.size(), rtp_header, video_header, kVideoFrameKey, + absl::nullopt, false); } class RtpVideoStreamReceiverTestH264 @@ -291,36 +290,38 @@ INSTANTIATE_TEST_SUITE_P(SpsPpsIdrIsKeyframe, TEST_P(RtpVideoStreamReceiverTestH264, InBandSpsPps) { std::vector sps_data; - WebRtcRTPHeader sps_packet = GetDefaultPacket(); - AddSps(&sps_packet, 0, &sps_data); - sps_packet.header.sequenceNumber = 0; - sps_packet.video_header().is_first_packet_in_frame = true; + RTPHeader rtp_header; + RTPVideoHeader sps_video_header = GetDefaultH264VideoHeader(); + AddSps(&sps_video_header, 0, &sps_data); + rtp_header.sequenceNumber = 0; + sps_video_header.is_first_packet_in_frame = true; mock_on_complete_frame_callback_.AppendExpectedBitstream( kH264StartCode, sizeof(kH264StartCode)); mock_on_complete_frame_callback_.AppendExpectedBitstream(sps_data.data(), sps_data.size()); rtp_video_stream_receiver_->OnReceivedPayloadData( - sps_data.data(), sps_data.size(), &sps_packet); + sps_data.data(), sps_data.size(), rtp_header, sps_video_header, + kEmptyFrame, absl::nullopt, false); std::vector pps_data; - WebRtcRTPHeader pps_packet = GetDefaultPacket(); - AddPps(&pps_packet, 0, 1, &pps_data); - pps_packet.header.sequenceNumber = 1; - pps_packet.video_header().is_first_packet_in_frame = true; + RTPVideoHeader pps_video_header = GetDefaultH264VideoHeader(); + AddPps(&pps_video_header, 0, 1, &pps_data); + rtp_header.sequenceNumber = 1; + pps_video_header.is_first_packet_in_frame = true; mock_on_complete_frame_callback_.AppendExpectedBitstream( kH264StartCode, sizeof(kH264StartCode)); mock_on_complete_frame_callback_.AppendExpectedBitstream(pps_data.data(), pps_data.size()); rtp_video_stream_receiver_->OnReceivedPayloadData( - pps_data.data(), pps_data.size(), &pps_packet); + pps_data.data(), pps_data.size(), rtp_header, pps_video_header, + kEmptyFrame, absl::nullopt, false); std::vector idr_data; - WebRtcRTPHeader idr_packet = GetDefaultPacket(); - AddIdr(&idr_packet, 1); - idr_packet.header.sequenceNumber = 2; - idr_packet.video_header().is_first_packet_in_frame = true; - idr_packet.video_header().is_last_packet_in_frame = true; - idr_packet.frameType = kVideoFrameKey; + RTPVideoHeader idr_video_header = GetDefaultH264VideoHeader(); + AddIdr(&idr_video_header, 1); + rtp_header.sequenceNumber = 2; + idr_video_header.is_first_packet_in_frame = true; + idr_video_header.is_last_packet_in_frame = true; idr_data.insert(idr_data.end(), {0x65, 1, 2, 3}); mock_on_complete_frame_callback_.AppendExpectedBitstream( kH264StartCode, sizeof(kH264StartCode)); @@ -328,7 +329,8 @@ TEST_P(RtpVideoStreamReceiverTestH264, InBandSpsPps) { idr_data.size()); EXPECT_CALL(mock_on_complete_frame_callback_, DoOnCompleteFrame(_)); rtp_video_stream_receiver_->OnReceivedPayloadData( - idr_data.data(), idr_data.size(), &idr_packet); + idr_data.data(), idr_data.size(), rtp_header, idr_video_header, + kVideoFrameKey, absl::nullopt, false); } TEST_P(RtpVideoStreamReceiverTestH264, OutOfBandFmtpSpsPps) { @@ -354,72 +356,79 @@ TEST_P(RtpVideoStreamReceiverTestH264, OutOfBandFmtpSpsPps) { sizeof(binary_pps)); std::vector data; - WebRtcRTPHeader idr_packet = GetDefaultPacket(); - AddIdr(&idr_packet, 0); - idr_packet.header.payloadType = kPayloadType; - idr_packet.video_header().is_first_packet_in_frame = true; - idr_packet.header.sequenceNumber = 2; - idr_packet.video_header().is_first_packet_in_frame = true; - idr_packet.video_header().is_last_packet_in_frame = true; - idr_packet.frameType = kVideoFrameKey; - idr_packet.video_header().codec = kVideoCodecH264; + RTPHeader rtp_header; + RTPVideoHeader video_header = GetDefaultH264VideoHeader(); + AddIdr(&video_header, 0); + rtp_header.payloadType = kPayloadType; + rtp_header.sequenceNumber = 2; + video_header.is_first_packet_in_frame = true; + video_header.is_last_packet_in_frame = true; + video_header.codec = kVideoCodecH264; data.insert(data.end(), {1, 2, 3}); mock_on_complete_frame_callback_.AppendExpectedBitstream( kH264StartCode, sizeof(kH264StartCode)); mock_on_complete_frame_callback_.AppendExpectedBitstream(data.data(), data.size()); EXPECT_CALL(mock_on_complete_frame_callback_, DoOnCompleteFrame(_)); - rtp_video_stream_receiver_->OnReceivedPayloadData(data.data(), data.size(), - &idr_packet); + rtp_video_stream_receiver_->OnReceivedPayloadData( + data.data(), data.size(), rtp_header, video_header, kVideoFrameKey, + absl::nullopt, false); } TEST_F(RtpVideoStreamReceiverTest, PaddingInMediaStream) { - WebRtcRTPHeader header = GetDefaultPacket(); + RTPHeader rtp_header; + RTPVideoHeader video_header = GetDefaultH264VideoHeader(); std::vector data; data.insert(data.end(), {1, 2, 3}); - header.header.payloadType = 99; - header.video_header().is_first_packet_in_frame = true; - header.video_header().is_last_packet_in_frame = true; - header.header.sequenceNumber = 2; - header.frameType = kVideoFrameKey; - header.video_header().codec = kVideoCodecGeneric; + rtp_header.payloadType = 99; + rtp_header.sequenceNumber = 2; + video_header.is_first_packet_in_frame = true; + video_header.is_last_packet_in_frame = true; + video_header.codec = kVideoCodecGeneric; mock_on_complete_frame_callback_.AppendExpectedBitstream(data.data(), data.size()); EXPECT_CALL(mock_on_complete_frame_callback_, DoOnCompleteFrame(_)); - rtp_video_stream_receiver_->OnReceivedPayloadData(data.data(), data.size(), - &header); + rtp_video_stream_receiver_->OnReceivedPayloadData( + data.data(), data.size(), rtp_header, video_header, kVideoFrameKey, + absl::nullopt, false); - header.header.sequenceNumber = 3; - rtp_video_stream_receiver_->OnReceivedPayloadData(nullptr, 0, &header); + rtp_header.sequenceNumber = 3; + rtp_video_stream_receiver_->OnReceivedPayloadData( + nullptr, 0, rtp_header, video_header, kVideoFrameKey, absl::nullopt, + false); - header.frameType = kVideoFrameDelta; - header.header.sequenceNumber = 4; + rtp_header.sequenceNumber = 4; EXPECT_CALL(mock_on_complete_frame_callback_, DoOnCompleteFrame(_)); - rtp_video_stream_receiver_->OnReceivedPayloadData(data.data(), data.size(), - &header); + rtp_video_stream_receiver_->OnReceivedPayloadData( + data.data(), data.size(), rtp_header, video_header, kVideoFrameDelta, + absl::nullopt, false); - header.header.sequenceNumber = 6; - rtp_video_stream_receiver_->OnReceivedPayloadData(data.data(), data.size(), - &header); + rtp_header.sequenceNumber = 6; + rtp_video_stream_receiver_->OnReceivedPayloadData( + data.data(), data.size(), rtp_header, video_header, kVideoFrameDelta, + absl::nullopt, false); EXPECT_CALL(mock_on_complete_frame_callback_, DoOnCompleteFrame(_)); - header.header.sequenceNumber = 5; - rtp_video_stream_receiver_->OnReceivedPayloadData(nullptr, 0, &header); + rtp_header.sequenceNumber = 5; + rtp_video_stream_receiver_->OnReceivedPayloadData( + nullptr, 0, rtp_header, video_header, kVideoFrameDelta, absl::nullopt, + false); } TEST_F(RtpVideoStreamReceiverTest, RequestKeyframeIfFirstFrameIsDelta) { - WebRtcRTPHeader rtp_header = {}; + RTPHeader rtp_header; + RTPVideoHeader video_header; const std::vector data({1, 2, 3, 4}); - rtp_header.header.sequenceNumber = 1; - rtp_header.video_header().is_first_packet_in_frame = true; - rtp_header.video_header().is_last_packet_in_frame = true; - rtp_header.frameType = kVideoFrameDelta; - rtp_header.video_header().codec = kVideoCodecGeneric; + rtp_header.sequenceNumber = 1; + video_header.is_first_packet_in_frame = true; + video_header.is_last_packet_in_frame = true; + video_header.codec = kVideoCodecGeneric; EXPECT_CALL(mock_key_frame_request_sender_, RequestKeyFrame()); - rtp_video_stream_receiver_->OnReceivedPayloadData(data.data(), data.size(), - &rtp_header); + rtp_video_stream_receiver_->OnReceivedPayloadData( + data.data(), data.size(), rtp_header, video_header, kVideoFrameDelta, + absl::nullopt, false); } TEST_F(RtpVideoStreamReceiverTest, SecondarySinksGetRtpNotifications) {