Fix potential race in PacketSequencer.
The race can happen when an encoder thread is packetizing a video frame
and is calling RTPSender::AssignSequenceNumber() while the RtpRtcp
module is calling GeneratePadding() and querying
PacketSequencer::CanSendPaddingOnMediaSsrc().
The solution for now is to simply not call
PacketSequencer::CanSendPaddingOnMediaSsrc() from the RtpRtcp module,
as that parameter will be ignored anyway - RTPSender will query that
method internally while holding the send lock.
Once deferred sequencing is implemented, the
can_send_padding_on_media_ssrc parameter can be populated safely since
it is then always called on the pacer thread.
Bug: webrtc:11340, webrtc:12470
Change-Id: I9e90808166453d0e29746df89044e1d3bdffa286
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/227767
Reviewed-by: Tommi <tommi@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34655}
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
index 52a16ec..f3e7e30 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
@@ -444,9 +444,12 @@
std::vector<std::unique_ptr<RtpPacketToSend>>
ModuleRtpRtcpImpl::GeneratePadding(size_t target_size_bytes) {
RTC_DCHECK(rtp_sender_);
+ // `can_send_padding_on_media_ssrc` set to false but is ignored at this
+ // point, RTPSender will internally query `sequencer_` while holding the
+ // send lock.
return rtp_sender_->packet_generator.GeneratePadding(
target_size_bytes, rtp_sender_->packet_sender.MediaHasBeenSent(),
- rtp_sender_->sequencer_.CanSendPaddingOnMediaSsrc());
+ /*can_send_padding_on_media_ssrc=*/false);
}
std::vector<RtpSequenceNumberMap::Info>
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc
index 38fbf45..af64ddb 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc
@@ -398,9 +398,12 @@
std::vector<std::unique_ptr<RtpPacketToSend>>
ModuleRtpRtcpImpl2::GeneratePadding(size_t target_size_bytes) {
RTC_DCHECK(rtp_sender_);
+ // `can_send_padding_on_media_ssrc` set to false but is ignored at this
+ // point, RTPSender will internally query `sequencer_` while holding the
+ // send lock.
return rtp_sender_->packet_generator.GeneratePadding(
target_size_bytes, rtp_sender_->packet_sender.MediaHasBeenSent(),
- rtp_sender_->sequencer_.CanSendPaddingOnMediaSsrc());
+ /*can_send_padding_on_media_ssrc=*/false);
}
std::vector<RtpSequenceNumberMap::Info>
