From 22d7f3b7dd0f70efe089a6e823739040d42b1690 Mon Sep 17 00:00:00 2001 From: "kxing@chromium.org" Date: Sat, 4 Aug 2012 01:15:20 +0000 Subject: Piping for audio decoding. Review URL: https://chromiumcodereview.appspot.com/10843031 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@149990 0039d316-1c4b-4281-b951-d872f2087c98 --- remoting/client/DEPS | 1 + remoting/client/audio_decode_scheduler.cc | 58 ++++++++++++++++++++++++++ remoting/client/audio_decode_scheduler.h | 59 +++++++++++++++++++++++++++ remoting/client/chromoting_client.cc | 23 ++++++----- remoting/client/chromoting_client.h | 18 ++++---- remoting/client/client_context.cc | 9 +++- remoting/client/client_context.h | 6 ++- remoting/client/plugin/chromoting_instance.cc | 7 ++-- remoting/client/plugin/chromoting_instance.h | 1 - remoting/codec/DEPS | 1 + remoting/codec/audio_decoder.cc | 25 ++++++++++++ remoting/codec/audio_decoder.h | 30 ++++++++++++++ remoting/codec/audio_decoder_verbatim.cc | 24 +++++++++++ remoting/codec/audio_decoder_verbatim.h | 31 ++++++++++++++ remoting/remoting.gyp | 6 +++ 15 files changed, 271 insertions(+), 28 deletions(-) create mode 100644 remoting/client/audio_decode_scheduler.cc create mode 100644 remoting/client/audio_decode_scheduler.h create mode 100644 remoting/codec/audio_decoder.cc create mode 100644 remoting/codec/audio_decoder.h create mode 100644 remoting/codec/audio_decoder_verbatim.cc create mode 100644 remoting/codec/audio_decoder_verbatim.h diff --git a/remoting/client/DEPS b/remoting/client/DEPS index 03cd3a6..235d972 100644 --- a/remoting/client/DEPS +++ b/remoting/client/DEPS @@ -3,6 +3,7 @@ include_rules = [ "+jingle/glue", "+net", + "+remoting/codec", "+remoting/protocol", "+remoting/jingle_glue", ] diff --git a/remoting/client/audio_decode_scheduler.cc b/remoting/client/audio_decode_scheduler.cc new file mode 100644 index 0000000..7dc0d3c --- /dev/null +++ b/remoting/client/audio_decode_scheduler.cc @@ -0,0 +1,58 @@ +// 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 "remoting/client/audio_decode_scheduler.h" + +#include "base/bind.h" +#include "base/location.h" +#include "base/single_thread_task_runner.h" +#include "remoting/client/audio_player.h" +#include "remoting/codec/audio_decoder.h" +#include "remoting/proto/audio.pb.h" + +namespace remoting { + +AudioDecodeScheduler::AudioDecodeScheduler( + scoped_refptr main_task_runner, + scoped_refptr audio_decode_task_runner, + scoped_ptr audio_player) + : main_task_runner_(main_task_runner), + audio_decode_task_runner_(audio_decode_task_runner), + audio_player_(audio_player.Pass()) { +} + +AudioDecodeScheduler::~AudioDecodeScheduler() { +} + +void AudioDecodeScheduler::Initialize(const protocol::SessionConfig& config) { + DCHECK(main_task_runner_->BelongsToCurrentThread()); + decoder_.reset(AudioDecoder::CreateAudioDecoder(config).release()); +} + +void AudioDecodeScheduler::ProcessAudioPacket(scoped_ptr packet, + const base::Closure& done) { + DCHECK(main_task_runner_->BelongsToCurrentThread()); + audio_decode_task_runner_->PostTask(FROM_HERE, base::Bind( + &AudioDecodeScheduler::DecodePacket, base::Unretained(this), + base::Passed(&packet), done)); +} + +void AudioDecodeScheduler::DecodePacket(scoped_ptr packet, + const base::Closure& done) { + DCHECK(audio_decode_task_runner_->BelongsToCurrentThread()); + scoped_ptr decoded_packet = decoder_->Decode(packet.Pass()); + + main_task_runner_->PostTask(FROM_HERE, base::Bind( + &AudioDecodeScheduler::ProcessDecodedPacket, base::Unretained(this), + base::Passed(decoded_packet.Pass()), done)); +} + +void AudioDecodeScheduler::ProcessDecodedPacket(scoped_ptr packet, + const base::Closure& done) { + DCHECK(main_task_runner_->BelongsToCurrentThread()); + audio_player_->ProcessAudioPacket(packet.Pass()); + done.Run(); +} + +} // namespace remoting diff --git a/remoting/client/audio_decode_scheduler.h b/remoting/client/audio_decode_scheduler.h new file mode 100644 index 0000000..be5cfcf --- /dev/null +++ b/remoting/client/audio_decode_scheduler.h @@ -0,0 +1,59 @@ +// 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. + +#ifndef REMOTING_CLIENT_AUDIO_DECODE_SCHEDULER_H_ +#define REMOTING_CLIENT_AUDIO_DECODE_SCHEDULER_H_ + +#include "base/memory/ref_counted.h" +#include "base/memory/scoped_ptr.h" +#include "remoting/protocol/audio_stub.h" + +namespace base { +class SingleThreadTaskRunner; +} // namespace base + +namespace remoting { + +namespace protocol { +class SessionConfig; +} // namespace protocol + +class AudioDecoder; +class AudioPacket; +class AudioPlayer; + +class AudioDecodeScheduler : public protocol::AudioStub { + public: + AudioDecodeScheduler( + scoped_refptr main_task_runner, + scoped_refptr audio_decode_task_runner, + scoped_ptr audio_player); + virtual ~AudioDecodeScheduler(); + + // Initializes decoder with the information from the protocol config. + void Initialize(const protocol::SessionConfig& config); + + // AudioStub implementation. + virtual void ProcessAudioPacket(scoped_ptr packet, + const base::Closure& done) OVERRIDE; + + private: + // Called on the audio decoder thread. + void DecodePacket(scoped_ptr packet, const base::Closure& done); + + // Called on the main thread. + void ProcessDecodedPacket(scoped_ptr packet, + const base::Closure& done); + + scoped_refptr main_task_runner_; + scoped_refptr audio_decode_task_runner_; + scoped_ptr decoder_; + scoped_ptr audio_player_; + + DISALLOW_COPY_AND_ASSIGN(AudioDecodeScheduler); +}; + +} // namespace remoting + +#endif // REMOTING_CLIENT_AUDIO_DECODE_SCHEDULER_H_ diff --git a/remoting/client/chromoting_client.cc b/remoting/client/chromoting_client.cc index f0dd8c7..7c05d83 100644 --- a/remoting/client/chromoting_client.cc +++ b/remoting/client/chromoting_client.cc @@ -5,6 +5,7 @@ #include "remoting/client/chromoting_client.h" #include "base/bind.h" +#include "remoting/client/audio_decode_scheduler.h" #include "remoting/client/audio_player.h" #include "remoting/client/client_context.h" #include "remoting/client/client_user_interface.h" @@ -31,20 +32,23 @@ ChromotingClient::QueuedVideoPacket::~QueuedVideoPacket() { ChromotingClient::ChromotingClient( const ClientConfig& config, - scoped_refptr task_runner, + ClientContext* client_context, protocol::ConnectionToHost* connection, ClientUserInterface* user_interface, RectangleUpdateDecoder* rectangle_decoder, - AudioPlayer* audio_player) + scoped_ptr audio_player) : config_(config), - task_runner_(task_runner), + task_runner_(client_context->main_task_runner()), connection_(connection), user_interface_(user_interface), rectangle_decoder_(rectangle_decoder), - audio_player_(audio_player), packet_being_processed_(false), last_sequence_number_(0), weak_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) { + audio_decode_scheduler_.reset(new AudioDecodeScheduler( + client_context->main_task_runner(), + client_context->audio_decode_task_runner(), + audio_player.Pass())); } ChromotingClient::~ChromotingClient() { @@ -65,7 +69,8 @@ void ChromotingClient::Start( connection_->Connect(xmpp_proxy, config_.local_jid, config_.host_jid, config_.host_public_key, transport_factory.Pass(), - authenticator.Pass(), this, this, this, this, this); + authenticator.Pass(), this, this, this, this, + audio_decode_scheduler_.get()); } void ChromotingClient::Stop(const base::Closure& shutdown_task) { @@ -141,12 +146,6 @@ int ChromotingClient::GetPendingVideoPackets() { return received_packets_.size(); } -void ChromotingClient::ProcessAudioPacket(scoped_ptr packet, - const base::Closure& done) { - audio_player_->ProcessAudioPacket(packet.Pass()); - done.Run(); -} - void ChromotingClient::DispatchPacket() { DCHECK(task_runner_->BelongsToCurrentThread()); CHECK(!packet_being_processed_); @@ -217,6 +216,8 @@ void ChromotingClient::Initialize() { // Initialize the decoder. rectangle_decoder_->Initialize(connection_->config()); + if (connection_->config().is_audio_enabled()) + audio_decode_scheduler_->Initialize(connection_->config()); } } // namespace remoting diff --git a/remoting/client/chromoting_client.h b/remoting/client/chromoting_client.h index 377badb..f2355f1 100644 --- a/remoting/client/chromoting_client.h +++ b/remoting/client/chromoting_client.h @@ -15,7 +15,6 @@ #include "base/time.h" #include "remoting/client/client_config.h" #include "remoting/client/chromoting_stats.h" -#include "remoting/protocol/audio_stub.h" #include "remoting/protocol/client_stub.h" #include "remoting/protocol/clipboard_stub.h" #include "remoting/protocol/connection_to_host.h" @@ -33,23 +32,24 @@ namespace protocol { class TransportFactory; } // namespace protocol +class AudioDecodeScheduler; class AudioPlayer; +class ClientContext; class ClientUserInterface; class RectangleUpdateDecoder; // TODO(sergeyu): Move VideoStub implementation to RectangleUpdateDecoder. class ChromotingClient : public protocol::ConnectionToHost::HostEventCallback, public protocol::ClientStub, - public protocol::VideoStub, - public protocol::AudioStub { + public protocol::VideoStub { public: // Objects passed in are not owned by this class. ChromotingClient(const ClientConfig& config, - scoped_refptr task_runner, + ClientContext* client_context, protocol::ConnectionToHost* connection, ClientUserInterface* user_interface, RectangleUpdateDecoder* rectangle_decoder, - AudioPlayer* audio_player); + scoped_ptr audio_player); virtual ~ChromotingClient(); @@ -80,10 +80,6 @@ class ChromotingClient : public protocol::ConnectionToHost::HostEventCallback, const base::Closure& done) OVERRIDE; virtual int GetPendingVideoPackets() OVERRIDE; - // AudioStub implementation. - virtual void ProcessAudioPacket(scoped_ptr packet, - const base::Closure& done) OVERRIDE; - private: struct QueuedVideoPacket { QueuedVideoPacket(scoped_ptr packet, @@ -112,8 +108,10 @@ class ChromotingClient : public protocol::ConnectionToHost::HostEventCallback, scoped_refptr task_runner_; protocol::ConnectionToHost* connection_; ClientUserInterface* user_interface_; + // TODO(kxing): Make ChromotingClient own RectangleUpdateDecoder. RectangleUpdateDecoder* rectangle_decoder_; - AudioPlayer* audio_player_; + + scoped_ptr audio_decode_scheduler_; // If non-NULL, this is called when the client is done. base::Closure client_done_; diff --git a/remoting/client/client_context.cc b/remoting/client/client_context.cc index 7335dc3..7dcbdf8 100644 --- a/remoting/client/client_context.cc +++ b/remoting/client/client_context.cc @@ -8,7 +8,8 @@ namespace remoting { ClientContext::ClientContext(base::SingleThreadTaskRunner* main_task_runner) : main_task_runner_(main_task_runner), - decode_thread_("ChromotingClientDecodeThread") { + decode_thread_("ChromotingClientDecodeThread"), + audio_decode_thread_("ChromotingClientAudioDecodeThread") { } ClientContext::~ClientContext() { @@ -17,11 +18,13 @@ ClientContext::~ClientContext() { void ClientContext::Start() { // Start all the threads. decode_thread_.Start(); + audio_decode_thread_.Start(); } void ClientContext::Stop() { // Stop all the threads. decode_thread_.Stop(); + audio_decode_thread_.Stop(); } base::SingleThreadTaskRunner* ClientContext::main_task_runner() { @@ -32,4 +35,8 @@ base::SingleThreadTaskRunner* ClientContext::decode_task_runner() { return decode_thread_.message_loop_proxy(); } +base::SingleThreadTaskRunner* ClientContext::audio_decode_task_runner() { + return audio_decode_thread_.message_loop_proxy(); +} + } // namespace remoting diff --git a/remoting/client/client_context.h b/remoting/client/client_context.h index 27a4416..19c36f3 100644 --- a/remoting/client/client_context.h +++ b/remoting/client/client_context.h @@ -29,13 +29,17 @@ class ClientContext { base::SingleThreadTaskRunner* main_task_runner(); base::SingleThreadTaskRunner* decode_task_runner(); + base::SingleThreadTaskRunner* audio_decode_task_runner(); private: scoped_refptr main_task_runner_; - // A thread that handles all decode operations. + // A thread that handles all video decode operations. base::Thread decode_thread_; + // A thread that handles all audio decode operations. + base::Thread audio_decode_thread_; + DISALLOW_COPY_AND_ASSIGN(ClientContext); }; diff --git a/remoting/client/plugin/chromoting_instance.cc b/remoting/client/plugin/chromoting_instance.cc index b58bea22..4f07f25 100644 --- a/remoting/client/plugin/chromoting_instance.cc +++ b/remoting/client/plugin/chromoting_instance.cc @@ -470,11 +470,11 @@ void ChromotingInstance::Connect(const ClientConfig& config) { jingle_glue::JingleThreadWrapper::EnsureForCurrentThread(); host_connection_.reset(new protocol::ConnectionToHost(true)); - audio_player_.reset(new PepperAudioPlayer(this)); - client_.reset(new ChromotingClient(config, context_.main_task_runner(), + scoped_ptr audio_player(new PepperAudioPlayer(this)); + client_.reset(new ChromotingClient(config, &context_, host_connection_.get(), this, rectangle_decoder_.get(), - audio_player_.get())); + audio_player.Pass())); // Construct the input pipeline mouse_input_filter_.reset( @@ -536,7 +536,6 @@ void ChromotingInstance::Disconnect() { input_handler_.reset(); input_tracker_.reset(); mouse_input_filter_.reset(); - audio_player_.reset(); host_connection_.reset(); } diff --git a/remoting/client/plugin/chromoting_instance.h b/remoting/client/plugin/chromoting_instance.h index 8e9ac8a..f66f1e0 100644 --- a/remoting/client/plugin/chromoting_instance.h +++ b/remoting/client/plugin/chromoting_instance.h @@ -199,7 +199,6 @@ class ChromotingInstance : #endif KeyEventMapper key_mapper_; scoped_ptr input_handler_; - scoped_ptr audio_player_; scoped_ptr client_; // XmppProxy is a refcounted interface used to perform thread-switching and diff --git a/remoting/codec/DEPS b/remoting/codec/DEPS index 9e5dc3c..6076cf2 100644 --- a/remoting/codec/DEPS +++ b/remoting/codec/DEPS @@ -1,3 +1,4 @@ include_rules = [ "+google/protobuf", + "+remoting/protocol", ] diff --git a/remoting/codec/audio_decoder.cc b/remoting/codec/audio_decoder.cc new file mode 100644 index 0000000..57a6e1c --- /dev/null +++ b/remoting/codec/audio_decoder.cc @@ -0,0 +1,25 @@ +// 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 "remoting/codec/audio_decoder.h" + +#include "base/logging.h" +#include "remoting/codec/audio_decoder_verbatim.h" +#include "remoting/protocol/session_config.h" + +namespace remoting { + +scoped_ptr AudioDecoder::CreateAudioDecoder( + const protocol::SessionConfig& config) { + const protocol::ChannelConfig& audio_config = config.audio_config(); + + if (audio_config.codec == protocol::ChannelConfig::CODEC_VERBATIM) { + return scoped_ptr(new AudioDecoderVerbatim()); + } + + NOTIMPLEMENTED(); + return scoped_ptr(NULL); +} + +} // namespace remoting diff --git a/remoting/codec/audio_decoder.h b/remoting/codec/audio_decoder.h new file mode 100644 index 0000000..72eedf2 --- /dev/null +++ b/remoting/codec/audio_decoder.h @@ -0,0 +1,30 @@ +// 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. + +#ifndef REMOTING_CODEC_AUDIO_DECODER_H_ +#define REMOTING_CODEC_AUDIO_DECODER_H_ + +#include "base/memory/scoped_ptr.h" + +namespace remoting { + +namespace protocol { +class SessionConfig; +} // namespace protocol + +class AudioPacket; + +class AudioDecoder { + public: + static scoped_ptr CreateAudioDecoder( + const protocol::SessionConfig& config); + + virtual ~AudioDecoder() {} + + virtual scoped_ptr Decode(scoped_ptr packet) = 0; +}; + +} // namespace remoting + +#endif // REMOTING_CODEC_AUDIO_DECODER_H_ diff --git a/remoting/codec/audio_decoder_verbatim.cc b/remoting/codec/audio_decoder_verbatim.cc new file mode 100644 index 0000000..8a69705 --- /dev/null +++ b/remoting/codec/audio_decoder_verbatim.cc @@ -0,0 +1,24 @@ +// 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 "remoting/codec/audio_decoder_verbatim.h" + +#include "base/logging.h" +#include "remoting/proto/audio.pb.h" + +namespace remoting { + +AudioDecoderVerbatim::AudioDecoderVerbatim() { +} + +AudioDecoderVerbatim::~AudioDecoderVerbatim() { +} + +scoped_ptr AudioDecoderVerbatim::Decode( + scoped_ptr packet) { + DCHECK_EQ(AudioPacket::ENCODING_RAW, packet->encoding()); + return packet.Pass(); +} + +} // namespace remoting diff --git a/remoting/codec/audio_decoder_verbatim.h b/remoting/codec/audio_decoder_verbatim.h new file mode 100644 index 0000000..b2d967e --- /dev/null +++ b/remoting/codec/audio_decoder_verbatim.h @@ -0,0 +1,31 @@ +// 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. + +#ifndef REMOTING_CODEC_AUDIO_DECODER_VERBATIM_H_ +#define REMOTING_CODEC_AUDIO_DECODER_VERBATIM_H_ + +#include "remoting/codec/audio_decoder.h" + +#include "base/memory/scoped_ptr.h" + +namespace remoting { + +class AudioPacket; + +// The decoder for raw audio streams. +class AudioDecoderVerbatim : public AudioDecoder { + public: + AudioDecoderVerbatim(); + virtual ~AudioDecoderVerbatim(); + + virtual scoped_ptr Decode( + scoped_ptr packet) OVERRIDE; + + private: + DISALLOW_COPY_AND_ASSIGN(AudioDecoderVerbatim); +}; + +} // namespace remoting + +#endif // REMOTING_CODEC_AUDIO_DECODER_VERBATIM_H_ diff --git a/remoting/remoting.gyp b/remoting/remoting.gyp index 02910e9..9fc0bfd 100644 --- a/remoting/remoting.gyp +++ b/remoting/remoting.gyp @@ -1194,6 +1194,10 @@ 'base/util.h', # TODO(kxing): Seperate the audio and video codec files into a separate # target. + 'codec/audio_decoder.cc', + 'codec/audio_decoder.h', + 'codec/audio_decoder_verbatim.cc', + 'codec/audio_decoder_verbatim.h', 'codec/audio_encoder.h', 'codec/audio_encoder_verbatim.cc', 'codec/audio_encoder_verbatim.h', @@ -1401,6 +1405,8 @@ 'remoting_protocol', ], 'sources': [ + 'client/audio_decode_scheduler.cc', + 'client/audio_decode_scheduler.h', 'client/audio_player.h', 'client/chromoting_client.cc', 'client/chromoting_client.h', -- cgit v1.1