Motown in-proc streaming framework used to implement media services.

services/media/framework/parts/lpcm_reformatter.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 "base/logging.h"
+#include "services/media/framework/parts/lpcm_reformatter.h"
+
+namespace mojo {
+namespace media {
+
+LpcmReformatter* LpcmReformatter::NewImpl(
+    const LpcmStreamType& in_type,
+    const LpcmStreamTypeSet& out_type) {
+  switch (in_type.sample_format()) {

[johngro, 2016/01/26 23:47:29]

up to you, but you can reduce the amount of duplication of the inner switch
block here by templating this method.  The idea would be...

1) Move the class definition of LpcmReformatterImpl into this transitional unit.
 You can stuff it into an anon namespace if you want to really bury it.
2) Make the impl class's constructor public.
3) Make a static function templated on typename TIn.  Have it implement the
inner switch to select the out type, and use the passed template parameter to
select the in type.
4) Implement the outer switch in LpcmReformatter::New, use it to select the in
type and then call the templated method to select the out type.

[dalesat, 2016/01/28 18:49:16]

Acknowledged.

+    case LpcmStreamType::SampleFormat::kUnsigned8:
+      switch (out_type.sample_format()) {
+        case LpcmStreamType::SampleFormat::kUnsigned8:
+        case LpcmStreamType::SampleFormat::kAny:
+          return new LpcmReformatterImpl<uint8_t, uint8_t>(
+              in_type, out_type);
+        case LpcmStreamType::SampleFormat::kSigned16:
+          return new LpcmReformatterImpl<uint8_t, int16_t>(
+              in_type, out_type);
+        case LpcmStreamType::SampleFormat::kSigned24In32:
+          return new LpcmReformatterImpl<uint8_t, int32_t>(
+              in_type, out_type);
+        case LpcmStreamType::SampleFormat::kFloat:
+          return new LpcmReformatterImpl<uint8_t, float>(
+              in_type, out_type);
+        default:
+          NOTREACHED() << "unsupported sample format";
+          return nullptr;
+      }
+    case LpcmStreamType::SampleFormat::kSigned16:
+      switch (out_type.sample_format()) {
+        case LpcmStreamType::SampleFormat::kUnsigned8:
+          return new LpcmReformatterImpl<int16_t, uint8_t>(
+              in_type, out_type);
+        case LpcmStreamType::SampleFormat::kSigned16:
+        case LpcmStreamType::SampleFormat::kAny:
+          return new LpcmReformatterImpl<int16_t, int16_t>(
+              in_type, out_type);
+        case LpcmStreamType::SampleFormat::kSigned24In32:
+          return new LpcmReformatterImpl<int16_t, int32_t>(
+              in_type, out_type);
+        case LpcmStreamType::SampleFormat::kFloat:
+          return new LpcmReformatterImpl<int16_t, float>(
+              in_type, out_type);
+        default:
+          NOTREACHED() << "unsupported sample format";
+          return nullptr;
+      }
+    case LpcmStreamType::SampleFormat::kSigned24In32:
+      switch (out_type.sample_format()) {
+        case LpcmStreamType::SampleFormat::kUnsigned8:
+          return new LpcmReformatterImpl<int32_t, uint8_t>(
+              in_type, out_type);
+        case LpcmStreamType::SampleFormat::kSigned16:
+          return new LpcmReformatterImpl<int32_t, int16_t>(
+              in_type, out_type);
+        case LpcmStreamType::SampleFormat::kSigned24In32:
+        case LpcmStreamType::SampleFormat::kAny:
+          return new LpcmReformatterImpl<int32_t, int32_t>(
+              in_type, out_type);
+        case LpcmStreamType::SampleFormat::kFloat:
+          return new LpcmReformatterImpl<int32_t, float>(
+              in_type, out_type);
+        default:
+          NOTREACHED() << "unsupported sample format";
+          return nullptr;
+      }
+    case LpcmStreamType::SampleFormat::kFloat:
+      switch (out_type.sample_format()) {
+        case LpcmStreamType::SampleFormat::kUnsigned8:
+          return new LpcmReformatterImpl<float, uint8_t>(
+              in_type, out_type);
+        case LpcmStreamType::SampleFormat::kSigned16:
+          return new LpcmReformatterImpl<float, int16_t>(
+              in_type, out_type);
+        case LpcmStreamType::SampleFormat::kSigned24In32:
+          return new LpcmReformatterImpl<float, int32_t>(
+              in_type, out_type);
+        case LpcmStreamType::SampleFormat::kFloat:
+        case LpcmStreamType::SampleFormat::kAny:
+          return new LpcmReformatterImpl<float, float>(in_type, out_type);
+        default:
+          NOTREACHED() << "unsupported sample format";
+          return nullptr;
+      }
+    default:
+      NOTREACHED() << "unsupported sample format";
+      return nullptr;
+  }
+}
+
+template<typename TIn, typename TOut>
+LpcmReformatterImpl<TIn, TOut>::LpcmReformatterImpl(
+    const LpcmStreamType& in_type,
+    const LpcmStreamTypeSet& out_type) :
+    in_type_(in_type),
+    out_type_(
+        out_type.sample_format() == LpcmStreamType::SampleFormat::kAny ?
+            in_type.sample_format() :
+            out_type.sample_format(),
+        in_type.channels(),
+        in_type.frames_per_second()) {}
+
+template<typename TIn, typename TOut>
+LpcmReformatterImpl<TIn, TOut>::~LpcmReformatterImpl() {}
+
+namespace {
+
+// TODO(dalesat): Limit, optimize.
+
+template<typename TIn, typename TOut>
+inline void CopySample(TOut* dest, TIn* source) {
+  *dest = static_cast<TOut>(*source);
+}
+
+inline void CopySample(uint8_t* dest, int16_t* source) {
+  *dest = static_cast<uint8_t>((*source >> 8) ^ 0x80);
+}
+
+inline void CopySample(uint8_t* dest, int32_t* source) {
+  *dest = static_cast<uint8_t>((*source >> 16) ^ 0x80);
+}
+
+inline void CopySample(uint8_t* dest, float* source) {

[johngro, 2016/01/26 23:47:29]

Source needs to be clamped to the range [-1.0, 1.0] here.  This goes for all of
the instances of CopySample which take float as a source.  During the demo, this
was causing some clicks and pops as the decoder spit out samples slightly above
and below [-1.0, 1.0]

FWIW - the same can be said for the 24-in-32 source format as well since, like
the float, not all of the input value are valid for the encoding.  Long term, I
suppose we need to decide whose job this ultimately is, but in the short term,
we have badly behaved codecs and no one fixing up their crap.

In case its helpful, I just ended up defining an inline templated clamp function
and applying it to the formats with float sources.  I skipped the 24-in-32
formats as they were not relevant to the demo, but it should be simple enough to
use the same function.

template <typename T>
inline constexpr T Clamp(T val, T min, T max) {
  return (val > max) ? max : ((val < min) ? min : val);
}

inline void CopySample(uint8_t* dest, float* source) {
  *dest = static_cast<uint8_t>((Clamp(*source, -1.0f, 1.0f) * 0x7f) + 128);
}

inline void CopySample(int16_t* dest, float* source) {
  *dest = static_cast<int16_t>(Clamp(*source, -1.0f, 1.0f) * 0x7fff);
}

inline void CopySample(int32_t* dest, float* source) {
  *dest = static_cast<int32_t>(Clamp(*source, -1.0f, 1.0f) * 0x7fffff);
}

[dalesat, 2016/01/28 18:49:15]

Done.

+  *dest = static_cast<uint8_t>((*source * 0x7f) + 128);
+}
+
+inline void CopySample(int16_t* dest, uint8_t* source) {
+  *dest = static_cast<int16_t>(*source ^ 0x80) << 8;
+}
+
+inline void CopySample(int16_t* dest, int32_t* source) {
+  *dest = static_cast<int16_t>(*source >> 8);
+}
+
+inline void CopySample(int16_t* dest, float* source) {
+  *dest = static_cast<int16_t>(*source * 0x7fff);
+}
+
+inline void CopySample(int32_t* dest, uint8_t* source) {
+  *dest = static_cast<int32_t>(*source ^ 0x80) << 16;
+}
+
+inline void CopySample(int32_t* dest, int16_t* source) {
+  *dest = static_cast<int32_t>(*source << 8);
+}
+
+inline void CopySample(int32_t* dest, float* source) {
+  *dest = static_cast<int32_t>(*source * 0x7fffff);
+}
+
+inline void CopySample(float* dest, uint8_t* source) {
+  *dest = static_cast<float>(*source ^ 0x80) / 0x80;
+}
+
+inline void CopySample(float* dest, int16_t* source) {
+  *dest = static_cast<float>(*source) / 0x8000;
+}
+
+inline void CopySample(float* dest, int32_t* source) {
+  *dest = static_cast<float>(*source) / 0x800000;
+}
+
+template<typename TIn, typename TOut>
+inline void MixSample(TOut* dest, TIn* source) {
+  *dest += static_cast<TOut>(*source);
+}
+
+inline void MixSample(uint8_t* dest, int16_t* source) {
+  *dest += static_cast<uint8_t>((*source >> 8) ^ 0x80);
+}
+
+inline void MixSample(uint8_t* dest, int32_t* source) {
+  *dest += static_cast<uint8_t>((*source >> 16) ^ 0x80);
+}
+
+inline void MixSample(uint8_t* dest, float* source) {
+  *dest += static_cast<uint8_t>((*source * 0x7f) + 128);
+}
+
+inline void MixSample(int16_t* dest, uint8_t* source) {
+  *dest += static_cast<int16_t>(*source ^ 0x80) << 8;
+}
+
+inline void MixSample(int16_t* dest, int32_t* source) {
+  *dest += static_cast<int16_t>(*source >> 8);
+}
+
+inline void MixSample(int16_t* dest, float* source) {
+  *dest += static_cast<int16_t>(*source * 0x7fff);
+}
+
+inline void MixSample(int32_t* dest, uint8_t* source) {
+  *dest += static_cast<int32_t>(*source ^ 0x80) << 16;
+}
+
+inline void MixSample(int32_t* dest, int16_t* source) {
+  *dest += static_cast<int32_t>(*source << 8);
+}
+
+inline void MixSample(int32_t* dest, float* source) {
+  *dest += static_cast<int32_t>(*source * 0x7fffff);
+}
+
+inline void MixSample(float* dest, uint8_t* source) {
+  *dest += static_cast<float>(*source ^ 0x80) / 0x80;
+}
+
+inline void MixSample(float* dest, int16_t* source) {
+  *dest += static_cast<float>(*source) / 0x8000;
+}
+
+inline void MixSample(float* dest, int32_t* source) {
+  *dest += static_cast<float>(*source) / 0x800000;
+}
+
+} // namespace
+
+template<typename TIn, typename TOut>
+const LpcmStreamType& LpcmReformatterImpl<TIn, TOut>::input_stream_type()
+    const {
+  return in_type_;
+}
+
+template<typename TIn, typename TOut>
+const LpcmStreamType& LpcmReformatterImpl<TIn, TOut>::output_stream_type()
+    const {
+  return out_type_;
+}
+
+template<typename TIn, typename TOut>
+void LpcmReformatterImpl<TIn, TOut>::TransformFrames(
+    LpcmFrames* source,
+    LpcmFrames* dest,
+    bool mix) {
+  DCHECK(source);
+  DCHECK(dest);
+  DCHECK(source->buffer());
+  DCHECK(source->frame_count());
+  DCHECK(dest->buffer());
+  DCHECK(dest->frame_count());
+
+  uint64_t frame_count = std::min(source->frame_count(), dest->frame_count());
+
+  uint8_t* in_channel = reinterpret_cast<uint8_t*>(source->buffer());
+  uint8_t* out_channel = reinterpret_cast<uint8_t*>(dest->buffer());

[johngro, 2016/01/26 23:47:29]

FWIW - when casing a pointer to a PoD to another pointer to a PoD, you can just
static_cast, it should do exactly the same thing.

+
+  for (uint32_t channel = 0; channel < in_type_.channels(); channel++) {
+    TIn* in_sample = reinterpret_cast<TIn*>(in_channel);
+    TOut* out_sample = reinterpret_cast<TOut*>(out_channel);
+    if (mix) {

[johngro, 2016/01/26 23:47:29]

if you are going to put the mix decision in here, you should put it outside of
the loop, not inside.  The compiler may attempt to make this optimization for
you, but this may be getting a bit complicated for it.

Alternatively, if you make a private template of this method which takes a bool
DoMix parameter, you can call if from the public method where mix is decided at
runtime.  Then, whether you put the if (DoMix) inside or outside of the loop the
compiler will kill the unused path in each of the generated copies of the
template expansion.

[dalesat, 2016/01/28 18:49:16]

The outer loop is per-channel. The test will happen approximately twice per call
to TransformFrames, which I'm OK with. I think the best solution involves the
mix template parameter, but I don't want to invest too heavily in optimization
until we have the time to run performance tests.

[johngro, 2016/02/01 22:38:16]

Acknowledged.

+      for (uint64_t sample = 0; sample < frame_count; sample++) {
+        MixSample(out_sample, in_sample);
+        in_sample += in_type_.channels();
+        out_sample += out_type_.channels();
+      }
+    } else {
+      for (uint64_t sample = 0; sample < frame_count; sample++) {
+        CopySample(out_sample, in_sample);
+        in_sample += in_type_.channels();
+        out_sample += out_type_.channels();
+      }
+    }
+    in_channel += in_type_.sample_size();
+    out_channel += out_type_.sample_size();
+  }
+
+  source->advance(frame_count);
+  dest->advance(frame_count);
+}
+
+}  // namespace media
+}  // namespace mojo