diff options
author | tommi@chromium.org <tommi@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-12-12 21:50:00 +0000 |
---|---|---|
committer | tommi@chromium.org <tommi@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-12-12 21:50:00 +0000 |
commit | 2e77cb39b4b2760c6aa15fb194355cc429f9cb8a (patch) | |
tree | 5e90a41a0c15409cee0fca52719187b2025a6dc9 /media/tools | |
parent | 25db386820026afcaf815b8e9290817146be111e (diff) | |
download | chromium_src-2e77cb39b4b2760c6aa15fb194355cc429f9cb8a.zip chromium_src-2e77cb39b4b2760c6aa15fb194355cc429f9cb8a.tar.gz chromium_src-2e77cb39b4b2760c6aa15fb194355cc429f9cb8a.tar.bz2 |
Remove the AudioManager singleton.
Unit tests now instantiate their own AudioManager and can choose to
use the default one or provide their own mock implementation without
having to worry about conflicting with the singleton.
The teardown sequence of the AudioManager and its thread has been cleaned
up significantly and I don't think it has been completely tested before as
the audio thread was terminated before all objects that belonged to the
thread had a chance to do cleanup. The AudioManager unit tests do not use
the actual audio thread, so this part seems to have been left out.
In Chrome, the AudioManager instance is now owned by BrowserProcessImpl
and always constructed on the UI thread. This instance is then shared
in the same way that several other 'manager' type objects are shared to
'content' code, via content::ResourceContext. Audio specific classes
do though receive a direct pointer to the AudioManager and are required
to do proper reference counting if they need to hold onto the instance.
I chose to use the ResourceContext rather than direct use of g_browser_process
to avoid requiring another singleton when writing relatively simple tests
that touch the AudioManager.
I added a couple of safeguards to guard against future regressions:
- Not more than one instance of the AudioManager should be created.
- The AudioManager should not be addrefed by its own thread. This
can basically become a circular reference and prevent deterministic
shutdown.
Reviewers: Of course you're free to review everything,
but here's the breakdown in terms of the bare minimum from
the standpoint of "Owners approval". I'm asking Henrik to be the
main reviewer of the entire patch (sorry!).
Henrik: Everything minus the below, but it would be great if you could
take a look at the whole thing, specifically media/audio.
Pawel: I'd like you to take a generic look at this approach.
The key areas as far as the singleton itself goes are in
media/audio/audio_manager[_base].* and
chrome/browser/browser_process*.*
Satish: content/browser/speech/*
media/audio/audio_manager_base.* (new reference counting code)
Andrew: content/browser/renderer_host/media/*
content/renderer/media/webrtc_audio_device_unittest.cc (Owner)
Avi: content/browser/renderer_host/render_process_host_impl.cc
content/browser/resource_context.*
William: chrome/browser/profiles/profile_io_data.cc
chrome/browser/browser_process*.*
Robert: This is basically a heads up. I hope that I didn't break the OpenBSD
implementation, but unfortunately I have no way of knowing for sure.
Shijing: Please take a look at AudioManagerLinux. I replaced the set of
active streams with a simple counter.
BUG=105249
TEST=content_unittests, media_unittests, browser_tests.
Review URL: http://codereview.chromium.org/8818012
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@114084 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'media/tools')
-rw-r--r-- | media/tools/player_wtl/movie.cc | 7 | ||||
-rw-r--r-- | media/tools/player_wtl/movie.h | 4 | ||||
-rw-r--r-- | media/tools/player_x11/player_x11.cc | 25 |
3 files changed, 26 insertions, 10 deletions
diff --git a/media/tools/player_wtl/movie.cc b/media/tools/player_wtl/movie.cc index 9b298ca..b6bd2f8 100644 --- a/media/tools/player_wtl/movie.cc +++ b/media/tools/player_wtl/movie.cc @@ -7,6 +7,7 @@ #include "base/memory/singleton.h" #include "base/threading/platform_thread.h" #include "base/utf_string_conversions.h" +#include "media/audio/audio_manager.h" #include "media/base/filter_collection.h" #include "media/base/media_log.h" #include "media/base/message_loop_factory_impl.h" @@ -30,7 +31,8 @@ using media::ReferenceAudioRenderer; namespace media { Movie::Movie() - : enable_audio_(true), + : audio_manager_(AudioManager::Create()), + enable_audio_(true), enable_draw_(true), enable_dump_yuv_file_(false), enable_pause_(false), @@ -78,7 +80,8 @@ bool Movie::Open(const wchar_t* url, WtlVideoRenderer* video_renderer) { message_loop_factory_->GetMessageLoop("VideoDecoderThread"))); if (enable_audio_) { - collection->AddAudioRenderer(new ReferenceAudioRenderer()); + collection->AddAudioRenderer( + new ReferenceAudioRenderer(audio_manager_)); } else { collection->AddAudioRenderer(new media::NullAudioRenderer()); } diff --git a/media/tools/player_wtl/movie.h b/media/tools/player_wtl/movie.h index afe80be..cd24c88 100644 --- a/media/tools/player_wtl/movie.h +++ b/media/tools/player_wtl/movie.h @@ -13,6 +13,7 @@ #include "base/memory/scoped_ptr.h" #include "media/base/message_loop_factory.h" +class AudioManager; template <typename T> struct DefaultSingletonTraits; class WtlVideoRenderer; @@ -22,7 +23,7 @@ class PipelineImpl; class Movie { public: - // Returns the singleton instance. + // Returns the singleton instance. static Movie* GetInstance(); // Open a movie. @@ -84,6 +85,7 @@ class Movie { scoped_refptr<PipelineImpl> pipeline_; scoped_ptr<media::MessageLoopFactory> message_loop_factory_; + scoped_refptr<AudioManager> audio_manager_; bool enable_audio_; bool enable_draw_; diff --git a/media/tools/player_x11/player_x11.cc b/media/tools/player_x11/player_x11.cc index 323f250..d563328 100644 --- a/media/tools/player_x11/player_x11.cc +++ b/media/tools/player_x11/player_x11.cc @@ -2,10 +2,11 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include <iostream> -#include <signal.h> #include <X11/keysym.h> #include <X11/Xlib.h> +#include <signal.h> + +#include <iostream> // NOLINT #include "base/at_exit.h" #include "base/bind.h" @@ -14,6 +15,7 @@ #include "base/memory/scoped_ptr.h" #include "base/threading/platform_thread.h" #include "base/threading/thread.h" +#include "media/audio/audio_manager.h" #include "media/base/filter_collection.h" #include "media/base/media.h" #include "media/base/media_log.h" @@ -33,6 +35,8 @@ static Display* g_display = NULL; static Window g_window = 0; static bool g_running = false; +AudioManager* g_audio_manager = NULL; + class MessageLoopQuitter { public: explicit MessageLoopQuitter(MessageLoop* loop) : loop_(loop) {} @@ -102,10 +106,12 @@ bool InitPipeline(MessageLoop* message_loop, new X11VideoRenderer(g_display, g_window, paint_message_loop)); } - if (enable_audio) - collection->AddAudioRenderer(new media::ReferenceAudioRenderer()); - else + if (enable_audio) { + collection->AddAudioRenderer( + new media::ReferenceAudioRenderer(g_audio_manager)); + } else { collection->AddAudioRenderer(new media::NullAudioRenderer()); + } // Create the pipeline and start it. *pipeline = new media::PipelineImpl(message_loop, new media::MediaLog()); @@ -176,7 +182,7 @@ void PeriodicalUpdate( base::Unretained(quitter))); return; } else if (key == XK_space) { - if (pipeline->GetPlaybackRate() < 0.01f) // paused + if (pipeline->GetPlaybackRate() < 0.01f) // paused pipeline->SetPlaybackRate(1.0f); else pipeline->SetPlaybackRate(0.0f); @@ -194,6 +200,9 @@ void PeriodicalUpdate( } int main(int argc, char** argv) { + scoped_refptr<AudioManager> audio_manager(AudioManager::Create()); + g_audio_manager = audio_manager; + // Read arguments. if (argc == 1) { std::cout << "Usage: " << argv[0] << " --file=FILE" << std::endl @@ -252,7 +261,7 @@ int main(int argc, char** argv) { message_loop.PostTask(FROM_HERE, base::Bind( &PeriodicalUpdate, pipeline, &message_loop, audio_only)); message_loop.Run(); - } else{ + } else { std::cout << "Pipeline initialization failed..." << std::endl; } @@ -262,5 +271,7 @@ int main(int argc, char** argv) { thread->Stop(); XDestroyWindow(g_display, g_window); XCloseDisplay(g_display); + g_audio_manager = NULL; + return 0; } |