Index: modules/rtp_rtcp/source/forward_error_correction.cc |
diff --git a/modules/rtp_rtcp/source/forward_error_correction.cc b/modules/rtp_rtcp/source/forward_error_correction.cc |
index fe348b5e3366f9f5cc93dfc73c23e42573d14d00..3c1b1b4154ef4f65687f8f41a7bb89c59687b7e2 100644 |
--- a/modules/rtp_rtcp/source/forward_error_correction.cc |
+++ b/modules/rtp_rtcp/source/forward_error_correction.cc |
@@ -342,16 +342,14 @@ void ForwardErrorCorrection::ResetState( |
void ForwardErrorCorrection::InsertMediaPacket( |
RecoveredPacketList* recovered_packets, |
- ReceivedPacket* received_packet) { |
- RTC_DCHECK_EQ(received_packet->ssrc, protected_media_ssrc_); |
+ const ReceivedPacket& received_packet) { |
+ RTC_DCHECK_EQ(received_packet.ssrc, protected_media_ssrc_); |
// Search for duplicate packets. |
for (const auto& recovered_packet : *recovered_packets) { |
- RTC_DCHECK_EQ(recovered_packet->ssrc, received_packet->ssrc); |
- if (recovered_packet->seq_num == received_packet->seq_num) { |
+ RTC_DCHECK_EQ(recovered_packet->ssrc, received_packet.ssrc); |
+ if (recovered_packet->seq_num == received_packet.seq_num) { |
// Duplicate packet, no need to add to list. |
- // Delete duplicate media packet data. |
- received_packet->pkt = nullptr; |
return; |
} |
} |
@@ -361,10 +359,10 @@ void ForwardErrorCorrection::InsertMediaPacket( |
recovered_packet->was_recovered = false; |
// This media packet has already been passed on. |
recovered_packet->returned = true; |
- recovered_packet->ssrc = received_packet->ssrc; |
- recovered_packet->seq_num = received_packet->seq_num; |
- recovered_packet->pkt = received_packet->pkt; |
- recovered_packet->pkt->length = received_packet->pkt->length; |
+ recovered_packet->ssrc = received_packet.ssrc; |
+ recovered_packet->seq_num = received_packet.seq_num; |
+ recovered_packet->pkt = received_packet.pkt; |
+ recovered_packet->pkt->length = received_packet.pkt->length; |
// TODO(holmer): Consider replacing this with a binary search for the right |
// position, and then just insert the new packet. Would get rid of the sort. |
RecoveredPacket* recovered_packet_ptr = recovered_packet.get(); |
@@ -390,23 +388,22 @@ void ForwardErrorCorrection::UpdateCoveringFecPackets( |
void ForwardErrorCorrection::InsertFecPacket( |
const RecoveredPacketList& recovered_packets, |
- ReceivedPacket* received_packet) { |
- RTC_DCHECK_EQ(received_packet->ssrc, ssrc_); |
+ const ReceivedPacket& received_packet) { |
+ RTC_DCHECK_EQ(received_packet.ssrc, ssrc_); |
// Check for duplicate. |
for (const auto& existing_fec_packet : received_fec_packets_) { |
- RTC_DCHECK_EQ(existing_fec_packet->ssrc, received_packet->ssrc); |
- if (existing_fec_packet->seq_num == received_packet->seq_num) { |
- // Delete duplicate FEC packet data. |
- received_packet->pkt = nullptr; |
+ RTC_DCHECK_EQ(existing_fec_packet->ssrc, received_packet.ssrc); |
+ if (existing_fec_packet->seq_num == received_packet.seq_num) { |
+ // Drop duplicate FEC packet data. |
return; |
} |
} |
std::unique_ptr<ReceivedFecPacket> fec_packet(new ReceivedFecPacket()); |
- fec_packet->pkt = received_packet->pkt; |
- fec_packet->ssrc = received_packet->ssrc; |
- fec_packet->seq_num = received_packet->seq_num; |
+ fec_packet->pkt = received_packet.pkt; |
+ fec_packet->ssrc = received_packet.ssrc; |
+ fec_packet->seq_num = received_packet.seq_num; |
// Parse ULPFEC/FlexFEC header specific info. |
bool ret = fec_header_reader_->ReadFecHeader(fec_packet.get()); |
if (!ret) { |
@@ -483,49 +480,46 @@ void ForwardErrorCorrection::AssignRecoveredPackets( |
} |
} |
-void ForwardErrorCorrection::InsertPackets( |
- ReceivedPacketList* received_packets, |
+void ForwardErrorCorrection::InsertPacket( |
+ const ReceivedPacket& received_packet, |
RecoveredPacketList* recovered_packets) { |
- while (!received_packets->empty()) { |
- ReceivedPacket* received_packet = received_packets->front().get(); |
- |
- // Discard old FEC packets such that the sequence numbers in |
- // |received_fec_packets_| span at most 1/2 of the sequence number space. |
- // This is important for keeping |received_fec_packets_| sorted, and may |
- // also reduce the possibility of incorrect decoding due to sequence number |
- // wrap-around. |
- // TODO(marpan/holmer): We should be able to improve detection/discarding of |
- // old FEC packets based on timestamp information or better sequence number |
- // thresholding (e.g., to distinguish between wrap-around and reordering). |
- if (!received_fec_packets_.empty() && |
- received_packet->ssrc == received_fec_packets_.front()->ssrc) { |
- // It only makes sense to detect wrap-around when |received_packet| |
- // and |front_received_fec_packet| belong to the same sequence number |
- // space, i.e., the same SSRC. This happens when |received_packet| |
- // is a FEC packet, or if |received_packet| is a media packet and |
- // RED+ULPFEC is used. |
- auto it = received_fec_packets_.begin(); |
- while (it != received_fec_packets_.end()) { |
- uint16_t seq_num_diff = abs(static_cast<int>(received_packet->seq_num) - |
- static_cast<int>((*it)->seq_num)); |
- if (seq_num_diff > 0x3fff) { |
- it = received_fec_packets_.erase(it); |
- } else { |
- // No need to keep iterating, since |received_fec_packets_| is sorted. |
- break; |
- } |
+ // Discard old FEC packets such that the sequence numbers in |
+ // |received_fec_packets_| span at most 1/2 of the sequence number space. |
+ // This is important for keeping |received_fec_packets_| sorted, and may |
+ // also reduce the possibility of incorrect decoding due to sequence number |
+ // wrap-around. |
+ // TODO(marpan/holmer): We should be able to improve detection/discarding of |
+ // old FEC packets based on timestamp information or better sequence number |
+ // thresholding (e.g., to distinguish between wrap-around and reordering). |
+ if (!received_fec_packets_.empty() && |
+ received_packet.ssrc == received_fec_packets_.front()->ssrc) { |
+ // It only makes sense to detect wrap-around when |received_packet| |
+ // and |front_received_fec_packet| belong to the same sequence number |
+ // space, i.e., the same SSRC. This happens when |received_packet| |
+ // is a FEC packet, or if |received_packet| is a media packet and |
+ // RED+ULPFEC is used. |
+ auto it = received_fec_packets_.begin(); |
+ while (it != received_fec_packets_.end()) { |
+ // TODO(nisse): This handling of wraparound appears broken, should be |
+ // static_cast<int16_t>( |
+ // received_packet.seq_num - back_recovered_packet->seq_num) |
+ uint16_t seq_num_diff = abs(static_cast<int>(received_packet.seq_num) - |
+ static_cast<int>((*it)->seq_num)); |
+ if (seq_num_diff > 0x3fff) { |
+ it = received_fec_packets_.erase(it); |
+ } else { |
+ // No need to keep iterating, since |received_fec_packets_| is sorted. |
+ break; |
} |
} |
+ } |
- if (received_packet->is_fec) { |
- InsertFecPacket(*recovered_packets, received_packet); |
- } else { |
- InsertMediaPacket(recovered_packets, received_packet); |
- } |
- // Delete the received packet "wrapper". |
- received_packets->pop_front(); |
+ if (received_packet.is_fec) { |
+ InsertFecPacket(*recovered_packets, received_packet); |
+ } else { |
+ InsertMediaPacket(recovered_packets, received_packet); |
} |
- RTC_DCHECK(received_packets->empty()); |
+ |
DiscardOldRecoveredPackets(recovered_packets); |
} |
@@ -716,38 +710,35 @@ uint32_t ForwardErrorCorrection::ParseSsrc(uint8_t* packet) { |
return (packet[8] << 24) + (packet[9] << 16) + (packet[10] << 8) + packet[11]; |
} |
-int ForwardErrorCorrection::DecodeFec( |
- ReceivedPacketList* received_packets, |
- RecoveredPacketList* recovered_packets) { |
- RTC_DCHECK(received_packets); |
+void ForwardErrorCorrection::DecodeFec(const ReceivedPacket& received_packet, |
+ RecoveredPacketList* recovered_packets) { |
RTC_DCHECK(recovered_packets); |
const size_t max_media_packets = fec_header_reader_->MaxMediaPackets(); |
if (recovered_packets->size() == max_media_packets) { |
const RecoveredPacket* back_recovered_packet = |
recovered_packets->back().get(); |
- for (const auto& received_packet : *received_packets) { |
- if (received_packet->ssrc == back_recovered_packet->ssrc) { |
- const unsigned int seq_num_diff = |
- abs(static_cast<int>(received_packet->seq_num) - |
- static_cast<int>(back_recovered_packet->seq_num)); |
- if (seq_num_diff > max_media_packets) { |
- // A big gap in sequence numbers. The old recovered packets |
- // are now useless, so it's safe to do a reset. |
- LOG(LS_INFO) << "Big gap in media/ULPFEC sequence numbers. No need " |
- "to keep the old packets in the FEC buffers, thus " |
- "resetting them."; |
- ResetState(recovered_packets); |
- break; |
- } |
+ |
+ if (received_packet.ssrc == back_recovered_packet->ssrc) { |
+ // TODO(nisse): This handling of wraparound appears broken, should be |
+ // static_cast<int16_t>( |
+ // received_packet.seq_num - back_recovered_packet->seq_num) |
+ const unsigned int seq_num_diff = |
+ abs(static_cast<int>(received_packet.seq_num) - |
+ static_cast<int>(back_recovered_packet->seq_num)); |
+ if (seq_num_diff > max_media_packets) { |
+ // A big gap in sequence numbers. The old recovered packets |
+ // are now useless, so it's safe to do a reset. |
+ LOG(LS_INFO) << "Big gap in media/ULPFEC sequence numbers. No need " |
+ "to keep the old packets in the FEC buffers, thus " |
+ "resetting them."; |
+ ResetState(recovered_packets); |
} |
} |
} |
- InsertPackets(received_packets, recovered_packets); |
+ InsertPacket(received_packet, recovered_packets); |
AttemptRecovery(recovered_packets); |
- |
- return 0; |
} |
size_t ForwardErrorCorrection::MaxPacketOverhead() const { |