Track more carefully which audio buffers to fill, based on interrupt channels

ports/raspberrypi/audio_dma.c

@@ -28,6 +28,7 @@
 
 #include "shared-bindings/audiocore/RawSample.h"
 #include "shared-bindings/audiocore/WaveFile.h"
+#include "shared-bindings/microcontroller/__init__.h"
 #include "supervisor/background_callback.h"
 
 #include "py/mpstate.h"
@@ -47,71 +48,78 @@ void audio_dma_reset(void) {
     }
 }
 
-void audio_dma_convert_signed(audio_dma_t *dma, uint8_t *buffer, uint32_t buffer_length,
-    uint8_t **output_buffer, uint32_t *output_buffer_length) {
 
-    size_t output_buffer_max_length;
+STATIC void audio_dma_convert_samples(
-    if (dma->first_buffer_free) {
+    audio_dma_t *dma,
-        *output_buffer = dma->first_buffer;
+    uint8_t *input, uint32_t input_length,
-        output_buffer_max_length = dma->first_buffer_length;
+    uint8_t *available_output_buffer, uint32_t available_output_buffer_length,
-    } else {
+    uint8_t **output, uint32_t *output_length) {
-        *output_buffer = dma->second_buffer;
-        output_buffer_max_length = dma->second_buffer_length;
-    }
 
     #pragma GCC diagnostic push
     #pragma GCC diagnostic ignored "-Wcast-align"
+
+    // Check whether a conversion is necessary
     if (dma->signed_to_unsigned ||
         dma->unsigned_to_signed ||
         dma->sample_spacing > 1 ||
         (dma->sample_resolution != dma->output_resolution)) {
-        *output_buffer_length = buffer_length / dma->sample_spacing;
+
+        // Must convert.
+        // Write the conversion into the passed-in output buffer
+        *output = available_output_buffer;
+        *output_length = input_length / dma->sample_spacing;
+
+        if (*output_length > available_output_buffer_length) {
+            mp_raise_RuntimeError(translate("Internal audio buffer too small"));
+        }
+
         uint32_t out_i = 0;
         if (dma->sample_resolution <= 8 && dma->output_resolution > 8) {
-            // reading bytes, writing 16-bit samples
+            // reading bytes, writing 16-bit words, so output buffer will be bigger.
-            *output_buffer_length = *output_buffer_length * 2;
+
-            if (*output_buffer_length > output_buffer_max_length) {
+            *output_length = *output_length * 2;
+            if (*output_length > available_output_buffer_length) {
                 mp_raise_RuntimeError(translate("Internal audio buffer too small"));
             }
 
             size_t shift = dma->output_resolution - dma->sample_resolution;
 
-            for (uint32_t i = 0; i < buffer_length; i += dma->sample_spacing) {
+            for (uint32_t i = 0; i < input_length; i += dma->sample_spacing) {
                 if (dma->signed_to_unsigned) {
-                    ((uint16_t *)*output_buffer)[out_i] = ((uint16_t)((int8_t *)buffer)[i] + 0x80) << shift;
+                    ((uint16_t *)*output)[out_i] = ((uint16_t)((int8_t *)input)[i] + 0x80) << shift;
                 } else if (dma->unsigned_to_signed) {
-                    ((int16_t *)*output_buffer)[out_i] = ((int16_t)((uint8_t *)buffer)[i] - 0x80) << shift;
+                    ((int16_t *)*output)[out_i] = ((int16_t)((uint8_t *)input)[i] - 0x80) << shift;
                 } else {
-                    ((uint16_t *)*output_buffer)[out_i] = ((uint16_t)((uint8_t *)buffer)[i]) << shift;
+                    ((uint16_t *)*output)[out_i] = ((uint16_t)((uint8_t *)input)[i]) << shift;
                 }
                 out_i += 1;
             }
         } else if (dma->sample_resolution <= 8 && dma->output_resolution <= 8) {
-            for (uint32_t i = 0; i < buffer_length; i += dma->sample_spacing) {
+            for (uint32_t i = 0; i < input_length; i += dma->sample_spacing) {
                 if (dma->signed_to_unsigned) {
-                    ((uint8_t *)*output_buffer)[out_i] = ((int8_t *)buffer)[i] + 0x80;
+                    ((uint8_t *)*output)[out_i] = ((int8_t *)input)[i] + 0x80;
                 } else if (dma->unsigned_to_signed) {
-                    ((int8_t *)*output_buffer)[out_i] = ((uint8_t *)buffer)[i] - 0x80;
+                    ((int8_t *)*output)[out_i] = ((uint8_t *)input)[i] - 0x80;
                 } else {
-                    ((uint8_t *)*output_buffer)[out_i] = ((uint8_t *)buffer)[i];
+                    ((uint8_t *)*output)[out_i] = ((uint8_t *)input)[i];
                 }
                 out_i += 1;
             }
         } else if (dma->sample_resolution > 8 && dma->output_resolution > 8) {
             size_t shift = 16 - dma->output_resolution;
-            for (uint32_t i = 0; i < buffer_length / 2; i += dma->sample_spacing) {
+            for (uint32_t i = 0; i < input_length / 2; i += dma->sample_spacing) {
                 if (dma->signed_to_unsigned) {
-                    ((uint16_t *)*output_buffer)[out_i] = ((int16_t *)buffer)[i] + 0x8000;
+                    ((uint16_t *)*output)[out_i] = ((int16_t *)input)[i] + 0x8000;
                 } else if (dma->unsigned_to_signed) {
-                    ((int16_t *)*output_buffer)[out_i] = ((uint16_t *)buffer)[i] - 0x8000;
+                    ((int16_t *)*output)[out_i] = ((uint16_t *)input)[i] - 0x8000;
                 } else {
-                    ((uint16_t *)*output_buffer)[out_i] = ((uint16_t *)buffer)[i];
+                    ((uint16_t *)*output)[out_i] = ((uint16_t *)input)[i];
                 }
                 if (dma->output_resolution < 16) {
                     if (dma->output_signed) {
-                        ((int16_t *)*output_buffer)[out_i] = ((int16_t *)*output_buffer)[out_i] >> shift;
+                        ((int16_t *)*output)[out_i] = ((int16_t *)*output)[out_i] >> shift;
                     } else {
-                        ((uint16_t *)*output_buffer)[out_i] = ((uint16_t *)*output_buffer)[out_i] >> shift;
+                        ((uint16_t *)*output)[out_i] = ((uint16_t *)*output)[out_i] >> shift;
                     }
                 }
                 out_i += 1;
@@ -122,65 +130,70 @@ void audio_dma_convert_signed(audio_dma_t *dma, uint8_t *buffer, uint32_t buffer
             mp_raise_RuntimeError(translate("Audio conversion not implemented"));
         }
     } else {
-        *output_buffer = buffer;
+        // No conversion necessary. Designate the input buffer as the output buffer.
-        *output_buffer_length = buffer_length;
+        *output = input;
+        *output_length = input_length;
     }
     #pragma GCC diagnostic pop
-    dma->first_buffer_free = !dma->first_buffer_free;
 }
 
-void audio_dma_load_next_block(audio_dma_t *dma) {
+// channel_idx is 0 or 1.
-    uint8_t dma_channel = dma->channel[1];
+STATIC void audio_dma_load_next_block(audio_dma_t *dma, size_t buffer_idx) {
-    if (dma->first_channel_free) {
+    size_t dma_channel = dma->channel[buffer_idx];
-        dma_channel = dma->channel[0];
-    }
-    dma->first_channel_free = !dma->first_channel_free;
 
-    uint8_t *output_buffer;
-    uint32_t output_buffer_length;
     audioio_get_buffer_result_t get_buffer_result;
-    uint8_t *buffer;
+    uint8_t *sample_buffer;
-    uint32_t buffer_length;
+    uint32_t sample_buffer_length;
     get_buffer_result = audiosample_get_buffer(dma->sample,
-        dma->single_channel_output, dma->audio_channel, &buffer, &buffer_length);
+        dma->single_channel_output, dma->audio_channel, &sample_buffer, &sample_buffer_length);
 
     if (get_buffer_result == GET_BUFFER_ERROR) {
         audio_dma_stop(dma);
         return;
     }
 
-    audio_dma_convert_signed(dma, buffer, buffer_length, &output_buffer, &output_buffer_length);
+    // Convert the sample format resolution and signedness, as necessary.
+    // The input sample buffer is what was read from a file or a raw sample buffer.
+    // The output buffer is one of the DMA buffers (passed in), or if no conversion was done,
+    // the original sample buffer (to save copying).
 
-    // If we don't have an output buffer, save the pointer to first_buffer for use in the single
+    // audio_dma_convert_samples() will write the converted samples into the given output
-    // buffer special case.
+    // buffer if necessary. If no conversion was needed, it will return the sample buffer
-    if (dma->first_buffer == NULL) {
+    // as the output buffer.
-        dma->first_buffer = output_buffer;
+    uint8_t *output_buffer;
-    }
+    uint32_t output_buffer_length;
+
+    audio_dma_convert_samples(dma, sample_buffer, sample_buffer_length,
+        dma->buffer[buffer_idx], dma->buffer_length[buffer_idx],
+        &output_buffer, &output_buffer_length);
 
-    dma_channel_set_trans_count(dma_channel, output_buffer_length / dma->output_size, false /* trigger */);
     dma_channel_set_read_addr(dma_channel, output_buffer, false /* trigger */);
+    dma_channel_set_trans_count(dma_channel, output_buffer_length / dma->output_size, false /* trigger */);
 
     if (get_buffer_result == GET_BUFFER_DONE) {
         if (dma->loop) {
             audiosample_reset_buffer(dma->sample, dma->single_channel_output, dma->audio_channel);
         } else {
+            // Set channel trigger to ourselves so we don't keep going.
+            dma_channel_hw_t *c = &dma_hw->ch[dma_channel];
+            c->al1_ctrl =
+                (c->al1_ctrl & ~DMA_CH0_CTRL_TRIG_CHAIN_TO_BITS) |
+                (dma_channel << DMA_CH0_CTRL_TRIG_CHAIN_TO_LSB);
+
             if (output_buffer_length == 0 &&
                 !dma_channel_is_busy(dma->channel[0]) &&
                 !dma_channel_is_busy(dma->channel[1])) {
                 // No data has been read, and both DMA channels have now finished, so it's safe to stop.
                 audio_dma_stop(dma);
                 dma->playing_in_progress = false;
-            } else {
-                // Set channel trigger to ourselves so we don't keep going.
-                dma_channel_hw_t *c = &dma_hw->ch[dma_channel];
-                c->al1_ctrl = (c->al1_ctrl & ~DMA_CH0_CTRL_TRIG_CHAIN_TO_BITS) | (dma_channel << DMA_CH0_CTRL_TRIG_CHAIN_TO_LSB);
             }
         }
     }
 }
 
 // Playback should be shutdown before calling this.
-audio_dma_result audio_dma_setup_playback(audio_dma_t *dma,
+audio_dma_result audio_dma_setup_playback(
+    audio_dma_t *dma,
     mp_obj_t sample,
     bool loop,
     bool single_channel_output,
@@ -189,6 +202,7 @@ audio_dma_result audio_dma_setup_playback(audio_dma_t *dma,
     uint8_t output_resolution,
     uint32_t output_register_address,
     uint8_t dma_trigger_source) {
+
     // Use two DMA channels to play because the DMA can't wrap to itself without the
     // buffer being power of two aligned.
     int dma_channel_0_maybe = dma_claim_unused_channel(false);
@@ -213,14 +227,15 @@ audio_dma_result audio_dma_setup_playback(audio_dma_t *dma,
     dma->unsigned_to_signed = false;
     dma->output_signed = output_signed;
     dma->sample_spacing = 1;
-    dma->first_channel_free = true;
     dma->output_resolution = output_resolution;
     dma->sample_resolution = audiosample_bits_per_sample(sample);
     dma->output_register_address = output_register_address;
 
     audiosample_reset_buffer(sample, single_channel_output, audio_channel);
 
-    bool single_buffer;
+
+    bool single_buffer;  // True if data fits in one single buffer.
+
     bool samples_signed;
     uint32_t max_buffer_length;
     audiosample_get_buffer_structure(sample, single_channel_output, &single_buffer, &samples_signed,
@@ -236,17 +251,16 @@ audio_dma_result audio_dma_setup_playback(audio_dma_t *dma,
         max_buffer_length /= dma->sample_spacing;
     }
 
-    dma->first_buffer = (uint8_t *)m_realloc(dma->first_buffer, max_buffer_length);
+    dma->buffer[0] = (uint8_t *)m_realloc(dma->buffer[0], max_buffer_length);
-    dma->first_buffer_length = max_buffer_length;
+    dma->buffer_length[0] = max_buffer_length;
-    if (dma->first_buffer == NULL) {
+    if (dma->buffer[0] == NULL) {
         return AUDIO_DMA_MEMORY_ERROR;
     }
 
-    dma->first_buffer_free = true;
     if (!single_buffer) {
-        dma->second_buffer = (uint8_t *)m_realloc(dma->second_buffer, max_buffer_length);
+        dma->buffer[1] = (uint8_t *)m_realloc(dma->buffer[1], max_buffer_length);
-        dma->second_buffer_length = max_buffer_length;
+        dma->buffer_length[1] = max_buffer_length;
-        if (dma->second_buffer == NULL) {
+        if (dma->buffer[1] == NULL) {
             return AUDIO_DMA_MEMORY_ERROR;
         }
     }
@@ -276,9 +290,11 @@ audio_dma_result audio_dma_setup_playback(audio_dma_t *dma,
         channel_config_set_dreq(&c, dma_trigger_source);
         channel_config_set_read_increment(&c, true);
         channel_config_set_write_increment(&c, false);
+
         // Chain to the other channel by default.
         channel_config_set_chain_to(&c, dma->channel[(i + 1) % 2]);
         dma_channel_set_config(dma->channel[i], &c, false /* trigger */);
+
         dma_channel_set_write_addr(dma->channel[i], (void *)output_register_address, false /* trigger */);
     }
 
@@ -288,9 +304,9 @@ audio_dma_result audio_dma_setup_playback(audio_dma_t *dma,
     MP_STATE_PORT(playing_audio)[dma->channel[1]] = dma;
 
     // Load the first two blocks up front.
-    audio_dma_load_next_block(dma);
+    audio_dma_load_next_block(dma, 0);
     if (!single_buffer) {
-        audio_dma_load_next_block(dma);
+        audio_dma_load_next_block(dma, 1);
     }
 
     // Special case the DMA for a single buffer. It's commonly used for a single wave length of sound
@@ -307,11 +323,11 @@ audio_dma_result audio_dma_setup_playback(audio_dma_t *dma,
         channel_config_set_chain_to(&c, dma->channel[1]); // Chain to ourselves so we stop.
         dma_channel_configure(dma->channel[1], &c,
             &dma_hw->ch[dma->channel[0]].al3_read_addr_trig, // write address
-            &dma->first_buffer, // read address
+            &dma->buffer[0], // read address
             1, // transaction count
             false); // trigger
     } else {
-        // Enable our DMA channels on DMA0 to the CPU. This will wake us up when
+        // Enable our DMA channels on DMA_IRQ_0 to the CPU. This will wake us up when
         // we're WFI.
         dma_hw->inte0 |= (1 << dma->channel[0]) | (1 << dma->channel[1]);
         irq_set_mask_enabled(1 << DMA_IRQ_0, true);
@@ -402,18 +418,19 @@ bool audio_dma_get_paused(audio_dma_t *dma) {
 }
 
 void audio_dma_init(audio_dma_t *dma) {
-    dma->first_buffer = NULL;
+    dma->buffer[0] = NULL;
-    dma->second_buffer = NULL;
+    dma->buffer[1] = NULL;
+
     dma->channel[0] = NUM_DMA_CHANNELS;
     dma->channel[1] = NUM_DMA_CHANNELS;
 }
 
 void audio_dma_deinit(audio_dma_t *dma) {
-    m_free(dma->first_buffer);
+    m_free(dma->buffer[0]);
-    dma->first_buffer = NULL;
+    dma->buffer[0] = NULL;
 
-    m_free(dma->second_buffer);
+    m_free(dma->buffer[1]);
-    dma->second_buffer = NULL;
+    dma->buffer[1] = NULL;
 }
 
 bool audio_dma_get_playing(audio_dma_t *dma) {
@@ -433,7 +450,25 @@ STATIC void dma_callback_fun(void *arg) {
         return;
     }
 
-    audio_dma_load_next_block(dma);
+    common_hal_mcu_disable_interrupts();
+    uint32_t channels_to_load_mask = dma->channels_to_load_mask;
+    dma->channels_to_load_mask = 0;
+    common_hal_mcu_enable_interrupts();
+
+    // Load the blocks for the requested channels.
+    uint32_t channel = 0;
+    while (channels_to_load_mask) {
+        if (channels_to_load_mask & 1) {
+            if (dma->channel[0] == channel) {
+                audio_dma_load_next_block(dma, 0);
+            }
+            if (dma->channel[1] == channel) {
+                audio_dma_load_next_block(dma, 1);
+            }
+        }
+        channels_to_load_mask >>= 1;
+        channel++;
+    }
 }
 
 void isr_dma_0(void) {
@@ -441,6 +476,8 @@ void isr_dma_0(void) {
         uint32_t mask = 1 << i;
         if ((dma_hw->intr & mask) != 0 && MP_STATE_PORT(playing_audio)[i] != NULL) {
             audio_dma_t *dma = MP_STATE_PORT(playing_audio)[i];
+            // Record all channels whose DMA has completed; they need loading.
+            dma->channels_to_load_mask |= mask;
             background_callback_add(&dma->callback, dma_callback_fun, (void *)dma);
             dma_hw->ints0 = mask;
         }

ports/raspberrypi/audio_dma.h

@@ -43,15 +43,12 @@ typedef struct {
     bool signed_to_unsigned;
     bool unsigned_to_signed;
     bool output_signed;
-    bool first_channel_free;
-    bool first_buffer_free;
     bool playing_in_progress;
     uint8_t output_resolution; // in bits
     uint8_t sample_resolution; // in bits
-    uint8_t *first_buffer;
+    uint8_t *buffer[2];
-    size_t first_buffer_length;
+    size_t buffer_length[2];
-    uint8_t *second_buffer;
+    uint32_t channels_to_load_mask;
-    size_t second_buffer_length;
     uint32_t output_register_address;
     background_callback_t callback;
 } audio_dma_t;

ports/raspberrypi/common-hal/audiopwmio/PWMAudioOut.c

@@ -157,27 +157,6 @@ void common_hal_audiopwmio_pwmaudioout_play(audiopwmio_pwmaudioout_obj_t *self,
         tx_register += self->left_pwm.channel * sizeof(uint16_t);
     }
 
-    audio_dma_result result = audio_dma_setup_playback(
-        &self->dma,
-        sample,
-        loop,
-        false, // single channel
-        0, // audio channel
-        false,  // output signed
-        BITS_PER_SAMPLE,
-        (uint32_t)tx_register,  // output register: PWM cc register
-        0x3b + pacing_timer); // data request line
-
-    if (result == AUDIO_DMA_DMA_BUSY) {
-        common_hal_audiopwmio_pwmaudioout_stop(self);
-        mp_raise_RuntimeError(translate("No DMA channel found"));
-    }
-    if (result == AUDIO_DMA_MEMORY_ERROR) {
-        common_hal_audiopwmio_pwmaudioout_stop(self);
-        mp_raise_RuntimeError(translate("Unable to allocate buffers for signed conversion"));
-    }
-
-    // OK! We got all of the resources we need and dma is ready.
     self->pacing_timer = pacing_timer;
 
     // Playback with two independent clocks. One is the sample rate which
@@ -214,6 +193,27 @@ void common_hal_audiopwmio_pwmaudioout_play(audiopwmio_pwmaudioout_obj_t *self,
     }
 
     dma_hw->timer[pacing_timer] = best_numerator << 16 | best_denominator;
+
+    audio_dma_result result = audio_dma_setup_playback(
+        &self->dma,
+        sample,
+        loop,
+        false, // single channel
+        0, // audio channel
+        false,  // output signed
+        BITS_PER_SAMPLE,
+        (uint32_t)tx_register,  // output register: PWM cc register
+        0x3b + pacing_timer); // data request line
+
+    if (result == AUDIO_DMA_DMA_BUSY) {
+        common_hal_audiopwmio_pwmaudioout_stop(self);
+        mp_raise_RuntimeError(translate("No DMA channel found"));
+    }
+    if (result == AUDIO_DMA_MEMORY_ERROR) {
+        common_hal_audiopwmio_pwmaudioout_stop(self);
+        mp_raise_RuntimeError(translate("Unable to allocate buffers for signed conversion"));
+    }
+    // OK! We got all of the resources we need and dma is ready.
 }
 
 void common_hal_audiopwmio_pwmaudioout_stop(audiopwmio_pwmaudioout_obj_t *self) {

shared-module/audiocore/WaveFile.c

@@ -206,7 +206,7 @@ audioio_get_buffer_result_t audioio_wavefile_get_buffer(audioio_wavefile_obj_t *
     }
 
     if (need_more_data) {
-        uint16_t num_bytes_to_load = self->len;
+        uint32_t num_bytes_to_load = self->len;
         if (num_bytes_to_load > self->bytes_remaining) {
             num_bytes_to_load = self->bytes_remaining;
         }