Add an initial revision of an audio server.

services/media/audio/audio_track_impl.cc

+// Copyright 2015 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 <algorithm>
+#include <limits>
+
+#include "base/logging.h"
+#include "mojo/services/media/common/cpp/linear_transform.h"
+#include "services/media/audio/audio_output_manager.h"
+#include "services/media/audio/audio_server_impl.h"
+#include "services/media/audio/audio_track_impl.h"
+#include "services/media/audio/audio_track_to_output_link.h"
+
+namespace mojo {
+namespace media {
+namespace audio {
+
+constexpr size_t AudioTrackImpl::PTS_FRACTIONAL_BITS;
+
+// TODO(johngro): If there is ever a better way to do this type of static-table
+// initialization using mojom generated structs, we should switch to it.
+static const struct {
+  LpcmSampleFormat sample_format;
+  uint8_t min_samples_per_frame;
+  uint8_t max_samples_per_frame;
+  uint32_t min_frames_per_second;
+  uint32_t max_frames_per_second;
+} kSupportedLpcmTypeSets[] = {
+  {
+    .sample_format = LpcmSampleFormat::UNSIGNED_8,
+    .min_samples_per_frame = 1,
+    .max_samples_per_frame = 2,
+    .min_frames_per_second = 1000,
+    .max_frames_per_second = 48000,
+  },
+  {
+    .sample_format = LpcmSampleFormat::SIGNED_16,
+    .min_samples_per_frame = 1,
+    .max_samples_per_frame = 2,
+    .min_frames_per_second = 1000,
+    .max_frames_per_second = 48000,
+  },
+};
+
+AudioTrackImpl::AudioTrackImpl(InterfaceRequest<AudioTrack> iface,
+                             AudioServerImpl* owner)
+  : owner_(owner),
+    binding_(this),
+    pipe_(this, owner) {
+  CHECK(nullptr != owner_);
+  binding_.Bind(iface.Pass());
+}
+
+AudioTrackImpl::~AudioTrackImpl() {
+}
+
+AudioTrackImplPtr AudioTrackImpl::Create(InterfaceRequest<AudioTrack> iface,
+                                       AudioServerImpl* owner) {
+  AudioTrackImplPtr ret(new AudioTrackImpl(iface.Pass(), owner));
+  ret->weak_this_ = ret;
+  return ret;
+}
+
+void AudioTrackImpl::Describe(const DescribeCallback& cbk) {
+  // Build a minimal descriptor
+  //
+  // TODO(johngro): one day, we need to make this description much more rich and
+  // fully describe our capabilities, based on things like what outputs are
+  // available, the class of hardware we are on, and what options we were
+  // compiled with.
+  //
+  // For now, it would be nice to just be able to have a static const tree of
+  // capabilities in this translational unit which we could use to construct our
+  // message, but the nature of the structures generated by the C++ bindings
+  // make this difficult.  For now, we just create a trivial descriptor entierly
+  // by hand.
+  AudioTrackDescriptorPtr desc(AudioTrackDescriptor::New());
+
+  desc->supports_push_transport = true;
+  desc->supports_pull_transport = false;
+  desc->supported_media_types =
+    Array<MediaTypeSetPtr>::New(arraysize(kSupportedLpcmTypeSets));
+
+  for (size_t i = 0; i < desc->supported_media_types.size(); ++i) {
+    const MediaTypeSetPtr& mts =
+      (desc->supported_media_types[i] = MediaTypeSet::New());
+
+    mts->scheme  = MediaTypeScheme::LPCM;
+    mts->details = MediaTypeSetDetails::New();
+
+    const auto& s = kSupportedLpcmTypeSets[i];
+    LpcmMediaTypeSetDetailsPtr lpcm_detail = LpcmMediaTypeSetDetails::New();
+
+    lpcm_detail->sample_format         = s.sample_format;
+    lpcm_detail->min_samples_per_frame = s.min_samples_per_frame;
+    lpcm_detail->max_samples_per_frame = s.max_samples_per_frame;
+    lpcm_detail->min_frames_per_second = s.min_frames_per_second;
+    lpcm_detail->max_frames_per_second = s.max_frames_per_second;
+    mts->details->set_lpcm(lpcm_detail.Pass());
+  }
+
+  cbk.Run(desc.Pass());
+}
+
+void AudioTrackImpl::Configure(AudioTrackConfigurationPtr   configuration,
+                              InterfaceRequest<MediaPipe> req,
+                              const ConfigureCallback&    cbk) {
+  // Are we already configured?

[jeffbrown, 2015/11/04 23:43:33]

Here's a place where it would be nice to just configure at the same time the
track is created.  Reduces the state space.

[johngro, 2015/11/06 02:20:26]

Acknowledged.
Its on the TODO list.

+  if (pipe_.IsInitialized()) {
+    cbk.Run(MediaResult::BAD_STATE);
+    return;
+  }
+
+  // Check the requested configuration.
+  if ((configuration->media_type->scheme != MediaTypeScheme::LPCM) ||
+      (!configuration->media_type->details->is_lpcm())) {
+    cbk.Run(MediaResult::UNSUPPORTED_CONFIG);

[jeffbrown, 2015/11/04 23:43:33]

Assuming clients should have known better before asking for this, just close the
connection.

I presume clients will need to know a-priori what kinds of configs are supported
so if they got this far then they already screwed up.

[johngro, 2015/11/06 02:20:25]

Acknowledged.
In the close-the-connection TODO bucket.

+    return;
+  }
+
+  // Search our supported configuration sets to find one compatible with this
+  // request.
+  const auto& cfg = configuration->media_type->details->get_lpcm();
+  size_t i;
+  for (i = 0; i < arraysize(kSupportedLpcmTypeSets); ++i) {
+    const auto& cfg_set = kSupportedLpcmTypeSets[i];
+
+    if ((cfg->sample_format == cfg_set.sample_format) &&
+        (cfg->samples_per_frame >= cfg_set.min_samples_per_frame) &&
+        (cfg->samples_per_frame <= cfg_set.max_samples_per_frame) &&
+        (cfg->frames_per_second >= cfg_set.min_frames_per_second) &&
+        (cfg->frames_per_second <= cfg_set.max_frames_per_second)) {
+      break;
+    }
+  }
+
+  if (i >= arraysize(kSupportedLpcmTypeSets)) {
+    cbk.Run(MediaResult::UNSUPPORTED_CONFIG);
+    return;
+  }
+
+  // Sanity check the ratio which relates audio frames to media time.
+  int32_t  N = static_cast<int32_t>(configuration->audio_frame_ratio);

[jeffbrown, 2015/11/04 23:43:33]

numerator / denominator, or num / denom

Also, seeing int32_t and uin32_t used together for these fractions makes me
worry that the wrong types might get used someplace (possibly in the client if
not here).  We spoke earlier of using a separate bool to encode whether playback
is forward or reversed.  I think that might be worth exploring further in future
patches.

(Or make both the numerator and denominator signed at the cost of 1 bit of
precision.)

[johngro, 2015/11/06 02:20:26]

re: future efforts...
Precision is important here.  I am loathe to sacrifice it.  I am much more
inclined to add the bool to gain another bit of precision rather than losing one
and adding redundant states.

Before I go either route, I want some consensus on whether or not signed scaling
is important at all or not.  For a generalized linear transformation library, it
is.  For media, it can be, but only in the context of reverse trick play (a
phrase which makes most people make "ewwwww, yucky!" faces) and usually requires
a ton of tricksy-ness at the player level to get right anyway (where you can
make an argument that perhaps the timestamps should just be re-written there;
the one case where they should be).

It should be noted that, with some minor modification, the sampler core here
could actually (rather easily) play audio backwards without needing to re-order
the samples in the buffer or re-write timestamps.  I'm probably the only person
who thinks that is cool, however, so...

re: numerator and denominator..
Dude... You are killing me....  The values came from audio_frame_RATIO and
media_time_RATIO.  The are then fed into a LinearTransform::***RATIO*** and
multiplied by other RATIOs.

If you don't know what N and D mean in this context, ffs how did you manage to
pass the Google interviews?

Done. (but my soul is crying inside).

+  uint32_t D = static_cast<int32_t>(configuration->media_time_ratio);
+  if ((N < 1) || (D < 1)) {
+    cbk.Run(MediaResult::INVALID_ARGUMENT);
+    return;
+  }
+
+
+  // Figure out the rate we need to scale by in order to produce our fixed
+  // point timestamps.
+  LinearTransform::Ratio frac_scale(1 << PTS_FRACTIONAL_BITS, 1);
+  LinearTransform::Ratio frame_scale(LinearTransform::Ratio(N, D));
+  bool no_loss = LinearTransform::Ratio::Compose(frac_scale,
+                                                 frame_scale,
+                                                 &frame_to_media_ratio_);
+  if (!no_loss) {
+    cbk.Run(MediaResult::INVALID_ARGUMENT);
+    return;
+  }
+
+  // Figure out how many bytes we need to hold the requested number of nSec of
+  // audio.
+  switch (cfg->sample_format) {
+    case LpcmSampleFormat::UNSIGNED_8:
+      bytes_per_frame_ = 1;
+      break;
+
+    case LpcmSampleFormat::SIGNED_16:
+      bytes_per_frame_ = 2;
+      break;
+
+    case LpcmSampleFormat::SIGNED_24_IN_32:
+      bytes_per_frame_ = 4;
+      break;
+
+    default:
+      DCHECK(false);
+      bytes_per_frame_ = 2;
+      break;
+  }
+  bytes_per_frame_ *= cfg->samples_per_frame;
+
+  // Overflow trying to convert from frames to bytes?
+  uint64_t requested_frames = configuration->max_frames;
+  if (requested_frames >
+     (std::numeric_limits<size_t>::max() / bytes_per_frame_)) {
+    cbk.Run(MediaResult::INSUFFICIENT_RESOURCES);
+    return;
+  }
+
+  size_t requested_bytes = (requested_frames * bytes_per_frame_);
+
+  // Attempt to initialize our shared buffer and bind it to our interface
+  // request.
+  if (pipe_.Init(req.Pass(), requested_bytes) != MOJO_RESULT_OK) {
+    cbk.Run(MediaResult::INSUFFICIENT_RESOURCES);
+    return;
+  }
+
+  // Stash our configuration.
+  format_ = cfg->Clone();

[jeffbrown, 2015/11/04 23:43:33]

Looks to me like the config is just going to be dropped on the floor when we
return.  So we might as well just Pass() it and take ownership.

[johngro, 2015/11/06 02:20:26]

Done.

The fact that this is even allowed is a bit surprising/disturbing.  Had an
interesting conversation with Vardhan about it.  Someday over drinks, we should
discuss the ramifications which allowing this might have on reaching a minimal
copy/minimal heap allocation world when it comes to marshalling parameters
across mojom interfaces.

+
+  // Have the audio output manager initialize our set of outputs.  Note; there
+  // is currently no need for a lock here.  Methods called from our user-facing
+  // interfaces are seriailzed by nature of the mojo framework, and none of the
+  // output manager's threads should ever need to manipulate the set.  Cleanup
+  // of outputs which have gone away is currently handled in a lazy fashion when
+  // the track fails to promote its weak reference during an operation involving
+  // its outputs.
+  //
+  // TODO(johngro): someday, we will need to deal with recalculating properties
+  // which depend on a track's current set of outputs (for example, the minimum
+  // latency).  This will probably be done using a dirty flag in the track
+  // implementations, and scheduling a job to recalculate the properties for the
+  // dirty tracks and notify the users as appropriate.
+
+  // If we cannot promote our own weak pointer, something is seriously wrong.
+  AudioTrackImplPtr strong_this(weak_this_.lock());
+  DCHECK(strong_this);
+  DCHECK(owner_);
+  owner_->GetOutputManager().SelectOutputsForTrack(strong_this);
+
+  // Done
+  cbk.Run(MediaResult::OK);
+}
+
+void AudioTrackImpl::GetRateControl(InterfaceRequest<RateControl> req,
+                                   const GetRateControlCallback& cbk) {
+  cbk.Run(rate_control_.Bind(req.Pass()));
+}
+
+void AudioTrackImpl::AddOutput(AudioTrackToOutputLinkPtr link) {
+  // TODO(johngro): assert that we are on the main message loop thread.
+  DCHECK(link);
+  auto res = outputs_.emplace(link);
+  DCHECK(res.second);
+}
+
+void AudioTrackImpl::RemoveOutput(AudioTrackToOutputLinkPtr link) {
+  // TODO(johngro): assert that we are on the main message loop thread.
+  DCHECK(link);
+
+  auto iter = outputs_.find(link);
+  if (iter != outputs_.end()) {
+    outputs_.erase(iter);
+  } else {
+    // TODO(johngro): that's odd.  I can't think of a reason why we we should
+    // not be able to find this link in our set of outputs... should we log
+    // something about this?

[jeffbrown, 2015/11/04 23:43:33]

DCHECK!  We failed an internal invariant.

[johngro, 2015/11/06 02:20:26]

Done.

+  }
+}
+
+void AudioTrackImpl::OnPacketReceived(AudioPipe::AudioPacketRefPtr packet) {

[jeffbrown, 2015/11/04 23:43:33]

Do we need to DCHECK the packet?

[johngro, 2015/11/06 02:20:26]

no, but it does not hurt to do so.  I have been called out by many engineers
many times for DCHECKing pointers which I am just about to de-reference.  Their
argument is that de-reffing NULL is just going to crash anyway (and mine is that
the DCHECK/ASSERT implementation usually produces much nicer output)

Done.

+  for (const auto& output : outputs_) {
+    DCHECK(output);
+    output->PushToPendingQueue(packet);
+  }
+}
+
+void AudioTrackImpl::OnFlushRequested(const MediaPipe::FlushCallback& cbk) {
+  for (const auto& output : outputs_) {
+    DCHECK(output);

[jeffbrown, 2015/11/04 23:43:33]

I feel like this DCHECK is kind of redundant given we're already enforcing
invariants on what goes into the outputs list, but it's not harmful.

[johngro, 2015/11/06 02:20:26]

Acknowledged.

Can't have it both ways.  Either we are checking at levels like this, or we are
not.  If we are it says, and I add the DCHECK(packet) above.  If not, I strike
both of them.

I lean in the cautious direction, so DCHECKs all around!

+    output->FlushPendingQueue();
+  }
+  cbk.Run(MediaResult::OK);
+}
+
+}  // namespace audio
+}  // namespace media
+}  // namespace mojo