DaisyPatchSM audio outputs are inverted

[adamjmurray]

The values I write to a DaisyPatchSM's audio outputs result in a negated voltage coming out of the hardware (I am testing with a patch.Init() module). This firmware demonstrates the behavior:

#include "daisy_patch_sm.h"

using namespace daisy;
using namespace patch_sm;

DaisyPatchSM patch;

void AudioCallback(AudioHandle::InputBuffer in,
                   AudioHandle::OutputBuffer out,
                   size_t size)
{
  for (size_t i = 0; i < size; i++)
  {
    OUT_L[i] = 1;  // outputs approximately -9V on patch.Init() (Daisy Patch Submodule) hardware
    OUT_R[i] = -1; // outputs approximately +9V
  }
}

int main(void)
{
  patch.Init();
  patch.SetLed(true);
  patch.StartAudio(AudioCallback);
  while (1)
  {
  }
}

A brief discussion on the Electrosmith forum leads me to believe this behavior is by design on the hardware side. In that case, I view the current behavior on the software side as a bug.

Because the Patch SM has DC-coupled audio outputs, it can be used as a dual bipolar modulation signal generator/modifier with a nice wide peak-to-peak voltage range. In practice, one must be aware of the above behavior and, for example, only output negative values when outputting an envelope. This is very counterintuitive.

I feel that libDaisy should abstract aware the hardware details that these audio outputs are inverted. If I send a signed value to the output from code, I expected a voltage of the same sign produced by the hardware. This would be consistent with the other outputs, where a positive value to the CV or gate outputs results in positive voltage.

So my request is to make libDaisy automatically negate the audio outputs when working with DaisyPatchSM objects (and anywhere else applicable in the Daisy ecosystem).

[stephenhensley]

Hi @adamjmurray!

Thanks for the thorough issue.

It is the case that both the input and output are inverting circuits.
In a use-case where you're running audio through, it would come out in phase since it's two inverting stages.

That said, your point about the Patch SM being DC-coupled definitely is a good case for doing some software inversion in the audio backend so that the signals handled within the audio callback match.

A basic implementation of this would be changing this line and replacing the 1.f with a -1.f.

This will consequentially invert the input, but I think there may be a combination of settings using the existing postgain and output_compensation settings to get expected polarity in both places.

[adamjmurray]

Hi @stephenhensley . Glad to hear you seem receptive to changing this.

My main concern is if anyone else has been using a Patch SM's audio outputs for modulation (like an envelope generator), introducing a software inversion of the output value is a major breaking change.

I'm not sure what Electro-Smith's strategy for backward compatibility. A safe way to roll this out would be to enhance the DaisyPatchSM constructor to let the developer explicitly choose whether to enable the software to do the inversion or not.

Personally I think the default should be to do the software inversion so things work as intuitively as possible, but perhaps when the change first releases, the default behavior shouldn't change and you can encourage people to use the constructor where they explicitly choose the behavior they want. Then revisit changing the default one day in the future.

Or maybe that's too much time and effort (especially since it may make it harder to fix with e.g. Oopsy). You could just clearly state the behavior is changing when an update is released. Maybe anyone else using audio outs for modulation also agree with me that it's currently backwards and will welcome the change. 🤷‍♂️

So that's why I defer to Electro-Smith on a roll-out approach. Otherwise I'd probably just send a PR because this seems easy enough to just change, but I'm thinking about the existing firmware code out there and don't want to cause any unpleasant surprises.

[stephenhensley]

Hi @adamjmurray

For sure! It is definitely a little bit counter-intuitive with the DC usage.
For the primary Daisy Seed, the inversion wasn't consequential since the audio pass-through preserves the phase, and DC usage is not possibly without modifications to the hardware.

We do tend to avoid breaking changes as much as possible. So I like the idea of it being optional, and unfortunately non-default.

This actually might tie in with one of the things I recently staged on the s2dfm-eval-bringup branch that allows reinitialization of the AudioHandle without having to manually reinitialize the entire hardware (SAI) interface/codec.
So we could set up something where it's as easy as:

DaisyPatchSm hw;

. . .

hw.Init();

auto audio_cfg = hw.audio.GetConfig();
audio_cfg.postgain = -1.f;
hw.audio.Init(audio_cfg, hw.AudioSaiHandle());

Which could be simplified to a single helper-function in the DaisyPatchSM and/or added as a new optional config struct for the PatchSM.

[adamjmurray]

That sounds good to me. I'll keep watching this issue for further developments.