Fix AudioBuffer verification tests.

media/base/audio_buffer_unittest.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 "media/base/audio_bus.h"
 #include "media/base/test_helpers.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace media {
 
 static const int kSampleRate = 48000;
 
-
 static void VerifyBusWithOffset(AudioBus* bus,
                                 int offset,
                                 int frames,
                                 float start,
+                                float start_offset,

[wolenetz, 2014/05/05 20:28:45]

Is there really a difference between start and start_offset? (Why do we need to
split out start_offset from start?)

[DaleCurtis, 2014/05/05 21:28:50]

Yes.  |start| is used to calculate a new |v| at the first frame of each channel.
 However if it's computed using |start_offset| it'll be completely wrong since
|start_offset| wasn't what was used to construct the buffer.

[wolenetz, 2014/05/05 22:16:29]

nit: Hmm. Then maybe lift the constant 'start_offset + ' out of the inner loop
into the calculation of v in the outer loop?

[DaleCurtis, 2014/05/05 22:54:44]

Done.

                                 float increment) {
   for (int ch = 0; ch < bus->channels(); ++ch) {
     const float v = start + ch * bus->frames() * increment;
-    for (int i = offset; i < frames; ++i) {
-      ASSERT_FLOAT_EQ(v + i * increment, bus->channel(ch)[i]) << "i=" << i
-                                                              << ", ch=" << ch;
+    for (int i = offset; i < offset + frames; ++i) {
+      ASSERT_FLOAT_EQ(start_offset + v + i * increment, bus->channel(ch)[i])
+          << "i=" << i << ", ch=" << ch;
     }
   }
 }
 
 static void VerifyBus(AudioBus* bus, int frames, float start, float increment) {
-  VerifyBusWithOffset(bus, 0, frames, start, increment);
+  VerifyBusWithOffset(bus, 0, frames, start, 0, increment);
 }
 
 static void TrimRangeTest(SampleFormat sample_format) {
   const ChannelLayout channel_layout = CHANNEL_LAYOUT_4_0;
   const int channels = ChannelLayoutToChannelCount(channel_layout);
   const int frames = kSampleRate / 10;
   const base::TimeDelta timestamp = base::TimeDelta();
   const base::TimeDelta duration = base::TimeDelta::FromMilliseconds(100);
   scoped_refptr<AudioBuffer> buffer = MakeAudioBuffer<float>(sample_format,
                                                              channel_layout,
@@ skipping 17 matching lines @@
   int trim_start = frames / 2;
   const int trim_length = kSampleRate / 100;
   const base::TimeDelta trim_duration = base::TimeDelta::FromMilliseconds(10);
   buffer->TrimRange(trim_start, trim_start + trim_length);
   EXPECT_EQ(frames - trim_length, buffer->frame_count());
   EXPECT_EQ(timestamp, buffer->timestamp());
   EXPECT_EQ(duration - trim_duration, buffer->duration());
   bus->Zero();
   buffer->ReadFrames(buffer->frame_count(), 0, 0, bus.get());
   VerifyBus(bus.get(), trim_start, 0, 1);
-  VerifyBusWithOffset(
-      bus.get(), trim_start, buffer->frame_count() - trim_start, 0, 1);
+  VerifyBusWithOffset(bus.get(),
+                      trim_start,
+                      buffer->frame_count() - trim_start,
+                      0,
+                      trim_length,
+                      1);
 
   // Trim 10ms of frames from the start, which just adjusts the buffer's
   // internal start offset.
   buffer->TrimStart(trim_length);
   trim_start -= trim_length;
   EXPECT_EQ(frames - 2 * trim_length, buffer->frame_count());
   EXPECT_EQ(timestamp + trim_duration, buffer->timestamp());
   EXPECT_EQ(duration - 2 * trim_duration, buffer->duration());
   bus->Zero();
   buffer->ReadFrames(buffer->frame_count(), 0, 0, bus.get());
   VerifyBus(bus.get(), trim_start, trim_length, 1);
-  VerifyBusWithOffset(
-      bus.get(), trim_start, buffer->frame_count() - trim_start, 0, 1);
+  VerifyBusWithOffset(bus.get(),
+                      trim_start,
+                      buffer->frame_count() - trim_start,
+                      trim_length,
+                      trim_length,
+                      1);
 
   // Trim 10ms of frames from the end, which just adjusts the buffer's frame
   // count.
   buffer->TrimEnd(trim_length);
   EXPECT_EQ(frames - 3 * trim_length, buffer->frame_count());
   EXPECT_EQ(timestamp + trim_duration, buffer->timestamp());
   EXPECT_EQ(duration - 3 * trim_duration, buffer->duration());
   bus->Zero();
   buffer->ReadFrames(buffer->frame_count(), 0, 0, bus.get());
   VerifyBus(bus.get(), trim_start, trim_length, 1);
-  VerifyBusWithOffset(
-      bus.get(), trim_start, buffer->frame_count() - trim_start, 0, 1);
+  VerifyBusWithOffset(bus.get(),
+                      trim_start,
+                      buffer->frame_count() - trim_start,
+                      trim_length,
+                      trim_length,
+                      1);
 
   // Trim another 10ms from the inner portion of the buffer.
   buffer->TrimRange(trim_start, trim_start + trim_length);
   EXPECT_EQ(frames - 4 * trim_length, buffer->frame_count());
   EXPECT_EQ(timestamp + trim_duration, buffer->timestamp());
   EXPECT_EQ(duration - 4 * trim_duration, buffer->duration());
   bus->Zero();
   buffer->ReadFrames(buffer->frame_count(), 0, 0, bus.get());
   VerifyBus(bus.get(), trim_start, trim_length, 1);
-  VerifyBusWithOffset(
-      bus.get(), trim_start, buffer->frame_count() - trim_start, 0, 1);
+  VerifyBusWithOffset(bus.get(),
+                      trim_start,
+                      buffer->frame_count() - trim_start,
+                      trim_length,
+                      trim_length * 2,
+                      1);
 
   // Trim off the end using TrimRange() to ensure end index is exclusive.
   buffer->TrimRange(buffer->frame_count() - trim_length, buffer->frame_count());
   EXPECT_EQ(frames - 5 * trim_length, buffer->frame_count());
   EXPECT_EQ(timestamp + trim_duration, buffer->timestamp());
   EXPECT_EQ(duration - 5 * trim_duration, buffer->duration());
   bus->Zero();
   buffer->ReadFrames(buffer->frame_count(), 0, 0, bus.get());
   VerifyBus(bus.get(), trim_start, trim_length, 1);
-  VerifyBusWithOffset(
-      bus.get(), trim_start, buffer->frame_count() - trim_start, 0, 1);
+  VerifyBusWithOffset(bus.get(),
+                      trim_start,
+                      buffer->frame_count() - trim_start,
+                      trim_length,
+                      trim_length * 2,
+                      1);
 
   // Trim off the start using TrimRange() to ensure start index is inclusive.
   buffer->TrimRange(0, trim_length);
   trim_start -= trim_length;
   EXPECT_EQ(frames - 6 * trim_length, buffer->frame_count());
   EXPECT_EQ(timestamp + trim_duration, buffer->timestamp());

[wolenetz, 2014/05/05 20:28:45]

nit: versus TrimStart, TrimRange(0,...) does not update timestamp because it is
only used in cases where the timestamp shouldn't need updating? This seems a
little too tightly bound to the current caller
(AudioDiscardHelper::ProcessBuffers()), which (upon success) replaces the
buffer's timestamp with it's own calculation, after any TrimRange() may or may
not have updated it.
Suggestion: Make TrimRange() more general purpose/intuitive by having it update
the buffer timestamp in TrimStart cases (possibly by just detecting such cases
and deferring to TrimStart instead of existing internal TrimRange() logic?)

[DaleCurtis, 2014/05/05 21:28:50]

No, callers can call TrimStart() if they want the timestamp updated. This
provides a way to Trim from the beginning without updating the timestamp.

   EXPECT_EQ(duration - 6 * trim_duration, buffer->duration());
   bus->Zero();
   buffer->ReadFrames(buffer->frame_count(), 0, 0, bus.get());
   VerifyBus(bus.get(), trim_start, 2 * trim_length, 1);
-  VerifyBusWithOffset(
-      bus.get(), trim_start, buffer->frame_count() - trim_start, 0, 1);
+  VerifyBusWithOffset(bus.get(),
+                      trim_start,
+                      buffer->frame_count() - trim_start,
+                      trim_length * 2,
+                      trim_length * 2,
+                      1);
 }
 
 TEST(AudioBufferTest, CopyFrom) {
   const ChannelLayout kChannelLayout = CHANNEL_LAYOUT_MONO;
   scoped_refptr<AudioBuffer> original_buffer =
       MakeAudioBuffer<uint8>(kSampleFormatU8,
                              kChannelLayout,
                              ChannelLayoutToChannelCount(kChannelLayout),
                              kSampleRate,
                              1,
@@ skipping 282 matching lines @@
 
 TEST(AudioBufferTest, TrimRangePlanar) {
   TrimRangeTest(kSampleFormatPlanarF32);
 }
 
 TEST(AudioBufferTest, TrimRangeInterleaved) {
   TrimRangeTest(kSampleFormatF32);
 }
 
 }  // namespace media