Add Audio support in Chromoting host when using WebRTC.

remoting/protocol/webrtc_audio_source_adapter.cc

+// Copyright 2016 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/protocol/webrtc_audio_source_adapter.h"
+
+#include "base/bind.h"
+#include "base/logging.h"
+#include "base/synchronization/lock.h"
+#include "base/threading/thread_checker.h"
+#include "remoting/proto/audio.pb.h"
+#include "remoting/protocol/audio_source.h"
+
+namespace remoting {
+namespace protocol {
+
+static const int kChannels = 2;
+static const int kBytesPerSample = 2;
+
+static constexpr base::TimeDelta kAudioFrameDuration =
+    base::TimeDelta::FromMilliseconds(10);
+
+
+class WebrtcAudioSourceAdapter::Core{

[Jamie, 2016/10/04 23:36:05]

Nit: Space after Core

[Sergey Ulanov, 2016/10/05 21:52:23]

Done.

+ public:
+  Core();
+  ~Core();
+
+  void Start(std::unique_ptr<AudioSource> audio_source);
+  void Pause(bool pause);
+  void AddSink(webrtc::AudioTrackSinkInterface* sink);
+  void RemoveSink(webrtc::AudioTrackSinkInterface* sink);
+
+ private:
+  void OnAudioPacket(std::unique_ptr<AudioPacket> packet);
+
+  std::unique_ptr<AudioSource> audio_source_;
+
+  bool paused_ = false;
+
+  int sampling_rate_ = 0;
+  std::vector<uint8_t> leftover_samples_;

[Jamie, 2016/10/04 23:36:05]

I'm not sure I fully understand the use of this member. If my reading of the
code is correct, I think something like |partial_sample_| (note the singular)
might be a better name.

[Sergey Ulanov, 2016/10/05 21:52:23]

webrtc::AudioTrackSinkInterface expects to get audio in 10ms frames (see
kAudioFrameDuration), i.e. 480 samples per frame at 48kHz. But
remoting::AudioSource doesn't have any guarantees the size of the AudioPacket's
it generates (each incoming packet still contains whole samples, so
leftover_samples_ will never contains partial sample). So if we get 45ms of
samples then 40ms are passed to the sinks and 5ms is kept until we receive 5
more ms.
I've renamed this vector to |partial_frame_| now. Also added comments to make it
clear why we need it here.

+
+  base::ObserverList<webrtc::AudioTrackSinkInterface> audio_sinks_;
+  base::Lock audio_sinks_lock_;
+
+  base::ThreadChecker thread_checker_;
+};
+
+WebrtcAudioSourceAdapter::Core::Core() {
+  thread_checker_.DetachFromThread();
+}

[Jamie, 2016/10/04 23:36:05]

Nit: Add blank line

[Sergey Ulanov, 2016/10/05 21:52:23]

Done.

+WebrtcAudioSourceAdapter::Core::~Core() {}
+
+void WebrtcAudioSourceAdapter::Core::Start(
+    std::unique_ptr<AudioSource> audio_source) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  audio_source_ = std::move(audio_source);
+  audio_source_->Start(
+      base::Bind(&Core::OnAudioPacket, base::Unretained(this)));
+}
+
+void WebrtcAudioSourceAdapter::Core::Pause(bool pause) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  paused_ = pause;
+}
+
+void WebrtcAudioSourceAdapter::Core::AddSink(
+    webrtc::AudioTrackSinkInterface* sink) {
+  // Can be called on any thread.
+  base::AutoLock lock(audio_sinks_lock_);
+  audio_sinks_.AddObserver(sink);
+}
+
+void WebrtcAudioSourceAdapter::Core::RemoveSink(
+    webrtc::AudioTrackSinkInterface* sink) {
+  // Can be called on any thread.
+  base::AutoLock lock(audio_sinks_lock_);
+  audio_sinks_.RemoveObserver(sink);
+}
+
+void WebrtcAudioSourceAdapter::Core::OnAudioPacket(
+    std::unique_ptr<AudioPacket> packet) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  if (paused_)
+    return;
+
+  DCHECK_EQ(packet->channels(), kChannels);
+  DCHECK_EQ(packet->bytes_per_sample(), kBytesPerSample);
+
+  if (sampling_rate_ != packet->sampling_rate()) {
+    sampling_rate_ = packet->sampling_rate();
+    leftover_samples_.clear();
+  }
+
+  size_t samples_per_frame =
+      kAudioFrameDuration * sampling_rate_ / base::TimeDelta::FromSeconds(1);
+  size_t bytes_per_frame = kBytesPerSample * kChannels * samples_per_frame;
+
+  const std::string& data = packet->data(0);
+
+  size_t position = 0;
+
+  base::AutoLock lock(audio_sinks_lock_);
+

[Jamie, 2016/10/04 23:36:05]

If I'm reading this correctly, the intent is to ensure that each audio sink only
receives complete frames of data, even if the audio packets contain less than
that. I don't think it does that though; consider the case where two consecutive
packets arrive with total payload < bytes_per_frame. The first will be queued in
leftover_samples_, only to be sent when the second arrives; if that's the
intent, then why not send the first one as soon as it arrives, as would have
been done if it had been >= bytes_per_frame?

[Sergey Ulanov, 2016/10/05 21:52:23]

This is a good catch. I wrote this code with incorrect assumption that
AudioPacket is always bigger than a single frame (e.g. 34ms and then 26ms would
generate 6 frames, 10ms each), but it doesn't work correctly when packets are
smaller than 10ms. Fixed now. Also added a unittest for this adapter.

+  if (!leftover_samples_.empty()) {
+    size_t bytes_to_append = bytes_per_frame - leftover_samples_.size();
+    position += bytes_to_append;
+    leftover_samples_.insert(leftover_samples_.end(), data.data(),
+                             data.data() + bytes_to_append);
+    FOR_EACH_OBSERVER(webrtc::AudioTrackSinkInterface, audio_sinks_,
+                      OnData(&leftover_samples_.front(), kBytesPerSample * 8,
+                             sampling_rate_, kChannels, samples_per_frame));
+  }
+
+  while (position + bytes_per_frame <= data.size()) {
+    FOR_EACH_OBSERVER(webrtc::AudioTrackSinkInterface, audio_sinks_,
+                      OnData(data.data() + position, kBytesPerSample * 8,
+                             sampling_rate_, kChannels, samples_per_frame));
+    position += bytes_per_frame;
+  }
+
+  leftover_samples_.assign(data.data() + position, data.data() + data.size());
+}
+
+WebrtcAudioSourceAdapter::WebrtcAudioSourceAdapter(
+    scoped_refptr<base::SingleThreadTaskRunner> audio_task_runner)
+    : audio_task_runner_(audio_task_runner), core_(new Core()) {}
+
+WebrtcAudioSourceAdapter::~WebrtcAudioSourceAdapter() {
+  audio_task_runner_->DeleteSoon(FROM_HERE, core_.release());
+}
+
+void WebrtcAudioSourceAdapter::Start(
+    std::unique_ptr<AudioSource> audio_source) {
+  audio_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&Core::Start, base::Unretained(core_.get()),
+                            base::Passed(&audio_source)));
+}
+
+void WebrtcAudioSourceAdapter::Pause(bool pause) {
+  audio_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&Core::Pause, base::Unretained(core_.get()), pause));
+}
+
+WebrtcAudioSourceAdapter::SourceState WebrtcAudioSourceAdapter::state() const {
+  return kLive;
+}
+
+bool WebrtcAudioSourceAdapter::remote() const {
+  return false;
+}
+
+void WebrtcAudioSourceAdapter::RegisterAudioObserver(AudioObserver* observer) {}
+
+void WebrtcAudioSourceAdapter::UnregisterAudioObserver(
+    AudioObserver* observer) {}
+
+void WebrtcAudioSourceAdapter::AddSink(webrtc::AudioTrackSinkInterface* sink) {
+  core_->AddSink(sink);
+}
+void WebrtcAudioSourceAdapter::RemoveSink(
+    webrtc::AudioTrackSinkInterface* sink) {
+  core_->RemoveSink(sink);
+}
+
+void WebrtcAudioSourceAdapter::RegisterObserver(
+    webrtc::ObserverInterface* observer) {}
+void WebrtcAudioSourceAdapter::UnregisterObserver(
+    webrtc::ObserverInterface* observer) {}
+
+}  // namespace protocol
+}  // namespace remoting