admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "Batch assign RTP seq# for all packets of a frame."

Browse Source 
This reverts commit 5cc99570620890edc3989b2cae1d1ee0669a021c.

Reason for revert: Seems this CL breaks the below test when being imported in google3
https://webrtc-review.googlesource.com/c/src/+/207867

Original change's description:
> Batch assign RTP seq# for all packets of a frame.
>
> This avoids a potential race where other call sites could assign
> sequence numbers while the video frame is mid packetization - resulting
> in a non-contiguous video sequence.
>
> Avoiding the tight lock-unlock within the loop also couldn't hurt from
> a performance standpoint.
>
> Bug: webrtc:12448
> Change-Id: I6cc31c7743d2ca75caeaeffb98651a480dbe08e2
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/207867
> Commit-Queue: Erik Språng <sprang@webrtc.org>
> Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#33291}

Bug: webrtc:12448
Change-Id: I2547f946a5ba75aa09cdbfd902157011425d1c30
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/208220
Reviewed-by: Jeremy Leconte <jleconte@google.com>
Commit-Queue: Jeremy Leconte <jleconte@google.com>
Cr-Commit-Position: refs/heads/master@{#33294}
This commit is contained in:
Jeremy Leconte 2021-02-18 08:53:51 +00:00 committed by Commit Bot
parent e11b4aef3f
commit 17f914ce50
3 changed files with 9 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
modules/rtp_rtcp/source/rtp_sender.cc
View File

@ -47,9 +47,6 @@ constexpr uint32_t kTimestampTicksPerMs = 90;
// Min size needed to get payload padding from packet history.
constexpr int kMinPayloadPaddingBytes = 50;
// RED header if first byte of payload.
constexpr size_t kRedForFecHeaderLength = 1;
template <typename Extension>
constexpr RtpExtensionSize CreateExtensionSize() {
return {Extension::kId, Extension::kValueSizeBytes};
@ -614,40 +611,16 @@ bool RTPSender::AssignSequenceNumber(RtpPacketToSend* packet) {
RTC_DCHECK(packet->Ssrc() == ssrc_);
packet->SetSequenceNumber(sequence_number_++);
UpdateLastPacketState(*packet);
return true;
}
bool RTPSender::AssignSequenceNumbersAndStoreLastPacketState(
rtc::ArrayView<std::unique_ptr<RtpPacketToSend>> packets) {
RTC_DCHECK(!packets.empty());
MutexLock lock(&send_mutex_);
if (!sending_media_)
return false;
for (auto& packet : packets) {
RTC_DCHECK_EQ(packet->Ssrc(), ssrc_);
packet->SetSequenceNumber(sequence_number_++);
}
UpdateLastPacketState(**packets.rbegin());
return true;
}
void RTPSender::UpdateLastPacketState(const RtpPacketToSend& packet) {
// Remember marker bit to determine if padding can be inserted with
// sequence number following |packet|.
last_packet_marker_bit_ = packet.Marker();
// Remember media payload type to use in the padding packet if rtx is
// disabled.
if (packet.is_red()) {
RTC_DCHECK_GE(packet.payload_size(), kRedForFecHeaderLength);
last_payload_type_ = packet.PayloadBuffer()[0];
} else {
last_payload_type_ = packet.PayloadType();
}
last_packet_marker_bit_ = packet->Marker();
// Remember payload type to use in the padding packet if rtx is disabled.
last_payload_type_ = packet->PayloadType();
// Save timestamps to generate timestamp field and extensions for the padding.
last_rtp_timestamp_ = packet.Timestamp();
last_rtp_timestamp_ = packet->Timestamp();
last_timestamp_time_ms_ = clock_->TimeInMilliseconds();
capture_time_ms_ = packet.capture_time_ms();
capture_time_ms_ = packet->capture_time_ms();
return true;
}
void RTPSender::SetSendingMediaStatus(bool enabled) {

8
modules/rtp_rtcp/source/rtp_sender.h
View File

@ -137,11 +137,6 @@ class RTPSender {
// Return false if sending was turned off.
bool AssignSequenceNumber(RtpPacketToSend* packet)
RTC_LOCKS_EXCLUDED(send_mutex_);
// Same as AssignSequenceNumber(), but applies sequence numbers atomically to
// a batch of packets.
bool AssignSequenceNumbersAndStoreLastPacketState(
rtc::ArrayView<std::unique_ptr<RtpPacketToSend>> packets)
RTC_LOCKS_EXCLUDED(send_mutex_);
// Maximum header overhead per fec/padding packet.
size_t FecOrPaddingPacketMaxRtpHeaderLength() const
RTC_LOCKS_EXCLUDED(send_mutex_);
@ -184,9 +179,6 @@ class RTPSender {
void UpdateHeaderSizes() RTC_EXCLUSIVE_LOCKS_REQUIRED(send_mutex_);
void UpdateLastPacketState(const RtpPacketToSend& packet)
RTC_EXCLUSIVE_LOCKS_REQUIRED(send_mutex_);
Clock* const clock_;
Random random_ RTC_GUARDED_BY(send_mutex_);

9
modules/rtp_rtcp/source/rtp_sender_video.cc
View File

@ -648,6 +648,8 @@ bool RTPSenderVideo::SendVideo(
if (!packetizer->NextPacket(packet.get()))
return false;
RTC_DCHECK_LE(packet->payload_size(), expected_payload_capacity);
if (!rtp_sender_->AssignSequenceNumber(packet.get()))
return false;
packet->set_allow_retransmission(allow_retransmission);
packet->set_is_key_frame(video_header.frame_type ==
@ -668,7 +670,7 @@ bool RTPSenderVideo::SendVideo(
red_packet->SetPayloadType(*red_payload_type_);
red_packet->set_is_red(true);
// Append |red_packet| instead of |packet| to output.
// Send |red_packet| instead of |packet| for allocated sequence number.
red_packet->set_packet_type(RtpPacketMediaType::kVideo);
red_packet->set_allow_retransmission(packet->allow_retransmission());
rtp_packets.emplace_back(std::move(red_packet));
@ -689,11 +691,6 @@ bool RTPSenderVideo::SendVideo(
}
}
if (!rtp_sender_->AssignSequenceNumbersAndStoreLastPacketState(rtp_packets)) {
// Media not being sent.
return false;
}
LogAndSendToNetwork(std::move(rtp_packets), payload.size());
// Update details about the last sent frame.