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

Unified Diff: media/audio/linux/alsa_output.h

Issue 7117001: Simplify AlsaPcmOutputStream and AudioManagerLinux. Code was thread-safe, but now (Closed) Base URL: http://src.chromium.org/svn/trunk/src/
Patch Set: Created 9 years, 6 months 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/audio/linux/alsa_output.cc » ('j') | media/audio/linux/alsa_output.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: media/audio/linux/alsa_output.h
===================================================================
--- media/audio/linux/alsa_output.h (revision 87982)
+++ media/audio/linux/alsa_output.h (working copy)
@@ -24,13 +24,6 @@
// threading assumptions at the top of the implementation file to avoid
// introducing race conditions between tasks posted to the internal
// message_loop, and the thread calling the public APIs.
-//
-// TODO(sergeyu): AlsaPcmOutputStream is always created and used from the
-// audio thread (i.e. |client_thread_loop_| and |message_loop_| always point
-// to the same thread), so it doesn't need to be thread-safe anymore.
-//
-// TODO(sergeyu): Remove refcounter from AlsaPcmOutputStream and use
-// ScopedRunnableMethodFactory to create tasks.
#ifndef MEDIA_AUDIO_LINUX_ALSA_OUTPUT_H_
#define MEDIA_AUDIO_LINUX_ALSA_OUTPUT_H_
@@ -40,9 +33,8 @@
#include <string>
#include "base/gtest_prod_util.h"
-#include "base/memory/ref_counted.h"
#include "base/memory/scoped_ptr.h"
-#include "base/synchronization/lock.h"
+#include "base/task.h"
#include "media/audio/audio_io.h"
#include "media/audio/audio_parameters.h"
@@ -54,9 +46,7 @@
class AudioManagerLinux;
class MessageLoop;
-class AlsaPcmOutputStream :
- public AudioOutputStream,
- public base::RefCountedThreadSafe<AlsaPcmOutputStream> {
+class AlsaPcmOutputStream : public AudioOutputStream {
public:
// String for the generic "default" ALSA device that has the highest
// compatibility and chance of working.
@@ -85,6 +75,8 @@
AudioManagerLinux* manager,
MessageLoop* message_loop);
+ virtual ~AlsaPcmOutputStream();
+
// Implementation of AudioOutputStream.
virtual bool Open();
virtual void Close();
@@ -94,7 +86,6 @@
virtual void GetVolume(double* volume);
private:
- friend class base::RefCountedThreadSafe<AlsaPcmOutputStream>;
friend class AlsaPcmOutputStreamTest;
FRIEND_TEST_ALL_PREFIXES(AlsaPcmOutputStreamTest,
AutoSelectDevice_DeviceSelect);
@@ -120,8 +111,6 @@
FRIEND_TEST_ALL_PREFIXES(AlsaPcmOutputStreamTest, WritePacket_StopStream);
FRIEND_TEST_ALL_PREFIXES(AlsaPcmOutputStreamTest, WritePacket_WriteFails);
- virtual ~AlsaPcmOutputStream();
-
// Flags indicating the state of the stream.
enum InternalState {
kInError = 0,
@@ -159,52 +148,31 @@
// Thread-asserting accessors for member variables.
AudioManagerLinux* manager();
- // Struct holding all mutable the data that must be shared by the
- // message_loop() and the thread that created the object.
- class SharedData {
- public:
- explicit SharedData(MessageLoop* state_transition_loop);
+ // Functions to safeguard state transitions. All changes to the object state
+ // should go through these functions.
+ bool CanTransitionTo(InternalState to);
+ InternalState TransitionTo(InternalState to);
+ InternalState state();
- // Functions to safeguard state transitions and ensure that transitions are
- // only allowed occuring on the thread that created the object. All
- // changes to the object state should go through these functions.
- bool CanTransitionTo(InternalState to);
- bool CanTransitionTo_Locked(InternalState to);
- InternalState TransitionTo(InternalState to);
- InternalState state();
+ float volume();
+ void set_volume(float v);
- float volume();
- void set_volume(float v);
+ // API for Proxying calls to the AudioSourceCallback provided during
+ // Start(). These APIs are threadsafe.
+ //
+ // TODO(ajwong): This is necessary because the ownership semantics for the
+ // |source_callback_| object are incorrect in AudioRenderHost. The callback
+ // is passed into the output stream, but ownership is not transfered which
+ // requires a synchronization on access of the |source_callback_| to avoid
+ // using a deleted callback.
+ uint32 OnMoreData(AudioOutputStream* stream, uint8* dest,
+ uint32 max_size, AudioBuffersState buffers_state);
+ void OnError(AudioOutputStream* stream, int code);
- // API for Proxying calls to the AudioSourceCallback provided during
- // Start(). These APIs are threadsafe.
- //
- // TODO(ajwong): This is necessary because the ownership semantics for the
- // |source_callback_| object are incorrect in AudioRenderHost. The callback
- // is passed into the output stream, but ownership is not transfered which
- // requires a synchronization on access of the |source_callback_| to avoid
- // using a deleted callback.
- uint32 OnMoreData(AudioOutputStream* stream, uint8* dest,
- uint32 max_size, AudioBuffersState buffers_state);
- void OnError(AudioOutputStream* stream, int code);
+ // Changes the AudioSourceCallback to proxy calls to. Pass in NULL to
+ // release ownership of the currently registered callback.
+ void set_source_callback(AudioSourceCallback* callback);
- // Changes the AudioSourceCallback to proxy calls to. Pass in NULL to
- // release ownership of the currently registered callback.
- void set_source_callback(AudioSourceCallback* callback);
-
- private:
- base::Lock lock_;
-
- InternalState state_;
- float volume_; // Volume level from 0.0 to 1.0.
-
- AudioSourceCallback* source_callback_;
-
- MessageLoop* const state_transition_loop_;
-
- DISALLOW_COPY_AND_ASSIGN(SharedData);
- } shared_data_;
-
// Configuration constants from the constructor. Referenceable by all threads
// since they are constants.
const std::string requested_device_name_;
@@ -242,13 +210,19 @@
scoped_ptr<media::SeekableBuffer> buffer_;
uint32 frames_per_packet_;
- // Used to check which message loop is allowed to call the public APIs.
- MessageLoop* client_thread_loop_;
-
// The message loop responsible for querying the data source, and writing to
// the output device.
MessageLoop* message_loop_;
+ // Allows us to run tasks on the AlsaPcmOutputStream instance which are
+ // bound by its lifetime.
+ ScopedRunnableMethodFactory<AlsaPcmOutputStream> method_factory_;
+
+ InternalState state_;
+ float volume_; // Volume level from 0.0 to 1.0.
+
+ AudioSourceCallback* source_callback_;
+
DISALLOW_COPY_AND_ASSIGN(AlsaPcmOutputStream);
};
« no previous file with comments | « no previous file | media/audio/linux/alsa_output.cc » ('j') | media/audio/linux/alsa_output.cc » ('J')

Powered by Google App Engine
This is Rietveld 408576698