Assume pending buffered bytes is zero when ALSA has underrun.

According to docs, snd_pcm_delay() may not reach zero when ALSA has underrun.  Furthermore it may return negative numbers when underrun, leading to an underflow when converted to uint32.  Previously none of this was an issue however as of r43546 we now wait for pending buffered bytes to reach zero before notifying that the audio stream has finished.

This doesn't completely fix the Linux ended event issue, but is a required fix regardless.

BUG=30452
TEST=media_unittests

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

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

5 files changed, 73 insertions, 16 deletions

diff --git a/media/audio/linux/alsa_output.cc b/media/audio/linux/alsa_output.cc
index 1ecd685..0726eff 100644
--- a/media/audio/linux/alsa_output.cc
+++ b/media/audio/linux/alsa_output.cc
@@ -474,21 +474,25 @@ void AlsaPcmOutputStream::BufferPacket(Packet* packet) {
   if (packet->used >= packet->size) {
     // Before making a request to source for data. We need to determine the
     // delay (in bytes) for the requested data to be played.
-    snd_pcm_sframes_t delay;
-    int error = wrapper_->PcmDelay(playback_handle_, &delay);
-    if (error < 0) {
-      error = wrapper_->PcmRecover(playback_handle_,
-                                   error,
-                                   kPcmRecoverIsSilent);
-      if (error < 0) {
-        LOG(ERROR) << "Failed querying delay: " << wrapper_->StrError(error);
+    snd_pcm_sframes_t delay = 0;
+
+    // Don't query ALSA's delay if we have underrun since it'll be jammed at
+    // some non-zero value and potentially even negative!
+    if (wrapper_->PcmState(playback_handle_) != SND_PCM_STATE_XRUN) {
+      int error = wrapper_->PcmDelay(playback_handle_, &delay);
+      if (error >= 0) {
+        // Convert frames to bytes, but watch out for those negatives!
+        delay = (delay < 0 ? 0 : delay) * bytes_per_output_frame_;
+      } else {
+        // Assume a delay of zero and attempt to recover the device.
+        delay = 0;
+        error = wrapper_->PcmRecover(playback_handle_,
+                                     error,
+                                     kPcmRecoverIsSilent);
+        if (error < 0) {
+          LOG(ERROR) << "Failed querying delay: " << wrapper_->StrError(error);
+        }
       }
-
-      // TODO(hclam): If we cannot query the delay, we may want to stop
-      // the playback and report an error.
-      delay = 0;
-    } else {
-      delay *= bytes_per_output_frame_;
     }
 
     packet->used = 0;
diff --git a/media/audio/linux/alsa_output.h b/media/audio/linux/alsa_output.h
index 5900f77..192186b 100644
--- a/media/audio/linux/alsa_output.h
+++ b/media/audio/linux/alsa_output.h
@@ -91,7 +91,9 @@ class AlsaPcmOutputStream :
   FRIEND_TEST(AlsaPcmOutputStreamTest, AutoSelectDevice_FallbackDevices);
   FRIEND_TEST(AlsaPcmOutputStreamTest, AutoSelectDevice_HintFail);
   FRIEND_TEST(AlsaPcmOutputStreamTest, BufferPacket);
+  FRIEND_TEST(AlsaPcmOutputStreamTest, BufferPacket_Negative);
   FRIEND_TEST(AlsaPcmOutputStreamTest, BufferPacket_StopStream);
+  FRIEND_TEST(AlsaPcmOutputStreamTest, BufferPacket_Underrun);
   FRIEND_TEST(AlsaPcmOutputStreamTest, BufferPacket_UnfinishedPacket);
   FRIEND_TEST(AlsaPcmOutputStreamTest, ConstructedState);
   FRIEND_TEST(AlsaPcmOutputStreamTest, LatencyFloor);
diff --git a/media/audio/linux/alsa_output_unittest.cc b/media/audio/linux/alsa_output_unittest.cc
index 3dcc610..598ca98 100644
--- a/media/audio/linux/alsa_output_unittest.cc
+++ b/media/audio/linux/alsa_output_unittest.cc
@@ -47,6 +47,7 @@ class MockAlsaWrapper : public AlsaWrapper {
                                  unsigned int latency));
   MOCK_METHOD1(PcmName, const char*(snd_pcm_t* handle));
   MOCK_METHOD1(PcmAvailUpdate, snd_pcm_sframes_t (snd_pcm_t* handle));
+  MOCK_METHOD1(PcmState, snd_pcm_state_t (snd_pcm_t* handle));
 
   MOCK_METHOD1(StrError, const char*(int errnum));
 };
@@ -400,6 +401,9 @@ TEST_F(AlsaPcmOutputStreamTest, StartStop) {
 
   // Expect the pre-roll.
   MockAudioSourceCallback mock_callback;
+  EXPECT_CALL(mock_alsa_wrapper_, PcmState(kFakeHandle))
+      .Times(2)
+      .WillRepeatedly(Return(SND_PCM_STATE_RUNNING));
   EXPECT_CALL(mock_alsa_wrapper_, PcmDelay(kFakeHandle, _))
       .Times(2)
       .WillRepeatedly(DoAll(SetArgumentPointee<1>(0), Return(0)));
@@ -494,6 +498,8 @@ TEST_F(AlsaPcmOutputStreamTest, BufferPacket) {
 
   // Return a partially filled packet.
   MockAudioSourceCallback mock_callback;
+  EXPECT_CALL(mock_alsa_wrapper_, PcmState(_))
+      .WillOnce(Return(SND_PCM_STATE_RUNNING));
   EXPECT_CALL(mock_alsa_wrapper_, PcmDelay(_, _))
       .WillOnce(DoAll(SetArgumentPointee<1>(1), Return(0)));
   EXPECT_CALL(mock_callback,
@@ -508,6 +514,46 @@ TEST_F(AlsaPcmOutputStreamTest, BufferPacket) {
   EXPECT_EQ(10u, packet_.size);
 }
 
+TEST_F(AlsaPcmOutputStreamTest, BufferPacket_Negative) {
+  packet_.used = packet_.size;
+
+  // Simulate where the underrun has occurred right after checking the delay.
+  MockAudioSourceCallback mock_callback;
+  EXPECT_CALL(mock_alsa_wrapper_, PcmState(_))
+      .WillOnce(Return(SND_PCM_STATE_RUNNING));
+  EXPECT_CALL(mock_alsa_wrapper_, PcmDelay(_, _))
+      .WillOnce(DoAll(SetArgumentPointee<1>(-1), Return(0)));
+  EXPECT_CALL(mock_callback,
+              OnMoreData(test_stream_.get(), packet_.buffer.get(),
+                         packet_.capacity, 0))
+      .WillOnce(Return(10));
+
+  test_stream_->shared_data_.set_source_callback(&mock_callback);
+  test_stream_->BufferPacket(&packet_);
+
+  EXPECT_EQ(0u, packet_.used);
+  EXPECT_EQ(10u, packet_.size);
+}
+
+TEST_F(AlsaPcmOutputStreamTest, BufferPacket_Underrun) {
+  packet_.used = packet_.size;
+
+  // If ALSA has underrun then we should assume a delay of zero.
+  MockAudioSourceCallback mock_callback;
+  EXPECT_CALL(mock_alsa_wrapper_, PcmState(_))
+      .WillOnce(Return(SND_PCM_STATE_XRUN));
+  EXPECT_CALL(mock_callback,
+              OnMoreData(test_stream_.get(), packet_.buffer.get(),
+                         packet_.capacity, 0))
+      .WillOnce(Return(10));
+
+  test_stream_->shared_data_.set_source_callback(&mock_callback);
+  test_stream_->BufferPacket(&packet_);
+
+  EXPECT_EQ(0u, packet_.used);
+  EXPECT_EQ(10u, packet_.size);
+}
+
 TEST_F(AlsaPcmOutputStreamTest, BufferPacket_UnfinishedPacket) {
   // No expectations set on the strict mock because nothing should be called.
   test_stream_->BufferPacket(&packet_);
diff --git a/media/audio/linux/alsa_wrapper.cc b/media/audio/linux/alsa_wrapper.cc
index 303a1f1..a028484 100644
--- a/media/audio/linux/alsa_wrapper.cc
+++ b/media/audio/linux/alsa_wrapper.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -71,6 +71,10 @@ snd_pcm_sframes_t AlsaWrapper::PcmAvailUpdate(snd_pcm_t* handle) {
   return snd_pcm_avail_update(handle);
 }
 
+snd_pcm_state_t AlsaWrapper::PcmState(snd_pcm_t* handle) {
+  return snd_pcm_state(handle);
+}
+
 const char* AlsaWrapper::StrError(int errnum) {
   return snd_strerror(errnum);
 }
diff --git a/media/audio/linux/alsa_wrapper.h b/media/audio/linux/alsa_wrapper.h
index 5ab3c84..ce045ea 100644
--- a/media/audio/linux/alsa_wrapper.h
+++ b/media/audio/linux/alsa_wrapper.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 //
@@ -35,6 +35,7 @@ class AlsaWrapper {
                            unsigned int latency);
   virtual const char* PcmName(snd_pcm_t* handle);
   virtual snd_pcm_sframes_t PcmAvailUpdate(snd_pcm_t* handle);
+  virtual snd_pcm_state_t PcmState(snd_pcm_t* handle);
 
   virtual const char* StrError(int errnum);