webrtc/modules/audio_coding/neteq/neteq_impl.cc - Issue 2411183003: Removed RTPHeader from NetEq's Packet struct.

Unified Diff: webrtc/modules/audio_coding/neteq/neteq_impl.cc

Issue 2411183003: Removed RTPHeader from NetEq's Packet struct. (Closed)

Patch Set: Created 4 years, 2 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

« webrtc/modules/audio_coding/neteq/decision_logic_normal.h ('K') | « webrtc/modules/audio_coding/neteq/mock/mock_packet_buffer.h ('k') | webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc » ('j') | webrtc/modules/audio_coding/neteq/packet.h » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: webrtc/modules/audio_coding/neteq/neteq_impl.cc

diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl.cc b/webrtc/modules/audio_coding/neteq/neteq_impl.cc

index b60f1b86e58a1cf05dd97eb3511edb607a8867fa..afc363d89bdd5cfe6194f6bdef643e04149d4232 100644

--- a/webrtc/modules/audio_coding/neteq/neteq_impl.cc

+++ b/webrtc/modules/audio_coding/neteq/neteq_impl.cc

@@ -581,48 +581,43 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header,

}

PacketList packet_list;

- RTPHeader main_header;

{

// Convert to Packet.

// Create |packet| within this separate scope, since it should not be used

// directly once it's been inserted in the packet list. This way, |packet|

// is not defined outside of this block.

Packet* packet = new Packet;

- packet->header.markerBit = false;

- packet->header.payloadType = rtp_header.header.payloadType;

- packet->header.sequenceNumber = rtp_header.header.sequenceNumber;

- packet->header.timestamp = rtp_header.header.timestamp;

- packet->header.ssrc = rtp_header.header.ssrc;

- packet->header.numCSRCs = 0;

+ packet->payloadType = rtp_header.header.payloadType;

+ packet->sequenceNumber = rtp_header.header.sequenceNumber;

+ packet->timestamp = rtp_header.header.timestamp;

packet->payload.SetData(payload.data(), payload.size());

// Waiting time will be set upon inserting the packet in the buffer.

RTC_DCHECK(!packet->waiting_time);

// Insert packet in a packet list.

packet_list.push_back(packet);

- // Save main payloads header for later.

- memcpy(&main_header, &packet->header, sizeof(main_header));

}

bool update_sample_rate_and_channels = false;

// Reinitialize NetEq if it's needed (changed SSRC or first call).

- if ((main_header.ssrc != ssrc_) || first_packet_) {

+ if ((rtp_header.header.ssrc != ssrc_) || first_packet_) {

// Note: |first_packet_| will be cleared further down in this method, once

// the packet has been successfully inserted into the packet buffer.

- rtcp_.Init(main_header.sequenceNumber);

+ rtcp_.Init(rtp_header.header.sequenceNumber);

// Flush the packet buffer and DTMF buffer.

packet_buffer_->Flush();

dtmf_buffer_->Flush();

// Store new SSRC.

- ssrc_ = main_header.ssrc;

+ ssrc_ = rtp_header.header.ssrc;

// Update audio buffer timestamp.

- sync_buffer_->IncreaseEndTimestamp(main_header.timestamp - timestamp_);

+ sync_buffer_->IncreaseEndTimestamp(rtp_header.header.timestamp -

+ timestamp_);

// Update codecs.

- timestamp_ = main_header.timestamp;

+ timestamp_ = rtp_header.header.timestamp;

// Reset timestamp scaling.

timestamp_scaler_->Reset();

@@ -632,10 +627,19 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header,

}

// Update RTCP statistics, only for regular packets.

- rtcp_.Update(main_header, receive_timestamp);

+ rtcp_.Update(rtp_header.header, receive_timestamp);

+ if (nack_enabled_) {

+ RTC_DCHECK(nack_);

+ if (update_sample_rate_and_channels) {

+ nack_->Reset();

+ }

+ nack_->UpdateLastReceivedPacket(rtp_header.header.sequenceNumber,

+ rtp_header.header.timestamp);

+ }

// Check for RED payload type, and separate payloads into several packets.

- if (decoder_database_->IsRed(main_header.payloadType)) {

+ if (decoder_database_->IsRed(rtp_header.header.payloadType)) {

if (!red_payload_splitter_->SplitRed(&packet_list)) {

PacketBuffer::DeleteAllPackets(&packet_list);

return kRedundancySplitError;

@@ -643,9 +647,7 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header,

// Only accept a few RED payloads of the same type as the main data,

// DTMF events and CNG.

red_payload_splitter_->CheckRedPayloads(&packet_list, *decoder_database_);

- // Update the stored main payload header since the main payload has now

- // changed.

- memcpy(&main_header, &packet_list.front()->header, sizeof(main_header));

+ // Update the main packet pointer since the main payload has now changed.

hlundin-webrtc 2016/10/12 20:39:57 You are not updating the main packet pointer here.

ossu 2016/10/12 21:46:02 Heh, that's true!

}

// Check payload types.

@@ -655,6 +657,11 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header,

return kUnknownRtpPayloadType;

}

+ RTC_DCHECK(!packet_list.empty());

+ const uint32_t main_timestamp = packet_list.front()->timestamp;

+ const uint8_t main_payload_type = packet_list.front()->payloadType;

+ const uint16_t main_sequence_number = packet_list.front()->sequenceNumber;

// Scale timestamp to internal domain (only for some codecs).

timestamp_scaler_->ToInternal(&packet_list);

@@ -665,9 +672,9 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header,

Packet* current_packet = (*it);

assert(current_packet);

assert(!current_packet->payload.empty());

- if (decoder_database_->IsDtmf(current_packet->header.payloadType)) {

+ if (decoder_database_->IsDtmf(current_packet->payloadType)) {

DtmfEvent event;

- int ret = DtmfBuffer::ParseEvent(current_packet->header.timestamp,

+ int ret = DtmfBuffer::ParseEvent(current_packet->timestamp,

current_packet->payload.data(),

current_packet->payload.size(), &event);

if (ret != DtmfBuffer::kOK) {

@@ -687,16 +694,15 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header,

// Update bandwidth estimate, if the packet is not comfort noise.

if (!packet_list.empty() &&

- !decoder_database_->IsComfortNoise(main_header.payloadType)) {

+ !decoder_database_->IsComfortNoise(main_payload_type)) {

// The list can be empty here if we got nothing but DTMF payloads.

- AudioDecoder* decoder =

- decoder_database_->GetDecoder(main_header.payloadType);

- assert(decoder); // Should always get a valid object, since we have

- // already checked that the payload types are known.

+ AudioDecoder* decoder = decoder_database_->GetDecoder(main_payload_type);

+ RTC_DCHECK(decoder); // Should always get a valid object, since we have

+ // already checked that the payload types are known.

decoder->IncomingPacket(packet_list.front()->payload.data(),

packet_list.front()->payload.size(),

- packet_list.front()->header.sequenceNumber,

- packet_list.front()->header.timestamp,

+ packet_list.front()->sequenceNumber,

+ packet_list.front()->timestamp,

receive_timestamp);

}

@@ -705,7 +711,7 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header,

std::unique_ptr<Packet> packet(packet_list.front());

packet_list.pop_front();

const DecoderDatabase::DecoderInfo* info =

- decoder_database_->GetDecoderInfo(packet->header.payloadType);

+ decoder_database_->GetDecoderInfo(packet->payloadType);

if (!info) {

LOG(LS_WARNING) << "SplitAudio unknown payload type";

return kUnknownRtpPayloadType;

@@ -717,17 +723,19 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header,

} else {

std::vector<AudioDecoder::ParseResult> results =

info->GetDecoder()->ParsePayload(std::move(packet->payload),

- packet->header.timestamp);

- const RTPHeader& original_header = packet->header;

+ packet->timestamp);

+ const auto sequenceNumber = packet->sequenceNumber;

+ const auto payloadType = packet->payloadType;

const Packet::Priority original_priority = packet->priority;

for (auto& result : results) {

RTC_DCHECK(result.frame);

// Reuse the packet if possible.

if (!packet) {

packet.reset(new Packet);

- packet->header = original_header;

+ packet->sequenceNumber = sequenceNumber;

+ packet->payloadType = payloadType;

}

- packet->header.timestamp = result.timestamp;

+ packet->timestamp = result.timestamp;

RTC_DCHECK_GE(result.priority, 0);

packet->priority.codec_level = result.priority;

packet->priority.red_level = original_priority.red_level;

@@ -737,16 +745,6 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header,

}

- if (nack_enabled_) {

- RTC_DCHECK(nack_);

- if (update_sample_rate_and_channels) {

- nack_->Reset();

- }

- nack_->UpdateLastReceivedPacket(

- parsed_packet_list.front()->header.sequenceNumber,

- parsed_packet_list.front()->header.timestamp);

- }

// Insert packets in buffer.

const size_t buffer_length_before_insert =

packet_buffer_->NumPacketsInBuffer();

@@ -781,9 +779,9 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header,

// CNG packet with a sample rate different than the current CNG then it

// flushes its buffer, assuming send codec must have been changed. However,

// payload type of the hypothetically new send codec is not known.

- const RTPHeader* rtp_header = packet_buffer_->NextRtpHeader();

- assert(rtp_header);

- int payload_type = rtp_header->payloadType;

+ const Packet* next_packet = packet_buffer_->PeekNextPacket();

+ RTC_DCHECK(next_packet);

+ const int payload_type = next_packet->payloadType;

size_t channels = 1;

if (!decoder_database_->IsComfortNoise(payload_type)) {

AudioDecoder* decoder = decoder_database_->GetDecoder(payload_type);

@@ -807,7 +805,7 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header,

// TODO(hlundin): Move this code to DelayManager class.

const DecoderDatabase::DecoderInfo* dec_info =

- decoder_database_->GetDecoderInfo(main_header.payloadType);

+ decoder_database_->GetDecoderInfo(main_payload_type);

assert(dec_info); // Already checked that the payload type is known.

delay_manager_->LastDecodedWasCngOrDtmf(dec_info->IsComfortNoise() ||

dec_info->IsDtmf());

@@ -828,12 +826,10 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header,

}

// Update statistics.

- if ((int32_t) (main_header.timestamp - timestamp_) >= 0 &&

- !new_codec_) {

+ if ((int32_t)(main_timestamp - timestamp_) >= 0 && !new_codec_) {

// Only update statistics if incoming packet is not older than last played

// out packet, and if new codec flag is not set.

- delay_manager_->Update(main_header.sequenceNumber, main_header.timestamp,

- fs_hz_);

+ delay_manager_->Update(main_sequence_number, main_timestamp, fs_hz_);

}

} else if (delay_manager_->last_pack_cng_or_dtmf() == -1) {

// This is first "normal" packet after CNG or DTMF.

@@ -1091,7 +1087,7 @@ int NetEqImpl::GetDecision(Operations* operation,

const uint32_t five_seconds_samples = 5 * fs_hz_;

packet_buffer_->DiscardOldPackets(end_timestamp, five_seconds_samples);

}

- const RTPHeader* header = packet_buffer_->NextRtpHeader();

+ const Packet* packet = packet_buffer_->PeekNextPacket();

RTC_DCHECK(!generated_noise_stopwatch_ ||

generated_noise_stopwatch_->ElapsedTicks() >= 1);

@@ -1106,9 +1102,9 @@ int NetEqImpl::GetDecision(Operations* operation,

// Because of timestamp peculiarities, we have to "manually" disallow using

// a CNG packet with the same timestamp as the one that was last played.

// This can happen when using redundancy and will cause the timing to shift.

- while (header && decoder_database_->IsComfortNoise(header->payloadType) &&

- (end_timestamp >= header->timestamp ||

- end_timestamp + generated_noise_samples > header->timestamp)) {

+ while (packet && decoder_database_->IsComfortNoise(packet->payloadType) &&

+ (end_timestamp >= packet->timestamp ||

+ end_timestamp + generated_noise_samples > packet->timestamp)) {

// Don't use this packet, discard it.

if (packet_buffer_->DiscardNextPacket() != PacketBuffer::kOK) {

assert(false); // Must be ok by design.

@@ -1117,7 +1113,7 @@ int NetEqImpl::GetDecision(Operations* operation,

if (!new_codec_) {

packet_buffer_->DiscardOldPackets(end_timestamp, 5 * fs_hz_);

}

- header = packet_buffer_->NextRtpHeader();

+ packet = packet_buffer_->PeekNextPacket();

}

@@ -1150,7 +1146,7 @@ int NetEqImpl::GetDecision(Operations* operation,

decision_logic_->noise_fast_forward()

: 0;

*operation = decision_logic_->GetDecision(

- *sync_buffer_, *expand_, decoder_frame_length_, header, last_mode_,

+ *sync_buffer_, *expand_, decoder_frame_length_, packet, last_mode_,

*play_dtmf, generated_noise_samples, &reset_decoder_);

// Check if we already have enough samples in the |sync_buffer_|. If so,

@@ -1171,16 +1167,16 @@ int NetEqImpl::GetDecision(Operations* operation,

if (new_codec_ || *operation == kUndefined) {

// The only valid reason to get kUndefined is that new_codec_ is set.

assert(new_codec_);

- if (*play_dtmf && !header) {

+ if (*play_dtmf && !packet) {

timestamp_ = dtmf_event->timestamp;

} else {

- if (!header) {

+ if (!packet) {

LOG(LS_ERROR) << "Packet missing where it shouldn't.";

return -1;

}

- timestamp_ = header->timestamp;

+ timestamp_ = packet->timestamp;

if (*operation == kRfc3389CngNoPacket &&

- decoder_database_->IsComfortNoise(header->payloadType)) {

+ decoder_database_->IsComfortNoise(packet->payloadType)) {

// Change decision to CNG packet, since we do have a CNG packet, but it

// was considered too early to use. Now, use it anyway.

*operation = kRfc3389Cng;

@@ -1294,18 +1290,17 @@ int NetEqImpl::GetDecision(Operations* operation,

// Get packets from buffer.

int extracted_samples = 0;

- if (header &&

- *operation != kAlternativePlc &&

+ if (packet && *operation != kAlternativePlc &&

*operation != kAlternativePlcIncreaseTimestamp &&

*operation != kAudioRepetition &&

*operation != kAudioRepetitionIncreaseTimestamp) {

- sync_buffer_->IncreaseEndTimestamp(header->timestamp - end_timestamp);

+ sync_buffer_->IncreaseEndTimestamp(packet->timestamp - end_timestamp);

if (decision_logic_->CngOff()) {

// Adjustment of timestamp only corresponds to an actual packet loss

// if comfort noise is not played. If comfort noise was just played,

// this adjustment of timestamp is only done to get back in sync with the

// stream timestamp; no loss to report.

- stats_.LostSamples(header->timestamp - end_timestamp);

+ stats_.LostSamples(packet->timestamp - end_timestamp);

}

if (*operation != kRfc3389Cng) {

@@ -1349,7 +1344,7 @@ int NetEqImpl::Decode(PacketList* packet_list, Operations* operation,

if (!packet_list->empty()) {

const Packet* packet = packet_list->front();

- uint8_t payload_type = packet->header.payloadType;

+ uint8_t payload_type = packet->payloadType;

if (!decoder_database_->IsComfortNoise(payload_type)) {

decoder = decoder_database_->GetDecoder(payload_type);

assert(decoder);

@@ -1485,8 +1480,7 @@ int NetEqImpl::DecodeLoop(PacketList* packet_list, const Operations& operation,

}

// Do decoding.

- while (packet &&

- !decoder_database_->IsComfortNoise(packet->header.payloadType)) {

+ while (packet && !decoder_database_->IsComfortNoise(packet->payloadType)) {

assert(decoder); // At this point, we must have a decoder object.

// The number of channels in the |sync_buffer_| should be the same as the

// number decoder channels.

@@ -1535,7 +1529,7 @@ int NetEqImpl::DecodeLoop(PacketList* packet_list, const Operations& operation,

// the while-loop, or list must hold exactly one CNG packet.

assert(packet_list->empty() || *decoded_length < 0 ||

(packet_list->size() == 1 && packet &&

- decoder_database_->IsComfortNoise(packet->header.payloadType)));

+ decoder_database_->IsComfortNoise(packet->payloadType)));

return 0;

}

@@ -1777,7 +1771,7 @@ int NetEqImpl::DoRfc3389Cng(PacketList* packet_list, bool play_dtmf) {

assert(packet_list->size() == 1);

Packet* packet = packet_list->front();

packet_list->pop_front();

- if (!decoder_database_->IsComfortNoise(packet->header.payloadType)) {

+ if (!decoder_database_->IsComfortNoise(packet->payloadType)) {

LOG(LS_ERROR) << "Trying to decode non-CNG payload as CNG.";

return kOtherError;

}

@@ -1953,22 +1947,22 @@ int NetEqImpl::ExtractPackets(size_t required_samples,

uint16_t prev_sequence_number = 0;

bool next_packet_available = false;

- const RTPHeader* header = packet_buffer_->NextRtpHeader();

- assert(header);

- if (!header) {

+ const Packet* next_packet = packet_buffer_->PeekNextPacket();

+ RTC_DCHECK(next_packet);

+ if (!next_packet) {

LOG(LS_ERROR) << "Packet buffer unexpectedly empty.";

return -1;

}

- uint32_t first_timestamp = header->timestamp;

+ uint32_t first_timestamp = next_packet->timestamp;

size_t extracted_samples = 0;

// Packet extraction loop.

do {

- timestamp_ = header->timestamp;

+ timestamp_ = next_packet->timestamp;

size_t discard_count = 0;

Packet* packet = packet_buffer_->GetNextPacket(&discard_count);

- // |header| may be invalid after the |packet_buffer_| operation.

- header = NULL;

+ // |next_packet| may be invalid after the |packet_buffer_| operation.

+ next_packet = NULL;

if (!packet) {

LOG(LS_ERROR) << "Should always be able to extract a packet here";

assert(false); // Should always be able to extract a packet here.

@@ -1984,12 +1978,12 @@ int NetEqImpl::ExtractPackets(size_t required_samples,

if (nack_enabled_) {

RTC_DCHECK(nack_);

// TODO(henrik.lundin): Should we update this for all decoded packets?

- nack_->UpdateLastDecodedPacket(packet->header.sequenceNumber,

- packet->header.timestamp);

+ nack_->UpdateLastDecodedPacket(packet->sequenceNumber,

+ packet->timestamp);

}

- prev_sequence_number = packet->header.sequenceNumber;

- prev_timestamp = packet->header.timestamp;

- prev_payload_type = packet->header.payloadType;

+ prev_sequence_number = packet->sequenceNumber;

+ prev_timestamp = packet->timestamp;

+ prev_payload_type = packet->payloadType;

}

// Store number of extracted samples.

@@ -2000,9 +1994,9 @@ int NetEqImpl::ExtractPackets(size_t required_samples,

if (packet->priority.codec_level > 0) {

stats_.SecondaryDecodedSamples(rtc::checked_cast<int>(packet_duration));

}

- } else if (!decoder_database_->IsComfortNoise(packet->header.payloadType)) {

+ } else if (!decoder_database_->IsComfortNoise(packet->payloadType)) {

LOG(LS_WARNING) << "Unknown payload type "

- << static_cast<int>(packet->header.payloadType);

+ << static_cast<int>(packet->payloadType);

RTC_NOTREACHED();

}

@@ -2011,22 +2005,21 @@ int NetEqImpl::ExtractPackets(size_t required_samples,

// contains the same number of samples as the previous one.

packet_duration = decoder_frame_length_;

}

- extracted_samples = packet->header.timestamp - first_timestamp +

- packet_duration;

+ extracted_samples = packet->timestamp - first_timestamp + packet_duration;

// Check what packet is available next.

- header = packet_buffer_->NextRtpHeader();

+ next_packet = packet_buffer_->PeekNextPacket();

next_packet_available = false;

- if (header && prev_payload_type == header->payloadType) {

- int16_t seq_no_diff = header->sequenceNumber - prev_sequence_number;

- size_t ts_diff = header->timestamp - prev_timestamp;

+ if (next_packet && prev_payload_type == next_packet->payloadType) {

+ int16_t seq_no_diff = next_packet->sequenceNumber - prev_sequence_number;

+ size_t ts_diff = next_packet->timestamp - prev_timestamp;

if (seq_no_diff == 1 ||

(seq_no_diff == 0 && ts_diff == decoder_frame_length_)) {

// The next sequence number is available, or the next part of a packet

// that was split into pieces upon insertion.

next_packet_available = true;

}

- prev_sequence_number = header->sequenceNumber;

+ prev_sequence_number = next_packet->sequenceNumber;

}

} while (extracted_samples < required_samples && next_packet_available);