NetEq: Fixing a bug that caused rtc::checked_cast to trigger

webrtc/modules/audio_coding/neteq/neteq_impl.cc

 /*
  *  Copyright (c) 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 581 matching lines @@
     assert(decoder);  // Should always get a valid object, since we have
                       // already checked that the payload types are known.
     decoder->IncomingPacket(packet_list.front()->payload,
                             packet_list.front()->payload_length,
                             packet_list.front()->header.sequenceNumber,
                             packet_list.front()->header.timestamp,
                             receive_timestamp);
   }
 
   // Insert packets in buffer.
-  size_t temp_bufsize = packet_buffer_->NumPacketsInBuffer();
+  const size_t buffer_length_before_insert =
+      packet_buffer_->NumPacketsInBuffer();
   ret = packet_buffer_->InsertPacketList(
       &packet_list,
       *decoder_database_,
       &current_rtp_payload_type_,
       &current_cng_rtp_payload_type_);
   if (ret == PacketBuffer::kFlushed) {
     // Reset DSP timestamp etc. if packet buffer flushed.
     new_codec_ = true;
     update_sample_rate_and_channels = true;
   } else if (ret != PacketBuffer::kOK) {
@@ skipping 35 matching lines @@
       SetSampleRateAndChannels(decoder_info->fs_hz, decoder->Channels());
   }
 
   // TODO(hlundin): Move this code to DelayManager class.
   const DecoderDatabase::DecoderInfo* dec_info =
           decoder_database_->GetDecoderInfo(main_header.payloadType);
   assert(dec_info);  // Already checked that the payload type is known.
   delay_manager_->LastDecoderType(dec_info->codec_type);
   if (delay_manager_->last_pack_cng_or_dtmf() == 0) {
     // Calculate the total speech length carried in each packet.
-    temp_bufsize = packet_buffer_->NumPacketsInBuffer() - temp_bufsize;
-    temp_bufsize *= decoder_frame_length_;
+    const size_t buffer_length_after_insert =
+        packet_buffer_->NumPacketsInBuffer();
 
-    if ((temp_bufsize > 0) &&

[minyue-webrtc, 2015/08/27 09:09:30]

Since we introduced size_t - size_t here, it should not have been difficult to
detect when changing the datatype. But the preceding CL was huge and might not
be easy.

Hope there are no other errors.

[Peter Kasting, 2015/08/27 09:14:54]

I wouldn't have caught this even had I just done this portion of the change. 
InsertPacketList() sounds as if it will either increase the buffer size or do
nothing, and the "> 0" check here looks as if it's distinguishing from the "0"
case rather than from the negative number case.

(I did look as a result of this CL to make sure InsertPacketList() documents
that it can decrease the buffer size.  I thought about suggesting it be called
InsertPacketsAndMaybeFlush() or something like that, but it seems awkward.)

-        (temp_bufsize != decision_logic_->packet_length_samples())) {
-      decision_logic_->set_packet_length_samples(temp_bufsize);
-      delay_manager_->SetPacketAudioLength(
-          static_cast<int>((1000 * temp_bufsize) / fs_hz_));
+    if (buffer_length_after_insert > buffer_length_before_insert) {
+      const size_t packet_length_samples =
+          (buffer_length_after_insert - buffer_length_before_insert) *
+          decoder_frame_length_;
+      if (packet_length_samples != decision_logic_->packet_length_samples()) {
+        decision_logic_->set_packet_length_samples(packet_length_samples);
+        delay_manager_->SetPacketAudioLength(
+            rtc::checked_cast<int>((1000 * packet_length_samples) / fs_hz_));
+      }
     }
 
     // Update statistics.
     if ((int32_t) (main_header.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_);
     }
@@ skipping 1294 matching lines @@
 
 void NetEqImpl::CreateDecisionLogic() {
   decision_logic_.reset(DecisionLogic::Create(fs_hz_, output_size_samples_,
                                               playout_mode_,
                                               decoder_database_.get(),
                                               *packet_buffer_.get(),
                                               delay_manager_.get(),
                                               buffer_level_filter_.get()));
 }
 }  // namespace webrtc