Fixed problems in neteq when RTP and decoder timestamps increment with
different sample rate frequency.
BUG=webrtc:7327
Problems before the fix:
1. NetEqImpl::timestamp_ is inconsistent. Initially it is set to
the original RTP timestamp, but later gets updated with the
scaled timestamp.
2. NetEqImpl::InsertPacketInternal::main_timestamp is set with
the original RTP timestamp, but later gets compared with the
NetEqImpl::timestamp_ which may or may not be with the same
sample rate frequency and this results in major problems.
3. IncreaseEndTimestamp(main_timestamp - timestamp_) will be
incorrect when SSRC is changed and not the first packet.
4. delay_manager_->Update() may not be always invoked, since
the (main_timestamp - timestamp_) >= 0 will not be true when
the previous scaled timestamp_ is bigger than the main_timestamp
(current RTP timestamp) even if the current RTP timestamp is
bigger than the previous RTP timestamp.
5. delay_manager_->Update() parameters are main_timestamp
which increments with the RTP sample rate frequency and the
fs_hz_ which is the decoder sample rate frequency. When these
two frequencies are different as is the case with g.722, the
DelayManager::Update() will misfire and calculate incorrect
packet_len_ms and inter-arrival time (IAT) as a result. This
in effect will cause neteq to enter kPreemptiveExpand operation
and will keep expanding the jitter buffer even if the RTP packets
arrive with no jitter at all.
The fix corrects all these problems by making sure the
main_timestamp and the timestamp_ are always set with the scaled
timestamp and increment with the decoder sample rate frequency.
Review-Url: https://codereview.webrtc.org/2743063005
Cr-Commit-Position: refs/heads/master@{#17232}
diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl.cc b/webrtc/modules/audio_coding/neteq/neteq_impl.cc
index 786cb84..5015b7e 100644
--- a/webrtc/modules/audio_coding/neteq/neteq_impl.cc
+++ b/webrtc/modules/audio_coding/neteq/neteq_impl.cc
@@ -594,9 +594,27 @@
return packet;
}());
- bool update_sample_rate_and_channels = false;
+ bool update_sample_rate_and_channels = first_packet_ ||
+ (rtp_header.header.ssrc != ssrc_);
+
+ if (update_sample_rate_and_channels) {
+ // Reset timestamp scaling.
+ timestamp_scaler_->Reset();
+ }
+
+ if (!decoder_database_->IsRed(rtp_header.header.payloadType)) {
+ // Scale timestamp to internal domain (only for some codecs).
+ timestamp_scaler_->ToInternal(&packet_list);
+ }
+
+ // Store these for later use, since the first packet may very well disappear
+ // before we need these values.
+ uint32_t main_timestamp = packet_list.front().timestamp;
+ uint8_t main_payload_type = packet_list.front().payload_type;
+ uint16_t main_sequence_number = packet_list.front().sequence_number;
+
// Reinitialize NetEq if it's needed (changed SSRC or first call).
- if ((rtp_header.header.ssrc != ssrc_) || first_packet_) {
+ if (update_sample_rate_and_channels) {
// Note: |first_packet_| will be cleared further down in this method, once
// the packet has been successfully inserted into the packet buffer.
@@ -610,17 +628,10 @@
ssrc_ = rtp_header.header.ssrc;
// Update audio buffer timestamp.
- sync_buffer_->IncreaseEndTimestamp(rtp_header.header.timestamp -
- timestamp_);
+ sync_buffer_->IncreaseEndTimestamp(main_timestamp - timestamp_);
// Update codecs.
- timestamp_ = rtp_header.header.timestamp;
-
- // Reset timestamp scaling.
- timestamp_scaler_->Reset();
-
- // Trigger an update of sampling rate and the number of channels.
- update_sample_rate_and_channels = true;
+ timestamp_ = main_timestamp;
}
// Update RTCP statistics, only for regular packets.
@@ -652,14 +663,15 @@
}
RTC_DCHECK(!packet_list.empty());
- // Store these for later use, since the first packet may very well disappear
- // before we need these values.
- const uint32_t main_timestamp = packet_list.front().timestamp;
- const uint8_t main_payload_type = packet_list.front().payload_type;
- const uint16_t main_sequence_number = packet_list.front().sequence_number;
- // Scale timestamp to internal domain (only for some codecs).
- timestamp_scaler_->ToInternal(&packet_list);
+ // Update main_timestamp, if new packets appear in the list
+ // after RED splitting.
+ if (decoder_database_->IsRed(rtp_header.header.payloadType)) {
+ timestamp_scaler_->ToInternal(&packet_list);
+ main_timestamp = packet_list.front().timestamp;
+ main_payload_type = packet_list.front().payload_type;
+ main_sequence_number = packet_list.front().sequence_number;
+ }
// Process DTMF payloads. Cycle through the list of packets, and pick out any
// DTMF payloads found.