diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc index 632088baec..51441f6ab8 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -39,6 +39,22 @@ constexpr size_t kRedForFecHeaderLength = 1; constexpr size_t kRtpSequenceNumberMapMaxEntries = 1 << 13; constexpr int64_t kMaxUnretransmittableFrameIntervalMs = 33 * 4; +// This is experimental field trial to exclude transport sequence number from +// FEC packets and should only be used in conjunction with datagram transport. +// Datagram transport removes transport sequence numbers from RTP packets and +// uses datagram feedback loop to re-generate RTCP feedback packets, but FEC +// contorol packets are calculated before sequence number is removed and as a +// result recovered packets will be corrupt unless we also remove transport +// sequence number during FEC calculation. +// +// TODO(sukhanov): We need to find find better way to implement FEC with +// datagram transport, probably moving FEC to datagram integration layter. We +// should also remove special field trial once we switch datagram path from +// RTCConfiguration flags to field trial and use the same field trial for FEC +// workaround. +const char kExcludeTransportSequenceNumberFromFecFieldTrial[] = + "WebRTC-ExcludeTransportSequenceNumberFromFec"; + void BuildRedPayload(const RtpPacketToSend& media_packet, RtpPacketToSend* red_packet) { uint8_t* red_payload = red_packet->AllocatePayload( @@ -212,7 +228,10 @@ RTPSenderVideo::RTPSenderVideo(Clock* clock, require_frame_encryption_(require_frame_encryption), generic_descriptor_auth_experiment_( field_trials.Lookup("WebRTC-GenericDescriptorAuth").find("Enabled") == - 0) { + 0), + exclude_transport_sequence_number_from_fec_experiment_( + field_trials.Lookup(kExcludeTransportSequenceNumberFromFecFieldTrial) + .find("Enabled") == 0) { RTC_DCHECK(playout_delay_oracle_); } @@ -277,6 +296,24 @@ void RTPSenderVideo::SendVideoPacketAsRedMaybeWithUlpfec( red_packet->SetPayloadType(red_payload_type_); if (ulpfec_enabled()) { if (protect_media_packet) { + if (exclude_transport_sequence_number_from_fec_experiment_) { + // See comments at the top of the file why experiment + // "WebRTC-kExcludeTransportSequenceNumberFromFec" is needed in + // conjunction with datagram transport. + // TODO(sukhanov): We may also need to implement it for flexfec_sender + // if we decide to keep this approach in the future. + uint16_t transport_senquence_number; + if (media_packet->GetExtension( + &transport_senquence_number)) { + if (!media_packet->RemoveExtension( + webrtc::TransportSequenceNumber::kId)) { + RTC_NOTREACHED() + << "Failed to remove transport sequence number, packet=" + << media_packet->ToString(); + } + } + } + ulpfec_generator_.AddRtpPacketAndGenerateFec( media_packet->data(), media_packet->payload_size(), media_packet->headers_size()); diff --git a/modules/rtp_rtcp/source/rtp_sender_video.h b/modules/rtp_rtcp/source/rtp_sender_video.h index 3555958e3c..2505fe544b 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.h +++ b/modules/rtp_rtcp/source/rtp_sender_video.h @@ -220,6 +220,8 @@ class RTPSenderVideo { bool require_frame_encryption_; // Set to true if the generic descriptor should be authenticated. const bool generic_descriptor_auth_experiment_; + + const bool exclude_transport_sequence_number_from_fec_experiment_; }; } // namespace webrtc diff --git a/pc/datagram_dtls_adaptor.cc b/pc/datagram_dtls_adaptor.cc index e1added653..0b47078274 100644 --- a/pc/datagram_dtls_adaptor.cc +++ b/pc/datagram_dtls_adaptor.cc @@ -92,6 +92,11 @@ DatagramDtlsAdaptor::DatagramDtlsAdaptor( "datagram transport connection"; } + // TODO(sukhanov): Add CHECK to make sure that field trial + // WebRTC-ExcludeTransportSequenceNumberFromFecFieldTrial is enabled. + // If feedback loop is translation is enabled, FEC packets must exclude + // transport sequence numbers, otherwise recovered packets will be corrupt. + RTC_DCHECK(ice_transport_); RTC_DCHECK(datagram_transport_); ConnectToIceTransport();