diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc index 3601d9b7db..25b7c56abd 100644 --- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -1306,143 +1306,6 @@ TEST_P(RtpSenderTest, SendFlexfecPackets) { EXPECT_EQ(kFlexFecSsrc, sent_flexfec_packet.Ssrc()); } -// TODO(ilnik): because of webrtc:7859. Once FEC moved below pacer, this test -// should be removed. -TEST_P(RtpSenderTest, NoFlexfecForTimingFrames) { - constexpr uint32_t kTimestamp = 1234; - const int64_t kCaptureTimeMs = fake_clock_.TimeInMilliseconds(); - constexpr int kMediaPayloadType = 127; - constexpr VideoCodecType kCodecType = VideoCodecType::kVideoCodecGeneric; - constexpr int kFlexfecPayloadType = 118; - const std::vector kNoRtpExtensions; - const std::vector kNoRtpExtensionSizes; - - FlexfecSender flexfec_sender(kFlexfecPayloadType, kFlexFecSsrc, kSsrc, kNoMid, - kNoRtpExtensions, kNoRtpExtensionSizes, - nullptr /* rtp_state */, &fake_clock_); - - // Reset |rtp_sender_| to use FlexFEC. - RtpRtcp::Configuration config; - config.clock = &fake_clock_; - config.outgoing_transport = &transport_; - config.paced_sender = &mock_paced_sender_; - config.flexfec_sender = &flexfec_sender; - config.event_log = &mock_rtc_event_log_; - config.send_packet_observer = &send_packet_observer_; - config.retransmission_rate_limiter = &retransmission_rate_limiter_; - config.local_media_ssrc = kSsrc; - rtp_sender_context_ = std::make_unique(config); - rtp_sender()->SetSequenceNumber(kSeqNum); - rtp_sender_context_->packet_history_.SetStorePacketsStatus( - RtpPacketHistory::StorageMode::kStoreAndCull, 10); - - PlayoutDelayOracle playout_delay_oracle; - FieldTrialBasedConfig field_trials; - RTPSenderVideo::Config video_config; - video_config.clock = &fake_clock_; - video_config.rtp_sender = rtp_sender(); - video_config.flexfec_sender = &flexfec_sender; - video_config.playout_delay_oracle = &playout_delay_oracle; - video_config.field_trials = &field_trials; - RTPSenderVideo rtp_sender_video(video_config); - - // Need extension to be registered for timing frames to be sent. - ASSERT_EQ(0, rtp_sender()->RegisterRtpHeaderExtension( - kRtpExtensionVideoTiming, kVideoTimingExtensionId)); - - // Parameters selected to generate a single FEC packet per media packet. - FecProtectionParams params; - params.fec_rate = 15; - params.max_fec_frames = 1; - params.fec_mask_type = kFecMaskRandom; - rtp_sender_video.SetFecParameters(params, params); - - RTPVideoHeader video_header; - video_header.video_timing.flags = VideoSendTiming::kTriggeredByTimer; - - EXPECT_CALL(mock_rtc_event_log_, - LogProxy(SameRtcEventTypeAs(RtcEvent::Type::RtpPacketOutgoing))) - .Times(1); - std::unique_ptr rtp_packet; - EXPECT_CALL( - mock_paced_sender_, - EnqueuePackets(Contains(AllOf( - Pointee(Property(&RtpPacketToSend::Ssrc, kSsrc)), - Pointee(Property(&RtpPacketToSend::SequenceNumber, kSeqNum)))))) - .WillOnce([&rtp_packet]( - std::vector> packets) { - EXPECT_EQ(packets.size(), 1u); - rtp_packet = std::move(packets[0]); - }); - - EXPECT_CALL(mock_paced_sender_, - EnqueuePackets(Contains( - Pointee(Property(&RtpPacketToSend::Ssrc, kFlexFecSsrc))))) - .Times(0); // Not called because packet should not be protected. - - video_header.frame_type = VideoFrameType::kVideoFrameKey; - EXPECT_TRUE(rtp_sender_video.SendVideo( - kMediaPayloadType, kCodecType, kTimestamp, kCaptureTimeMs, kPayloadData, - nullptr, video_header, kDefaultExpectedRetransmissionTimeMs)); - - rtp_egress()->SendPacket(rtp_packet.get(), PacedPacketInfo()); - - ASSERT_EQ(1, transport_.packets_sent()); - const RtpPacketReceived& sent_media_packet1 = transport_.sent_packets_[0]; - EXPECT_EQ(kMediaPayloadType, sent_media_packet1.PayloadType()); - EXPECT_EQ(kSeqNum, sent_media_packet1.SequenceNumber()); - EXPECT_EQ(kSsrc, sent_media_packet1.Ssrc()); - - // Now try to send not a timing frame. - uint16_t flexfec_seq_num; - - EXPECT_CALL(mock_rtc_event_log_, - LogProxy(SameRtcEventTypeAs(RtcEvent::Type::RtpPacketOutgoing))) - .Times(2); - std::unique_ptr media_packet2; - std::unique_ptr fec_packet; - - EXPECT_CALL(mock_paced_sender_, EnqueuePackets) - .WillOnce([&](std::vector> packets) { - for (auto& packet : packets) { - if (packet->packet_type() == RtpPacketToSend::Type::kVideo) { - EXPECT_EQ(packet->Ssrc(), kSsrc); - EXPECT_EQ(packet->SequenceNumber(), kSeqNum + 1); - media_packet2 = std::move(packet); - } else { - EXPECT_EQ(packet->packet_type(), - RtpPacketToSend::Type::kForwardErrorCorrection); - EXPECT_EQ(packet->Ssrc(), kFlexFecSsrc); - fec_packet = std::move(packet); - } - } - }); - - video_header.video_timing.flags = VideoSendTiming::kInvalid; - video_header.frame_type = VideoFrameType::kVideoFrameKey; - EXPECT_TRUE(rtp_sender_video.SendVideo( - kMediaPayloadType, kCodecType, kTimestamp + 1, kCaptureTimeMs + 1, - kPayloadData, nullptr, video_header, - kDefaultExpectedRetransmissionTimeMs)); - - ASSERT_TRUE(media_packet2 != nullptr); - ASSERT_TRUE(fec_packet != nullptr); - - flexfec_seq_num = fec_packet->SequenceNumber(); - rtp_egress()->SendPacket(media_packet2.get(), PacedPacketInfo()); - rtp_egress()->SendPacket(fec_packet.get(), PacedPacketInfo()); - - ASSERT_EQ(3, transport_.packets_sent()); - const RtpPacketReceived& sent_media_packet2 = transport_.sent_packets_[1]; - EXPECT_EQ(kMediaPayloadType, sent_media_packet2.PayloadType()); - EXPECT_EQ(kSeqNum + 1, sent_media_packet2.SequenceNumber()); - EXPECT_EQ(kSsrc, sent_media_packet2.Ssrc()); - const RtpPacketReceived& flexfec_packet = transport_.sent_packets_[2]; - EXPECT_EQ(kFlexfecPayloadType, flexfec_packet.PayloadType()); - EXPECT_EQ(flexfec_seq_num, flexfec_packet.SequenceNumber()); - EXPECT_EQ(kFlexFecSsrc, flexfec_packet.Ssrc()); -} - TEST_P(RtpSenderTestWithoutPacer, SendFlexfecPackets) { constexpr uint32_t kTimestamp = 1234; constexpr int kMediaPayloadType = 127; diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc index 236dd340c6..436962c38e 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -662,13 +662,6 @@ bool RTPSenderVideo::SendVideo( // Put packetization finish timestamp into extension. if (packet->HasExtension()) { packet->set_packetization_finish_time_ms(clock_->TimeInMilliseconds()); - // TODO(webrtc:10750): wait a couple of months and remove the statement - // below. For now we can't use packets with VideoTimingFrame extensions in - // Fec because the extension is modified after FEC is calculated by pacer - // and network. This may cause corruptions in video payload and header. - // The fix in receive code is implemented, but until all the receivers - // are updated, senders can't send potentially breaking packets. - protect_packet = false; } if (red_enabled()) {