Add status parameter to DemuxerStream::ReadCB

media/filters/audio_renderer_impl.cc

 // Copyright (c) 2012 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/filters/audio_renderer_impl.h"
 
 #include <math.h>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 54 matching lines @@
 
 void AudioRendererImpl::Pause(const base::Closure& callback) {
   {
     base::AutoLock auto_lock(lock_);
     DCHECK(state_ == kPlaying || state_ == kUnderflow ||
            state_ == kRebuffering);
     pause_cb_ = callback;
     state_ = kPaused;
 
     // Pause only when we've completed our pending read.
-    if (!pending_read_) {
-      pause_cb_.Run();
-      pause_cb_.Reset();
-    }
+    if (!pending_read_)
+      base::ResetAndReturn(&pause_cb_).Run();
   }
 
   if (stopped_)
     return;
 
   DoPause();
 }
 
 void AudioRendererImpl::DoPause() {
   DCHECK(sink_.get());
@@ skipping 138 matching lines @@
     return;
   sink_->SetVolume(volume);
 }
 
 AudioRendererImpl::~AudioRendererImpl() {
   // Stop() should have been called and |algorithm_| should have been destroyed.
   DCHECK(state_ == kUninitialized || state_ == kStopped);
   DCHECK(!algorithm_.get());
 }
 
-void AudioRendererImpl::DecodedAudioReady(scoped_refptr<Buffer> buffer) {
+void AudioRendererImpl::DecodedAudioReady(AudioDecoder::Status status,
+                                          const scoped_refptr<Buffer>& buffer) {
   base::AutoLock auto_lock(lock_);
   DCHECK(state_ == kPaused || state_ == kSeeking || state_ == kPlaying ||
          state_ == kUnderflow || state_ == kRebuffering || state_ == kStopped);
 
   CHECK(pending_read_);
   pending_read_ = false;
 
-  if (buffer && buffer->IsEndOfStream()) {
+  if (status == AudioDecoder::kDecodeError) {

[Ami GONE FROM CHROMIUM, 2012/07/12 17:10:26]

swap order to match enum

[acolwell GONE FROM CHROMIUM, 2012/07/12 21:59:58]

Done.

+    HandleDecodeError();
+    return;
+  }
+
+  if (status == AudioDecoder::kAborted) {
+    HandleAbortedRead();
+    return;
+  }
+
+  DCHECK_EQ(status, AudioDecoder::kOk);
+  DCHECK(buffer);
+
+  if (buffer->IsEndOfStream()) {
     received_end_of_stream_ = true;
 
     // Transition to kPlaying if we are currently handling an underflow since
     // no more data will be arriving.
     if (state_ == kUnderflow || state_ == kRebuffering)
       state_ = kPlaying;
   }
 
   switch (state_) {
     case kUninitialized:
       NOTREACHED();
       return;
     case kPaused:
-      if (buffer && !buffer->IsEndOfStream())
+      if (!buffer->IsEndOfStream())
         algorithm_->EnqueueBuffer(buffer);
       DCHECK(!pending_read_);
       base::ResetAndReturn(&pause_cb_).Run();
       return;
     case kSeeking:
       if (IsBeforeSeekTime(buffer)) {
         ScheduleRead_Locked();
         return;
       }
-      if (buffer && !buffer->IsEndOfStream()) {
+      if (!buffer->IsEndOfStream()) {
         algorithm_->EnqueueBuffer(buffer);
         if (!algorithm_->IsQueueFull())
           return;
       }
       state_ = kPaused;
       base::ResetAndReturn(&seek_cb_).Run(PIPELINE_OK);
       return;
     case kPlaying:
     case kUnderflow:
     case kRebuffering:
-      if (buffer && !buffer->IsEndOfStream())
+      if (!buffer->IsEndOfStream())
         algorithm_->EnqueueBuffer(buffer);
       return;
     case kStopped:
       return;
   }
 }
 
 void AudioRendererImpl::ScheduleRead_Locked() {
   lock_.AssertAcquired();
   if (pending_read_ || state_ == kPaused)
@@ skipping 217 matching lines @@
 
 void AudioRendererImpl::OnRenderError() {
   host_->DisableAudioRenderer();
 }
 
 void AudioRendererImpl::DisableUnderflowForTesting() {
   DCHECK(!is_initialized_);
   underflow_disabled_ = true;
 }
 
+void AudioRendererImpl::HandleDecodeError() {
+  switch (state_) {
+    case kUninitialized:
+      NOTREACHED();
+      return;
+    case kPaused:
+      host_->SetError(PIPELINE_ERROR_DECODE);
+      base::ResetAndReturn(&pause_cb_).Run();
+      return;
+    case kSeeking:
+      state_ = kPaused;
+      base::ResetAndReturn(&seek_cb_).Run(PIPELINE_ERROR_DECODE);
+      return;
+    case kPlaying:
+    case kUnderflow:
+    case kRebuffering:
+    case kStopped:
+      host_->SetError(PIPELINE_ERROR_DECODE);
+      return;
+  }
+}
+
+void AudioRendererImpl::HandleAbortedRead() {

[Ami GONE FROM CHROMIUM, 2012/07/12 17:10:26]

This and the previous method are awfully similar and could be collapsed to
PipelineStatus status = is_decode_error ? PIPELINE_ERROR_DECODE : PIPELINE_OK;
and then only call SetError if !OK in the two places SetError is called.

Or do you expect these to (intentionally) diverge over time?

[acolwell GONE FROM CHROMIUM, 2012/07/12 21:59:58]

Done. Merged into a single method.

+  switch (state_) {
+    case kUninitialized:
+      NOTREACHED();
+      return;
+    case kPaused:
+      base::ResetAndReturn(&pause_cb_).Run();
+      return;
+    case kSeeking:
+      state_ = kPaused;
+      base::ResetAndReturn(&seek_cb_).Run(PIPELINE_OK);
+      return;
+    case kPlaying:
+    case kUnderflow:
+    case kRebuffering:
+    case kStopped:
+      return;
+  }
+}
+
 }  // namespace media