From a4ba7350a7a9b294d4efc347e339fad25ce297e5 Mon Sep 17 00:00:00 2001
From: jacqueline <me@jacqueline.id.au>
Date: Fri, 11 Aug 2023 08:39:52 +1000
Subject: [PATCH] Fix docs nits, avoid recalculating frame length

---
 src/audio/audio_task.cpp         | 41 +++++++++++++-------------------
 src/audio/include/audio_task.hpp |  7 +++---
 src/codecs/include/sample.hpp    | 16 +++++++++++--
 3 files changed, 33 insertions(+), 31 deletions(-)

diff --git a/src/audio/audio_task.cpp b/src/audio/audio_task.cpp
index 05a06ad1..d1432007 100644
--- a/src/audio/audio_task.cpp
+++ b/src/audio/audio_task.cpp
@@ -7,6 +7,7 @@
 #include "audio_task.hpp"
 
 #include <stdlib.h>
+#include <sys/_stdint.h>
 
 #include <algorithm>
 #include <cmath>
@@ -17,34 +18,28 @@
 #include <memory>
 #include <variant>
 
-#include "audio_decoder.hpp"
-#include "audio_events.hpp"
-#include "audio_fsm.hpp"
-#include "audio_sink.hpp"
-#include "audio_source.hpp"
 #include "cbor.h"
-#include "codec.hpp"
 #include "esp_err.h"
 #include "esp_heap_caps.h"
 #include "esp_log.h"
-#include "event_queue.hpp"
-#include "fatfs_audio_input.hpp"
 #include "freertos/portmacro.h"
 #include "freertos/projdefs.h"
 #include "freertos/queue.h"
 #include "freertos/ringbuf.h"
-#include "pipeline.hpp"
-#include "sample.hpp"
-#include "sink_mixer.hpp"
 #include "span.hpp"
 
-#include "arena.hpp"
+#include "audio_decoder.hpp"
 #include "audio_element.hpp"
+#include "audio_events.hpp"
+#include "audio_fsm.hpp"
+#include "audio_sink.hpp"
+#include "audio_source.hpp"
 #include "chunk.hpp"
-#include "stream_event.hpp"
-#include "stream_info.hpp"
-#include "stream_message.hpp"
-#include "sys/_stdint.h"
+#include "codec.hpp"
+#include "event_queue.hpp"
+#include "fatfs_audio_input.hpp"
+#include "sample.hpp"
+#include "sink_mixer.hpp"
 #include "tasks.hpp"
 #include "track.hpp"
 #include "types.hpp"
@@ -57,19 +52,18 @@ static const char* kTag = "audio_dec";
 static constexpr std::size_t kCodecBufferLength = 240 * 4;
 
 Timer::Timer(const codecs::ICodec::OutputFormat& format)
-    : format_(format),
-      current_seconds_(0),
+    : current_seconds_(0),
       current_sample_in_second_(0),
+      samples_per_second_(format.sample_rate_hz * format.num_channels),
       total_duration_seconds_(format.total_samples.value_or(0) /
                               format.num_channels / format.sample_rate_hz) {}
 
 auto Timer::AddSamples(std::size_t samples) -> void {
   bool incremented = false;
   current_sample_in_second_ += samples;
-  while (current_sample_in_second_ >=
-         format_.sample_rate_hz * format_.num_channels) {
+  while (current_sample_in_second_ >= samples_per_second_) {
     current_seconds_++;
-    current_sample_in_second_ -= format_.sample_rate_hz * format_.num_channels;
+    current_sample_in_second_ -= samples_per_second_;
     incremented = true;
   }
 
@@ -87,10 +81,7 @@ auto Timer::AddSamples(std::size_t samples) -> void {
 
 auto AudioTask::Start(IAudioSource* source, IAudioSink* sink) -> AudioTask* {
   AudioTask* task = new AudioTask(source, sink);
-  // Pin to CORE1 because codecs should be fixed point anyway, and being on
-  // the opposite core to the mixer maximises throughput in the worst case
-  // (some heavy codec like opus + resampling for bluetooth).
-  tasks::StartPersistent<tasks::Type::kAudio>(1, [=]() { task->Main(); });
+  tasks::StartPersistent<tasks::Type::kAudio>([=]() { task->Main(); });
   return task;
 }
 
diff --git a/src/audio/include/audio_task.hpp b/src/audio/include/audio_task.hpp
index 72b89ba2..cd79b708 100644
--- a/src/audio/include/audio_task.hpp
+++ b/src/audio/include/audio_task.hpp
@@ -7,15 +7,15 @@
 #pragma once
 
 #include <sys/_stdint.h>
+
 #include <cstdint>
 #include <memory>
+
 #include "audio_decoder.hpp"
 #include "audio_sink.hpp"
 #include "audio_source.hpp"
 #include "codec.hpp"
-#include "pipeline.hpp"
 #include "sink_mixer.hpp"
-#include "stream_info.hpp"
 #include "track.hpp"
 #include "types.hpp"
 
@@ -28,10 +28,9 @@ class Timer {
   auto AddSamples(std::size_t) -> void;
 
  private:
-  codecs::ICodec::OutputFormat format_;
-
   uint32_t current_seconds_;
   uint32_t current_sample_in_second_;
+  uint32_t samples_per_second_;
 
   uint32_t total_duration_seconds_;
 };
diff --git a/src/codecs/include/sample.hpp b/src/codecs/include/sample.hpp
index ea3a7ffc..f77284bb 100644
--- a/src/codecs/include/sample.hpp
+++ b/src/codecs/include/sample.hpp
@@ -1,3 +1,9 @@
+/*
+ * Copyright 2023 jacqueline <me@jacqueline.id.au>
+ *
+ * SPDX-License-Identifier: GPL-3.0-only
+ */
+
 #pragma once
 
 #include <stdint.h>
@@ -8,7 +14,14 @@
 
 namespace sample {
 
-// A signed, 32-bit PCM sample.
+// A signed, 16-bit PCM sample. All decoder output should be normalised to this
+// format, in order to simplify resampling and/or re-encoding for bluetooth.
+// Why 'only' 16 bits?
+//  1. It's the lowest common bits per sample amongst our codecs. A higher bits
+//     per sample would require us to uselessly scale up those outputs.
+//  2. With appropriate dithering, you're not going to hear a difference
+//     between 16 bit samples and higher bits anyway.
+//  3. Monty from Xiph.org reckons it's all you need.
 typedef int16_t Sample;
 
 constexpr auto Clip(int64_t v) -> Sample {
@@ -21,7 +34,6 @@ constexpr auto Clip(int64_t v) -> Sample {
 
 constexpr auto FromSigned(int32_t src, uint_fast8_t bits) -> Sample {
   if (bits > 16) {
-    // Left-align samples, effectively scaling them up to 32 bits.
     return src << (sizeof(Sample) * 8 - bits);
   } else if (bits < 16) {
     return src << (bits - (sizeof(Sample) * 8));