Allow PushSource to have smaller write size than read size (packet size)

The change actually supports arbitrary write size but
write size greater than read size (packet size) is prohibited
by an explicit check.

Review URL: http://codereview.chromium.org/118170

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@17686 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 133 insertions, 56 deletions

diff --git a/media/audio/simple_sources.cc b/media/audio/simple_sources.cc
index 6643fc1..f941782 100644
--- a/media/audio/simple_sources.cc
+++ b/media/audio/simple_sources.cc
@@ -50,7 +50,9 @@ void SineWaveAudioSource::OnError(AudioOutputStream* stream, int code) {
 // PushSource implementation.
 
 PushSource::PushSource(size_t packet_size)
-    : packet_size_(packet_size), buffered_bytes_(0) {
+    : packet_size_(packet_size),
+      buffered_bytes_(0),
+      front_buffer_consumed_(0) {
 }
 
 PushSource::~PushSource() {
@@ -59,20 +61,29 @@ PushSource::~PushSource() {
 
 size_t PushSource::OnMoreData(AudioOutputStream* stream,
                               void* dest, size_t max_size) {
-  Packet packet;
-  {
+  size_t copied = 0;
+  while (copied < max_size) {
     AutoLock auto_lock(lock_);
+
     // Under lock processing in this scope.
     if (!packets_.size())
-      return 0;
-    packet = packets_.front();
-    packets_.pop_front();
-    buffered_bytes_ -= packet.size;
+      break;
+    Packet packet = packets_.front();
+    size_t size = std::min(max_size - copied,
+                           packet.size - front_buffer_consumed_);
+    memcpy(static_cast<char*>(dest) + copied,
+           packet.buffer + front_buffer_consumed_,
+           size);
+    front_buffer_consumed_ += size;
+    buffered_bytes_ -= size;
+    copied += size;
+    if (front_buffer_consumed_ == packet.size) {
+      delete [] packet.buffer;
+      packets_.pop_front();
+      front_buffer_consumed_ = 0;
+    }
   }
-  size_t size = std::min(max_size, packet.size);
-  memcpy(dest, packet.buffer, size);
-  delete [] packet.buffer;
-  return size;
+  return copied;
 }
 
 void PushSource::OnClose(AudioOutputStream* stream) {
@@ -85,7 +96,7 @@ void PushSource::OnError(AudioOutputStream* stream, int code) {
 
 // TODO(cpu): Manage arbitrary large sizes.
 bool PushSource::Write(const void *data, size_t len) {
-  if ((len == 0) || (len > packet_size_)) {
+  if (len == 0) {
     NOTREACHED();
     return false;
   }
diff --git a/media/audio/simple_sources.h b/media/audio/simple_sources.h
index 1da4d4f..4a3db8d 100644
--- a/media/audio/simple_sources.h
+++ b/media/audio/simple_sources.h
@@ -60,11 +60,12 @@ class PushSource : public AudioOutputStream::AudioSourceCallback,
  public:
   // Construct the audio source. Pass the same |packet_size| specified in the
   // AudioOutputStream::Open() here.
+  // TODO(hclam): |packet_size| is not used anymore, remove it.
   explicit PushSource(size_t packet_size);
   virtual ~PushSource();
 
   // Write one buffer. The ideal size is |packet_size| but smaller sizes are
-  // accepted. Bigger sizes are an error. Returns false on error.
+  // accepted.
   virtual bool Write(const void* data, size_t len);
 
   // Return the total number of bytes not given to the audio device yet.
@@ -90,6 +91,7 @@ class PushSource : public AudioOutputStream::AudioSourceCallback,
   typedef std::list<Packet> PacketList; 
   PacketList packets_;
   size_t buffered_bytes_;
+  size_t front_buffer_consumed_;
   // Serialize access to packets_ and buffered_bytes_ using this lock.
   Lock lock_;
 };
diff --git a/media/audio/simple_sources_unittest.cc b/media/audio/simple_sources_unittest.cc
new file mode 100644
index 0000000..0ebb335
--- /dev/null
+++ b/media/audio/simple_sources_unittest.cc
@@ -0,0 +1,106 @@
+// Copyright (c) 2009 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 "base/logging.h"

+#include "base/basictypes.h"

+#include "base/scoped_ptr.h"

+#include "base/time.h"

+#include "media/audio/simple_sources.h"

+#include "testing/gtest/include/gtest/gtest.h"

+

+namespace {

+

+void GenerateRandomData(char* buffer, size_t len) {

+  static bool called = false;

+  if (!called) {

+    called = true;

+    int seed = static_cast<int>(base::Time::Now().ToInternalValue());

+    srand(seed);

+    LOG(INFO) << "Random seed: " << seed;

+  }

+

+  for (size_t i = 0; i < len; i++) {

+    buffer[i] = static_cast<char>(rand());

+  }

+}

+

+}  // namespace

+

+// To test write size smaller than read size.

+TEST(SimpleSourcesTest, PushSourceSmallerWrite) {

+  const size_t kDataSize = 40960;

+  scoped_array<char> data(new char[kDataSize]);

+  GenerateRandomData(data.get(), kDataSize);

+

+  // Choose two prime numbers for read and write sizes.

+  const size_t kWriteSize = 283;

+  const size_t kReadSize = 293;

+  scoped_array<char> read_data(new char[kReadSize]);

+

+  // Create a PushSource that assumes the hardware audio buffer size is always

+  // bigger than the write size.

+  PushSource push_source(kReadSize);

+  EXPECT_EQ(0u, push_source.UnProcessedBytes());

+

+  // Write everything into this push source.

+  for (size_t i = 0; i < kDataSize; i += kWriteSize) {

+    size_t size = std::min(kDataSize - i, kWriteSize);

+    EXPECT_TRUE(push_source.Write(data.get() + i, size));

+  }

+  EXPECT_EQ(kDataSize, push_source.UnProcessedBytes());

+

+  // Read everything from the push source.

+  for (size_t i = 0; i < kDataSize; i += kReadSize) {

+    size_t size = std::min(kDataSize - i , kReadSize);

+    EXPECT_EQ(size, push_source.OnMoreData(NULL, read_data.get(), size));

+    EXPECT_EQ(0, memcmp(data.get() + i, read_data.get(), size));

+  }

+  EXPECT_EQ(0u, push_source.UnProcessedBytes());

+

+  push_source.OnClose(NULL);

+}

+

+
+// Validate that the SineWaveAudioSource writes the expected values for
+// the FORMAT_16BIT_MONO. The values are carefully selected so rounding issues
+// do not affect the result. We also test that AudioManager::GetLastMockBuffer
+// works.
+TEST(SimpleSourcesTest, SineWaveAudio16MonoTest) {
+  const size_t samples = 1024;
+  const size_t bytes_per_sample = 2;
+  const int freq = 200;
+
+  SineWaveAudioSource source(SineWaveAudioSource::FORMAT_16BIT_LINEAR_PCM, 1,
+                             freq, AudioManager::kTelephoneSampleRate);
+
+  AudioManager* audio_man = AudioManager::GetAudioManager();
+  ASSERT_TRUE(NULL != audio_man);
+  AudioOutputStream* oas =
+      audio_man->MakeAudioStream(AudioManager::AUDIO_MOCK, 1,
+                                 AudioManager::kTelephoneSampleRate,
+                                 bytes_per_sample * 2);
+  ASSERT_TRUE(NULL != oas);
+  EXPECT_TRUE(oas->Open(samples * bytes_per_sample));
+
+  oas->Start(&source);
+  oas->Stop();
+  oas->Close();
+
+  const int16* last_buffer =
+      reinterpret_cast<const int16*>(audio_man->GetLastMockBuffer());
+  ASSERT_TRUE(NULL != last_buffer);
+
+  size_t half_period = AudioManager::kTelephoneSampleRate / (freq * 2);
+
+  // Spot test positive incursion of sine wave.
+  EXPECT_EQ(0, last_buffer[0]);
+  EXPECT_EQ(5126, last_buffer[1]);
+  EXPECT_TRUE(last_buffer[1] < last_buffer[2]);
+  EXPECT_TRUE(last_buffer[2] < last_buffer[3]);
+  // Spot test negative incursion of sine wave.
+  EXPECT_EQ(0, last_buffer[half_period]);
+  EXPECT_EQ(-5126, last_buffer[half_period + 1]);
+  EXPECT_TRUE(last_buffer[half_period + 1] > last_buffer[half_period + 2]);
+  EXPECT_TRUE(last_buffer[half_period + 2] > last_buffer[half_period + 3]);
+}
diff --git a/media/audio/win/audio_output_win_unittest.cc b/media/audio/win/audio_output_win_unittest.cc
index 5366754..00b9dd7 100644
--- a/media/audio/win/audio_output_win_unittest.cc
+++ b/media/audio/win/audio_output_win_unittest.cc
@@ -193,49 +193,6 @@ TEST(WinAudioTest, MockStreamBasicCallbacks) {
   EXPECT_EQ(1, source.was_closed());
 }
 
-// Validate that the SineWaveAudioSource writes the expected values for
-// the FORMAT_16BIT_MONO. The values are carefully selected so rounding issues
-// do not affect the result. We also test that AudioManager::GetLastMockBuffer
-// works.
-TEST(WinAudioTest, SineWaveAudio16MonoTest) {
-  const size_t samples = 1024;
-  const size_t bytes_per_sample = 2;
-  const int freq = 200;
-
-  SineWaveAudioSource source(SineWaveAudioSource::FORMAT_16BIT_LINEAR_PCM, 1,
-                             freq, AudioManager::kTelephoneSampleRate);
-
-  AudioManager* audio_man = AudioManager::GetAudioManager();
-  ASSERT_TRUE(NULL != audio_man);
-  AudioOutputStream* oas =
-      audio_man->MakeAudioStream(AudioManager::AUDIO_MOCK, 1,
-                                 AudioManager::kTelephoneSampleRate,
-                                 bytes_per_sample * 2);
-  ASSERT_TRUE(NULL != oas);
-  EXPECT_TRUE(oas->Open(samples * bytes_per_sample));
-
-  oas->Start(&source);
-  oas->Stop();
-  oas->Close();
-
-  const int16* last_buffer =
-      reinterpret_cast<const int16*>(audio_man->GetLastMockBuffer());
-  ASSERT_TRUE(NULL != last_buffer);
-
-  size_t half_period = AudioManager::kTelephoneSampleRate / (freq * 2);
-
-  // Spot test positive incursion of sine wave.
-  EXPECT_EQ(0, last_buffer[0]);
-  EXPECT_EQ(5126, last_buffer[1]);
-  EXPECT_TRUE(last_buffer[1] < last_buffer[2]);
-  EXPECT_TRUE(last_buffer[2] < last_buffer[3]);
-  // Spot test negative incursion of sine wave.
-  EXPECT_EQ(0, last_buffer[half_period]);
-  EXPECT_EQ(-5126, last_buffer[half_period + 1]);
-  EXPECT_TRUE(last_buffer[half_period + 1] > last_buffer[half_period + 2]);
-  EXPECT_TRUE(last_buffer[half_period + 2] > last_buffer[half_period + 3]);
-}
-
 // ===========================================================================
 // Validation of AudioManager::AUDIO_PCM_LINEAR
 //
diff --git a/media/media.gyp b/media/media.gyp
index db387c8..a62da2a 100644
--- a/media/media.gyp
+++ b/media/media.gyp
@@ -145,6 +145,7 @@
       'sources': [
         'audio/win/audio_output_win_unittest.cc',
         'audio/mac/audio_output_mac_unittest.cc',
+        'audio/simple_sources_unittest.cc',
         'base/data_buffer_unittest.cc',
         'base/pipeline_impl_unittest.cc',
         'base/run_all_unittests.cc',