Support start trimming post-decoding. Use it with FFmpegDemuxer.

media/base/audio_buffer.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/base/audio_buffer.h"
 
 #include "base/logging.h"
 #include "media/base/audio_bus.h"
 #include "media/base/buffers.h"
 #include "media/base/limits.h"
@@ skipping 257 matching lines @@
   double offset = static_cast<double>(duration_.InMicroseconds()) *
                   frames_to_trim / adjusted_frame_count_;
   base::TimeDelta offset_as_time =
       base::TimeDelta::FromMicroseconds(static_cast<int64>(offset));
   duration_ -= offset_as_time;
 
   // Finally adjust the number of frames in this buffer.
   adjusted_frame_count_ -= frames_to_trim;
 }
 
+void AudioBuffer::TrimRange(int start, int end) {
+  const int frames_to_trim = end - start;
+  CHECK_GE(frames_to_trim, 0);
+  CHECK_LE(frames_to_trim, adjusted_frame_count_);
+
+  const int bytes_per_channel = SampleFormatToBytesPerChannel(sample_format_);
+  const int frames_to_copy = adjusted_frame_count_ - frames_to_trim;

[wolenetz, 2014/04/29 01:18:30]

This is wrong for all cases where start > 0:
example:
trim_start_: 0
adjusted_frame_count_: 32
start: 15
end: 20
frames_to_copy should be 12 (== 32-20), not 27 (== 32-5).
Copying way too much is bad :) : (security, page fault, etc issues could occur).

Furthermore, I'm not convinced this method correctly understands any prior
TrimStart() which could have left trim_start_>0: What is the assumption of the
index-base of start and end?

Please fix and add test cases, too, to verify TrimStart() followed by
TrimRange() at least behave in sequence, possibly adding TrimEnd().

[DaleCurtis, 2014/04/29 03:16:20]

Yeah this is all wrong, I'm surprised it works at all!  I promise it passed all
the tests :) The concept will be the same once I fix the implementation, so the
rest of CL is good for further review.

+  switch (sample_format_) {

[wolenetz, 2014/04/29 01:18:30]

If frames_to_copy is 0, skip the shifting and just trim it all (rather than
memcpy(...,...,0) ?

[DaleCurtis, 2014/04/30 22:04:04]

Done.

+    case kSampleFormatPlanarS16:
+    case kSampleFormatPlanarF32:
+      // Planar data must be shifted per channel.
+      for (int ch = 0; ch < channel_count_; ++ch) {
+        memcpy(channel_data_[ch] + start * bytes_per_channel,

[wolenetz, 2014/04/29 19:18:08]

Use memmove, not memcpy due to potentially overlapping ranges (per msg#8) ?

[DaleCurtis, 2014/04/30 22:04:04]

Done.

+               channel_data_[ch] + end * bytes_per_channel,
+               bytes_per_channel * frames_to_copy);
+      }
+      break;
+    case kSampleFormatU8:
+    case kSampleFormatS16:
+    case kSampleFormatS32:
+    case kSampleFormatF32: {
+      // Interleaved data can be shifted all at once.
+      const int frame_size = channel_count_ * bytes_per_channel;
+      memcpy(channel_data_[0] + start * frame_size,

[wolenetz, 2014/04/29 19:18:08]

ditto: memmove?

[DaleCurtis, 2014/04/30 22:04:04]

Done.

+             channel_data_[0] + end * frame_size,
+             frame_size * frames_to_copy);
+      break;
+    }
+    case kUnknownSampleFormat:
+      NOTREACHED() << "Invalid sample format!";
+  }
+
+  // Trim the leftover data off the end of the buffer and update duration.
+  TrimEnd(frames_to_trim);
+}
+
 }  // namespace media