Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1133)

Unified Diff: media/filters/audio_renderer_impl.h

Issue 11275087: Move OnDecoderInitDone() from decoder to pipeline thread. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Comments. Created 8 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | media/filters/audio_renderer_impl.cc » ('j') | media/filters/audio_renderer_impl.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: media/filters/audio_renderer_impl.h
diff --git a/media/filters/audio_renderer_impl.h b/media/filters/audio_renderer_impl.h
index 47a498b4079ee5f9d59f5fea28c4dbc15ca62980..b3767d01149a037c285833233fb2d3a62e371d01 100644
--- a/media/filters/audio_renderer_impl.h
+++ b/media/filters/audio_renderer_impl.h
@@ -23,6 +23,7 @@
#include <deque>
#include "base/synchronization/lock.h"
+#include "base/threading/thread_checker.h"
#include "media/base/audio_decoder.h"
#include "media/base/audio_renderer.h"
#include "media/base/audio_renderer_sink.h"
@@ -113,6 +114,7 @@ class MEDIA_EXPORT AudioRendererImpl
// Estimate earliest time when current buffer can stop playing.
void UpdateEarliestEndTime(int bytes_filled,
+ float playback_rate,
base::TimeDelta request_delay,
base::Time time_now);
@@ -120,7 +122,8 @@ class MEDIA_EXPORT AudioRendererImpl
void DoPlay();
void DoPause();
- // media::AudioRendererSink::RenderCallback implementation.
+ // media::AudioRendererSink::RenderCallback implementation. Called on the
+ // AudioDevice thread.
virtual int Render(AudioBus* audio_bus,
int audio_delay_milliseconds) OVERRIDE;
virtual void OnRenderError() OVERRIDE;
@@ -151,11 +154,41 @@ class MEDIA_EXPORT AudioRendererImpl
// Audio decoder.
scoped_refptr<AudioDecoder> decoder_;
- // Algorithm for scaling audio.
- scoped_ptr<AudioRendererAlgorithm> algorithm_;
+ // The sink (destination) for rendered audio. |sink_| must only be accessed
+ // on the pipeline thread (verify with |pipeline_thread_checker_|). |sink_|
+ // must never be called under |lock_| or the 3-way thread bridge between the
+ // audio, pipeline, and decoder threads may dead lock.
Ami GONE FROM CHROMIUM 2012/11/03 00:11:45 deadlock is one word.
DaleCurtis 2012/11/03 01:32:47 Done.
+ scoped_refptr<media::AudioRendererSink> sink_;
+
+ // Ensures certain methods are always called on the pipeline thread.
+ base::ThreadChecker pipeline_thread_checker_;
+
+ // AudioParameters constructed during Initialize() based on |decoder_|.
+ AudioParameters audio_parameters_;
+
+ // Callbacks provided during Initialize().
+ PipelineStatusCB init_cb_;
+ StatisticsCB statistics_cb_;
+ base::Closure underflow_cb_;
+ TimeCB time_cb_;
+ base::Closure ended_cb_;
+ base::Closure disabled_cb_;
+ PipelineStatusCB error_cb_;
+
+ // Callback provided to Pause(...).
Ami GONE FROM CHROMIUM 2012/11/03 00:11:45 ellipses here and below aren't useful
DaleCurtis 2012/11/03 01:32:47 Done.
+ base::Closure pause_cb_;
+ // Callback provided to Preroll(...).
+ PipelineStatusCB preroll_cb_;
+
+ // After Initialize() has completed, all variables below must be accessed
+ // under |lock_|. ------------------------------------------------------------
base::Lock lock_;
+ // Algorithm for scaling audio. Access must be guarded by |lock_| if not on
+ // the pipeline thread.
Ami GONE FROM CHROMIUM 2012/11/03 00:11:45 this is a very codesmelly comment. Why is it OK to
DaleCurtis 2012/11/03 01:32:47 Removed.
+ scoped_ptr<AudioRendererAlgorithm> algorithm_;
+
// Simple state tracking variable.
enum State {
kUninitialized,
@@ -166,6 +199,8 @@ class MEDIA_EXPORT AudioRendererImpl
kUnderflow,
kRebuffering,
};
+
+ // |state_| access must always be guarded by |lock_|
Ami GONE FROM CHROMIUM 2012/11/03 00:11:45 redundant to l.184?
DaleCurtis 2012/11/03 01:32:47 Done.
State state_;
// Keep track of our outstanding read to |decoder_|.
@@ -180,29 +215,8 @@ class MEDIA_EXPORT AudioRendererImpl
base::TimeDelta audio_time_buffered_;
base::TimeDelta current_time_;
- PipelineStatusCB init_cb_;
- StatisticsCB statistics_cb_;
-
- // Filter callbacks.
- base::Closure pause_cb_;
- PipelineStatusCB preroll_cb_;
-
- base::Closure underflow_cb_;
- TimeCB time_cb_;
- base::Closure ended_cb_;
- base::Closure disabled_cb_;
- PipelineStatusCB error_cb_;
-
base::TimeDelta preroll_timestamp_;
- uint32 bytes_per_frame_;
-
- // A flag that indicates this filter is called to stop.
- bool stopped_;
-
- // The sink (destination) for rendered audio.
- scoped_refptr<media::AudioRendererSink> sink_;
-
// We're supposed to know amount of audio data OS or hardware buffered, but
// that is not always so -- on my Linux box
// AudioBuffersState::hardware_delay_bytes never reaches 0.
@@ -219,14 +233,14 @@ class MEDIA_EXPORT AudioRendererImpl
// than nothing.
base::Time earliest_end_time_;
- AudioParameters audio_parameters_;
-
bool underflow_disabled_;
// True if the renderer receives a buffer with kAborted status during preroll,
// false otherwise. This flag is cleared on the next Preroll() call.
bool preroll_aborted_;
+ // End variables which must be accessed under |lock_|. ----------------------
+
DISALLOW_COPY_AND_ASSIGN(AudioRendererImpl);
};
« no previous file with comments | « no previous file | media/filters/audio_renderer_impl.cc » ('j') | media/filters/audio_renderer_impl.cc » ('J')

Powered by Google App Engine
This is Rietveld 408576698