Add an ALSA output to the motown audio server.

services/media/audio/platform/linux/alsa_output.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 <limits>
+#include <set>
+
+#include "mojo/services/media/common/cpp/local_time.h"
+#include "services/media/audio/platform/linux/alsa_output.h"
+
+namespace mojo {
+namespace media {
+namespace audio {
+
+static constexpr LocalDuration TGT_LATENCY = local_time::from_msec(35);

[jeffbrown, 2015/11/04 20:33:42]

TGT is pretty opaque as acronyms go, prefer TARGET_LATENCY

[johngro, 2015/11/06 20:22:07]

Done.

+static constexpr LocalDuration LOW_BUF_THRESH = local_time::from_msec(30);
+static constexpr LocalDuration ERROR_RECOVERY_TIME = local_time::from_msec(300);
+static constexpr LocalDuration WAIT_FOR_ALSA_DELAY = local_time::from_usec(500);
+static const std::set<uint8_t> SUPPORTED_CHANNEL_COUNTS({ 1, 2 });
+static const std::set<uint32_t> SUPPORTED_SAMPLE_RATES({
+    48000, 32000, 24000, 16000, 8000, 4000,
+    44100, 22050, 11025,
+});
+static const std::set<int> RECOVERABLE_ALSA_ERRORS({
+    -EINTR, -EPIPE, -ESTRPIPE,

[jeffbrown, 2015/11/04 20:33:42]

Kind of odd to put these in a set<> given that it'll take longer to evaluate
than a switch statement and that the response to each of these errors may
differ.  Particularly EINTR.

(oh man, can we make out C library just deal with EINTR internally?  I'm so sick
of it with Bionic.  For the one or two times I really care... ugh.)

[johngro, 2015/11/06 20:22:07]

It just makes the code a bit more readable and maintainable than a switch IMO.
I'll replace with a switch instead, it just turns 3 lines of code into 10.

+});
+
+AlsaOutput::AlsaOutput(AudioOutputManager* manager)
+  : StandardOutputBase(manager) { }
+
+AlsaOutput::~AlsaOutput() {
+  // We should have been cleaned up already, but in release builds, call cleanup
+  // anyway, just in case something got missed.

[jeffbrown, 2015/11/04 20:33:42]

Why not just CHECK in release mode?  Yeah we'll crash but it's better than
having dead code.

[johngro, 2015/11/06 20:22:07]

A call to cleanup might (someday) do more than just close the device.  I don't
consider it to be dead code, I consider it to be structure provided so that
future maintainers have a clear place to put idempotent cleanup/shutdown tasks.

re: checking in release mode.
If a failed check here posed a potential security risk, or implied some form
lingering corruption which was going to blow up in a strange way hours from now,
I think a CHECK would be warranted.  As it stands, I don't think that either is
the case.  This is strange, and it should not happen, but we can probably
soldier on and be OK.  The DCHECK should catch most mistakes during internal
testing.

+  DCHECK(!alsa_device_);
+  Cleanup();
+}
+
+AudioOutputPtr AlsaOutput::New(AudioOutputManager* manager) {
+  return AudioOutputPtr(new AlsaOutput(manager));
+}
+
+MediaResult AlsaOutput::Configure(LpcmMediaTypeDetailsPtr config) {
+  if (!config) { return MediaResult::INVALID_ARGUMENT; }
+  if (output_format_) { return MediaResult::BAD_STATE; }
+
+  uint32_t bytes_per_sample;
+  switch (config->sample_format) {
+  case LpcmSampleFormat::UNSIGNED_8:
+    alsa_format_ = SND_PCM_FORMAT_U8;
+    silence_byte_ = 0x80;
+    bytes_per_sample = 1;
+    break;
+
+  case LpcmSampleFormat::SIGNED_16:
+    alsa_format_ = SND_PCM_FORMAT_S16;
+    silence_byte_ = 0x00;
+    bytes_per_sample = 2;
+    break;
+
+  case LpcmSampleFormat::SIGNED_24_IN_32:
+  default:
+    return MediaResult::UNSUPPORTED_CONFIG;
+  }
+
+  if (SUPPORTED_SAMPLE_RATES.find(config->frames_per_second) ==
+      SUPPORTED_SAMPLE_RATES.end()) {
+    return MediaResult::UNSUPPORTED_CONFIG;
+  }
+
+  if (SUPPORTED_CHANNEL_COUNTS.find(config->samples_per_frame) ==
+      SUPPORTED_CHANNEL_COUNTS.end()) {
+    return MediaResult::UNSUPPORTED_CONFIG;
+  }
+
+  // Compute the ratio between frames and local time ticks.
+  LinearTransform::Ratio sec_per_tick(LocalDuration::period::num,
+                                      LocalDuration::period::den);
+  LinearTransform::Ratio frames_per_sec(config->frames_per_second, 1);
+  bool is_precise = LinearTransform::Ratio::Compose(frames_per_sec,
+                                                    sec_per_tick,
+                                                    &frames_per_tick_);
+  DCHECK(is_precise);
+
+  // Figure out how many bytes there are per frame.
+  output_bytes_per_frame_ = bytes_per_sample * config->samples_per_frame;
+
+  // Success
+  output_format_ = config.Pass();
+  return MediaResult::OK;
+}
+
+MediaResult AlsaOutput::Init() {
+  if (!output_format_) { return MediaResult::BAD_STATE; }
+  if (alsa_device_) { return MediaResult::BAD_STATE; }
+
+  snd_pcm_sframes_t res;
+  res = snd_pcm_open(&alsa_device_,
+                     "default",
+                     SND_PCM_STREAM_PLAYBACK,
+                     SND_PCM_NONBLOCK);
+  if (res != 0) {
+    // TODO(johngro): log something here?

[jeffbrown, 2015/11/04 20:33:42]

Yeah.  LOG(ERROR) << "You're hosed."  ;)

[johngro, 2015/11/06 20:22:07]

Done.

+    DCHECK(!alsa_device_);

[jeffbrown, 2015/11/04 20:33:42]

This DCHECK seems unnecessary given your test at the top of this function.

[johngro, 2015/11/06 20:22:06]

Assuming that ALSA never has any bugs, but yes...

+    return MediaResult::INTERNAL_ERROR;
+  }
+
+  res = snd_pcm_set_params(alsa_device_,
+                           alsa_format_,
+                           SND_PCM_ACCESS_RW_INTERLEAVED,
+                           output_format_->samples_per_frame,
+                           output_format_->frames_per_second,
+                           1,   // allow ALSA resample

[jeffbrown, 2015/11/04 20:33:42]

Hmm.  Guess we'll disable this in the future.

[johngro, 2015/11/06 20:22:07]

how about right now?  Not sure how this got set TBH.  Probably cut-n-paste from
my earlier proto-server.

+                           local_time::to_usec<unsigned int>(TGT_LATENCY));
+  if (res) {
+    // TODO(johngro): log something here?

[jeffbrown, 2015/11/04 20:33:42]

Please do.  ALSA is such a pain to debug when bringing up devices.

[johngro, 2015/11/06 20:22:07]

Done.

+    Cleanup();
+    return MediaResult::INTERNAL_ERROR;
+  }
+
+  // Figure out how big our mixing buffer needs to be, then allocate it.
+  res = snd_pcm_avail_update(alsa_device_);
+  if (res <= 0) {
+    LOG(ERROR) << "[" << this << "] : "
+               << "Fatal error (" << res
+               << ") attempting to determine ALSA buffer size.";
+    Cleanup();
+    return MediaResult::INTERNAL_ERROR;
+  }
+
+  mix_buf_frames_ = res;
+  mix_buf_bytes_  = res * output_bytes_per_frame_;
+  mix_buf_.reset(new uint8_t[mix_buf_bytes_]);
+
+  return MediaResult::OK;
+}
+
+void AlsaOutput::Cleanup() {
+  if (alsa_device_) {
+    snd_pcm_close(alsa_device_);
+    alsa_device_ = nullptr;
+  }
+
+  mix_buf_ = nullptr;
+  mix_buf_bytes_ = 0;
+  mix_buf_frames_ = 0;
+}
+
+bool AlsaOutput::StartMixJob(MixJob* job, const LocalTime& process_start) {
+  DCHECK(job);
+
+  // Are we not primed?  If so, fill a mix buffer with silence and send it to
+  // the alsa device.  Schedule a callback for a short time in the future so
+  // ALSA has a chance to start the output and we can take our best guess of the
+  // function which maps output frames to local time.
+  if (!primed_) {
+    HandleAsUnderflow();
+    return false;
+  }
+
+  // Figure out how many frames of audio we need to produce in order to top off
+  // the buffer.  If we are primed, but do not know the transformation between
+  // audio frames and local time ticks, do our best to figure it out in the
+  // process.
+  snd_pcm_sframes_t avail;
+  if (!local_to_output_known_) {
+    snd_pcm_sframes_t delay;
+
+    int res = snd_pcm_avail_delay(alsa_device_, &avail, &delay);
+    LocalTime now = LocalClock::now();
+
+    if (res < 0) {
+      HandleAlsaError(res);
+      return false;
+    }
+
+    DCHECK_GE(delay, 0);
+    int64_t now_ticks = now.time_since_epoch().count();

[jeffbrown, 2015/11/04 20:33:42]

Looking at this conversion to int64_t, I wonder whether it would make sense to
specialize LinearTransform in relation to std::chrono::duration types.  Kind of
weird maybe and it would make the math more annoying.  Anyhow, something to keep
in mind for later if you like.

[johngro, 2015/11/06 20:22:07]

yeah, an adapter might be a good idea.  If we end up really running with the
idea of having identifiers for the timelines which define the domains on either
side of the transformation, we could have the specialization DCHECK that the
domain matches the chrono clock source.

+    local_to_output_ = LinearTransform(now_ticks, frames_per_tick_, -delay);
+    local_to_output_known_ = true;
+    frames_sent_ = 0;
+    while (++local_to_output_gen_ == MixJob::INVALID_GENERATION) {}

[jeffbrown, 2015/11/04 20:33:42]

A little weird but ok.

[johngro, 2015/11/06 20:22:07]

Acknowledged.

I could wrap this in a method if you think it would help clarity.

+  } else {
+    avail = snd_pcm_avail_update(alsa_device_);
+    if (avail < 0) {
+      HandleAlsaError(avail);
+      return false;
+    }
+  }
+
+  // Compute the time that we think we will completely underflow, then back off
+  // from that by the low buffer threshold and use that to determine when we
+  // should mix again.
+  int64_t playout_time_ticks;
+  bool trans_ok = local_to_output_.DoReverseTransform(frames_sent_,

[jeffbrown, 2015/11/04 20:33:42]

Am I correct in thinking that the reverse transform isn't always well-defined? 
(scale factor might be zero)

Do we need to check for singularity?

[johngro, 2015/11/06 20:22:07]

No, outputs cannot be paused.  They are either running or not, and when they are
not, there is no transformation (it needs to be primed).

Related:  This is why we always boost into our track's domain and perform
sampling there.  Even when pause, transformations into the tracks domain are
non-singular.  In the paused case, they are also non-1-to-1; all points in the
real world map to the paused point in the track.  Still, the transformation is
meaningful for doing things like trimming pending queues.

re: checking for singularity.
This is handled by the transform class.  It will never divide by zero.  If
boosting from one domain to the other fails (either because of overflow or
because of singularity) it will return false.  This should never happen for us,
which is why we DCHECK that result.

+                                                      &playout_time_ticks);
+  DCHECK(trans_ok);

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

Ahh, here's our check for singularity.

Odd thought, but what if DoReverseTransform checked this already for you? 
Essentially say: "don't ever call this function is the transform is singular". 
Seems like a reasonable precondition.

[johngro, 2015/11/06 20:22:07]

This also checks for overflow, not just singularity.

+  LocalTime playout_time = LocalTime(LocalDuration(playout_time_ticks));
+  LocalTime low_buf_time = playout_time - LOW_BUF_THRESH;
+
+  if (process_start >= low_buf_time) {
+    // Because of the way that ALSA consumes data and updates its internal
+    // bookkeeping, it is possible that we are past our low buffer threshold,
+    // but ALSA still thinks that there is no room to write new frames.  If this
+    // is the case, just try again a short amt of time in the future.

[jeffbrown, 2015/11/04 20:33:42]

amt -> amount

[johngro, 2015/11/06 20:22:07]

Done.

+    DCHECK_GE(avail, 0);
+    if (!avail) {
+      SetNextSchedDelay(WAIT_FOR_ALSA_DELAY);
+      return false;
+    }
+
+    // Limit the amt that we queue to be no more than what ALSA will currently
+    // accept, or what it currently will take to fill us to our target latency.
+    //
+    // The playout target had better be ahead of the playout time, or we are
+    // almost certainly going to underflow.  If this happens, for whatever
+    // reason, just try to send a full buffer and deal with the underflow when
+    // ALSA notices it.
+    int64_t fill_amt;
+    LocalTime playout_target = LocalClock::now() + TGT_LATENCY;
+    if (playout_target > playout_time) {
+      fill_amt = (playout_target - playout_time).count();
+    } else {
+      fill_amt = TGT_LATENCY.count();
+    }
+
+    DCHECK_GE(fill_amt, 0);
+    DCHECK_LE(fill_amt, std::numeric_limits<int32_t>::max());

[jeffbrown, 2015/11/04 20:33:42]

Are we guaranteed that numerator <= denominator?  Otherwise we might still
overflow.

[johngro, 2015/11/06 20:22:07]

We should be.  If N > D, it implies that our audio frame rate is greater than
our local clock frequency.  If this is the case, our local clock probably does
not have sufficient resolution to be used for media (that, or we are running our
audio output at something like 1GHz :O)

The real solution here is to expose a scale method from LinearTransform::Ratio
which (just like LT itself) returns an error code in case the transformation
either overflows or is singular.  I mention that I really need to do this in the
few other places in the server where I do a by-hand scale operation.  It's on my
list of things to do.

Still, I would just do it and DCHECK.  We should never overflow here.

+    fill_amt *= frames_per_tick_.numerator;

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

I might recommend that we create a for rounded scaling.

[johngro, 2015/11/06 20:22:07]

good idea.  See above; its the type of thing which belongs as part of the
utility methods for LT::Ratio

+    fill_amt += frames_per_tick_.denominator - 1;
+    fill_amt /= frames_per_tick_.denominator;
+
+    job->buf_frames = (avail < fill_amt) ? avail : fill_amt;
+    if (job->buf_frames > mix_buf_frames_) {
+      job->buf_frames = mix_buf_frames_;
+    }
+
+    job->buf = mix_buf_.get();
+    job->start_pts_of = frames_sent_;
+    job->local_to_output = &local_to_output_;
+    job->local_to_output_gen = local_to_output_gen_;
+
+    // TODO(johngro): optimize this if we can.  The first buffer we mix can just
+    // put its samples directly into the output buffer, and does not need to
+    // accumulate and clip.  In theory, we only need to put silence in the
+    // places where our outputs are not going to already overwrite.
+    ::memset(job->buf,
+             silence_byte_,

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

Technically silence might not always be a byte.  An unsigned 16-bit format, for
instance.  Or floats.  If you believe in them.  :)

[johngro, 2015/11/06 20:22:07]

Acknowledged.

This only works because we (currently) hardcode our format to signed 16 bit.  As
things develop, this will need to be updated.

+             job->buf_frames * output_bytes_per_frame_);
+    return true;
+  }
+
+  // Wait until its time to mix some more data.
+  SetNextSchedTime(low_buf_time);
+  return false;
+}
+
+bool AlsaOutput::FinishMixJob(const MixJob& job) {
+  DCHECK(job.buf == mix_buf_.get());
+  DCHECK(job.buf_frames);
+
+  // We should always be able to write all of the data that we mixed.
+  snd_pcm_sframes_t res;
+  res = snd_pcm_writei(alsa_device_, job.buf, job.buf_frames);
+  if (res != job.buf_frames) {
+    HandleAlsaError(res);
+    return false;
+  }
+
+  frames_sent_ += res;
+  return true;
+}
+
+void AlsaOutput::HandleAsUnderflow() {
+  snd_pcm_sframes_t res;
+
+  // If we were already primed, then this is a legitimate underflow, not the
+  // startup case or recovery from some other error.
+  if (primed_) {
+    // TODO(johngro): come up with a way to properly throttle this.  Also, add a
+    // friendly name to the output so the log helps to identify which output
+    // underflowed.
+    LOG(WARNING) << "[" << this << "] : underflow";
+    res = snd_pcm_recover(alsa_device_, -EPIPE, true);
+    if (res < 0) {
+      HandleAsError(res);
+      return;
+    }
+  }
+
+  // TODO(johngro): We don't actually have to fill up the entire lead time with
+  // silence.  When we have better control of our thread priorities, prime this
+  // with the minimimum amt we can get away with and still be able to start
+  // mixing without underflowing.
+  DCHECK(mix_buf_);
+  ::memset(mix_buf_.get(), silence_byte_, mix_buf_bytes_);

[jeffbrown, 2015/11/04 20:33:42]

This has shown up twice now.  Consider making a function to fill a buffer with N
frames of silence.  (Dealing with bytes and frames in the same function is a
little confusing anyhow.)

[johngro, 2015/11/06 20:22:07]

Sure; this (moving to frames only) means performing a multiply which we don't
strictly need to do.  It makes me a bit sad inside, but only because of the
unhealthy aspects of my OCD.  In reality, its not a big deal.

+  res = snd_pcm_writei(alsa_device_, mix_buf_.get(), mix_buf_frames_);
+
+  if (res < 0) {
+    HandleAsError(res);
+    return;
+  }
+
+  primed_ = true;
+  local_to_output_known_ = false;
+  SetNextSchedDelay(local_time::from_msec(1));
+}
+
+void AlsaOutput::HandleAsError(snd_pcm_sframes_t code) {

[jeffbrown, 2015/11/04 20:33:42]

The name of this function is a little too similar to HandleAlsaError for casual
inspection.

[johngro, 2015/11/06 20:22:07]

Permission to say HandleALSAError instead of HandleAlsaError?  I think this
would make things much more clear.  Failing that, I am open to suggestions.

+  // TODO(johngro): Throttle this somehow.
+  LOG(WARNING) << "[" << this << "] : Attempting to recover from ALSA error "
+               << code;
+
+  if (RECOVERABLE_ALSA_ERRORS.find(code) != RECOVERABLE_ALSA_ERRORS.end()) {

[jeffbrown, 2015/11/04 20:33:42]

If we got EINTR then the correct response was to retry immediately.  Probably
want to use TEMP_FAILURE_RETRY for that one instead of delaying.

[johngro, 2015/11/06 20:22:07]

I'm not sure I understand your request here.  The logic here is supposed to
be...

+ We have received an error from ALSA which is not underflow.
+ If this error is not on the short list which ALSA says it can attempt to
recover from, then LOG and shutdown.
+ If we can recover from it, then attempt to do so immediately.
+ If recovery succeeds, then wait our ERROR_RECOVERY_TIME (so speakers can
settle and we don't end up in a rapid, and unpleasant sounding,
start/fail/recover loop), then start the system back up again.
+ If recovery fails, treat this as an unrecoverable error (Apparently it is) and
shut ourselves down.

+    code = snd_pcm_recover(alsa_device_, code, true);
+    if (!code) {
+      primed_ = false;
+      local_to_output_known_ = false;
+      SetNextSchedDelay(ERROR_RECOVERY_TIME);
+    }
+  }
+
+  LOG(ERROR) << "[" << this << "] : Fatal ALSA error "
+             << code << ".  Shutting down";
+  ShutdownSelf();
+}
+
+void AlsaOutput::HandleAlsaError(snd_pcm_sframes_t code) {
+  // ALSA signals an underflow by returning -EPIPE from jobs.  If the error code
+  // is -EPIPE, treat this as an underflow and attempt to reprime the pipeline.
+  if (code == -EPIPE) {
+    HandleAsUnderflow();
+  } else {
+    HandleAsError(code);
+  }
+}
+
+}  // namespace audio
+}  // namespace media
+}  // namespace mojo
+