pschatzmann / arduino-audiokit

Arduino ADF/Audiokit HAL (support for ESP32-A1S, AI-Thinker, LyraT for ES7148, ES7210, ES7243, ES8311, ES8347, ES8388, TAS5805M, AC101 audio chips)
GNU General Public License v3.0
153 stars 40 forks source link

24 bit bit depth only produces 0s #65

Closed learex closed 1 year ago

learex commented 1 year ago

Using the MWE below, I can utilise 16 and 32 bit bits per sample but 24 ominously does not work and only outputs a stream of 0s. Replacing int24_t with int32_t and AUDIO_HAL_BIT_LENGTH_24BITS with AUDIO_HAL_BIT_LENGTH_32BITS seems to work flawlessly though, as does the original 16bit version of the code (int16_t and len/4) (input example).

I'm using an AIThinker V2.2 AC101 2748.

#include "AudioKitHAL.h"
#include "AudioTools.h"
#include "AudioBasic/Int24.h"

AudioKit kit;
const int BUFFER_SIZE = 1024;
uint8_t buffer[BUFFER_SIZE];

void printBuffer(int len){
  int24_t *value_ptr = (int24_t*) buffer;
  for (int j=0;j<len/8;j++){
    Serial.print(*value_ptr++);
    Serial.print(", ");
    Serial.println(*value_ptr++);
  }
}

void setup() {
  Serial.begin(115200);
  auto cfg = kit.defaultConfig(AudioInput);
  cfg.adc_input = AUDIO_HAL_ADC_INPUT_LINE1;
  cfg.sample_rate = AUDIO_HAL_16K_SAMPLES;
  cfg.bits_per_sample = AUDIO_HAL_BIT_LENGTH_24BITS;
  kit.begin(cfg);
}

void loop() {
  size_t len = kit.read(buffer, BUFFER_SIZE);
  printBuffer(len);
}
pschatzmann commented 1 year ago

Hmm, I never tested the 24bit functionality. I committed a correction and I am getting values now, but I am not sure if they make sense.

My latest understanding was that 24bit values are just 24bit represented as int32_t values, but I am not so sure about this any more...

learex commented 1 year ago

Unfortunately, I don't see any (new) commit I could try out.

From reading the comments in the code, yes 24 bit is int32_t with only 3 bytes used, if that's what you've meant.

pschatzmann commented 1 year ago

AudioBasic/Int24.h is part of the Audiotools, so the commits can be found there. So this issue is actually in the wrong project.

I needed to shift the int24 value by 8 bits to the left to make it work. The following output test with int24_t is working: https://github.com/pschatzmann/arduino-audio-tools/blob/main/examples/tests/24bits/24bits-write/24bits-write.ino

See https://www.pschatzmann.ch/home/2023/02/01/audiotools-and-24-bit-values/

pschatzmann commented 1 year ago

Since I did not get any feedback, I assume that the issue has been resolved.

learex commented 1 year ago

Well, reading from the Mics still doesn't work. I don't think that it's the 24bit implementation but rather the ADC control or something. That's also why I opened the issue in the audiokit lib.

Also: Editing comments days later is rather unhelpful as I don't get any kind of notification that this has been done.

pschatzmann commented 1 year ago

So you are still getting 0 values with 24 bits? What device did you connect to LineIn ?

learex commented 1 year ago

Yeah, when using my MWE with the updated arduino-audio-tools still doesn't work. Your example with self-generated data works fine.

learex commented 1 year ago

What device did you connect to LineIn ?

Nothing. I'm using the AC101 Dev board, thus utilising the onboard mics.

pschatzmann commented 1 year ago

If I remember right the mics are AUDIO_HAL_ADC_INPUT_LINE2

learex commented 1 year ago

No that's the other way around. [...]_LINE1 is the mics. As I said before. Changing the MWE to 32 bit works flawlessly.

pschatzmann commented 1 year ago

Since I did not want to debug your example I just modified my simple demo input sketch to 24 bits and it works like a charm!


AudioKitStream kit; // Access I2S as stream
CsvStream<int24_t> csvStream(Serial,2);
StreamCopy copier(csvStream, kit); // copy kit to csvStream

// Arduino Setup
void setup(void) {
    Serial.begin(115200);
    AudioLogger::instance().begin(Serial, AudioLogger::Warning);

    auto cfg = kit.defaultConfig(RX_MODE);
    cfg.bits_per_sample = 24;
    cfg.input_device = AUDIO_HAL_ADC_INPUT_LINE2;
    kit.begin(cfg);

    // make sure that we have the correct channels set up
    csvStream.begin();

}

// Arduino loop - copy data
void loop() {
    copier.copy();
}

If I whistle I am getting

![Uploading image.png…]() image

learex commented 1 year ago

You are right, this one works.

Your libraries still are a bit of a mystery for me since (at least I find) the documentation quite lacking. E.g. What is the difference between the audio-tools and audio-kit library? - you seem to be doing with audio-tools what I've used audiokit for as it's the lib made for my dev-board. What does AudioLibs/AudioKit.h do differently? - they have the exact same name... Why are you using 232 bits per sample? - like huh?!? You're using defaultConfig(RX_MODE) - what is RX? - what are the other options? It is really confusing... On the FLAC-Encoder issue you commented I should use Audiokit's audio actions - these aren't documented anywhere and autocomplete does not find them. I really appreciate your voluntary help, and I also know that documentation is tedious and annoying, but I hope you can, at least partly, understand where my confusion comes from. When a library is this large, you'd want the user not to read through the code itself, or read through all examples and tests, and, to me at least, there seems hardly a way around it.

pschatzmann commented 1 year ago

I do not agree: There is extensive documentation in the Wiki and a complete class documentation for each project! Maybe you have missed this ?

The AudioKit library is just (one of the different) low level Hardware Abstraction Layers (HAL) which are used by the AudioTools. In this case to provide the AudioKistStream class. The Idea of the AudioTools is that you can just replace the Stream implementation to send it to a different output or get the information form a different input device.

RX_MODE is used for reading data (e.g. from a microphone) TX_MODE is used just to output data and RXTX_MODE is used for both. This is not only working for AudioKistStream but for other Streams which support both: input and output like I2SStream, AnalogAudioStream, VS1053Stream etc

Of cause you can just stick with the AudioKit if you just need that functionality, but then you should not use e.g. int24_t. AudioActions are part of the AudioTools and not the AudioKit!

learex commented 1 year ago

Interestingly: 232 works, the corrected 24 does not. Very strange.

pschatzmann commented 1 year ago

I tested it with 24: maybe you still have an old version of the AudioTools? Double check the amplitude: it is only 24 bits if it is outside of 32767

learex commented 1 year ago

I do not agree: There is extensive documentation in the Wiki and a complete class documentation for each project! Maybe you have missed this ?

Yes, I know, and I'm using it, what I meant are the details and how it all ties together, that I either find somewhat confusing or not expressed in detail enough. As an engineer I'm always comparing other documentation to Mathematica's or Matlab's and those are incomparably more extensive. Still, this is a passion project, so once more: thanks for the (working) library and your help.

It might also be my specific Dev Board(s), which then is rather annoying.

me@mac ~ % cd ~/Documents/Arduino/libraries/arduino-audio-tools
me@mac arduino-audio-tools % git pull
Already up to date.
pschatzmann commented 1 year ago

Maybe you can contribute a nice architechture picture which provides the AudioTools on the top and summerizes the following chapters with dependencies

learex commented 1 year ago

Once I completed my project I definitely will. There's quite a bit of work still left, though. I also think, that my project might be a nice entry for new people to your library(ies).

Btw, the main reason I'm using arduino-audiokit is that it's the only implementation that was actually able to write to the SD. arduino-audio-tools was only able to create a new file (which I could see on my computer) but then failed to write the content. So I guess, when I'm uploading my project, it'll still use both since I need the flexibility of arduino-audio-tools...

pschatzmann commented 1 year ago

I tested the AudioTools with an AudioKit with the following libraries:

learex commented 1 year ago

I believe that you tested them; yet, my FLAC-Encoder fails when I let it write to a file, 24bit is off limits as it seems, SD-output was really tricky to get working and (something I've not reported) when I'm using 48k@32, 96k@16 or 192k@16 I'm getting weird digital artifacts. 22k@32 does not show problems. And even more strange: only in the left channel. (I extended the AudioKitHal.h and others for the higher sampling rates...) image image

pschatzmann commented 1 year ago

To be honest I never tried this because I always thought that this is a stretch: I usually stick with 16 bits and most often go for a sampling rate of max 44100 (or when needed below).

Maybe the delays introduced by the encoder or file write are just too big and lead to i2s under/overruns. Did you try to increase the I2S buffers or buffer size ?

learex commented 1 year ago

I'm using:

const int BUFFER_SIZE = 1024;
uint8_t buffer[BUFFER_SIZE];

2048 seems to be not supported (by your implementation? the hardware? idk).

The reason I want to go higher is less "audiophile" snake oil (🤮) but because of my technical application that might benefit from inaudible frequencies and increased dynamic range. In general: I can downsample/down-bit-depth after a measuring campaign, but I can never regain unrecorded data. As an additional benefit: I could digitally add some live metadata in inaudible frequency ranges.

pschatzmann commented 1 year ago

No, I was talking about the the ESP32 I2S buffer configuration ( buffer_count and buffer_size attributes)

learex commented 1 year ago

Similar to my FLAC MWE, I set the following. I've left buffer_count untouched (i.e. =6)

EDIT: I've just tried buffer_count=12, but to no avail.

AudioKit i2s;
[...]
auto cfg = i2s.defaultConfig(AudioInput);
[...]
cfg.buffer_size = BUFFER_SIZE;
i2s.begin(cfg);
[..]
image
pschatzmann commented 1 year ago

Did you try e.g. a WavEncoder instead of Flac to exclude that it is not coming from the overhead added by Flac ?

learex commented 1 year ago

Oh, this is RAW-PCM/WAVE - the FLAC implementation still doesn't work.

This (higher bit rate/bit depth) is also the biggest reason I try to get FLAC working: 48k@16 WAV files are easily manageable in an 24h measuring campaign (~8 GB mono), but 96k@32bit is already at 31GB for a mono track...

learex commented 1 year ago

I'm currently trying to work around EncodedAudioStream and trying to write the FLAC-packet into another buffer and that into the file, yet I don't quite understand how I should then feed the encoder data... Again similar to my FLAC_MWE, replacing FLAC with WAV also produces working code. I just don't understand why FLAC refuses to write to the file...

pschatzmann commented 1 year ago

I tried the following:

#include "AudioTools.h"
#include "AudioLibs/AudioKit.h"

AudioKitStream kit; // Access I2S as stream
StreamCopy copier(kit, kit); // copy kit to kit

// Arduino Setup
void setup(void) {
    Serial.begin(115200);
    AudioLogger::instance().begin(Serial, AudioLogger::Warning);

    auto cfg = kit.defaultConfig(RXTX_MODE);
    cfg.sd_active = false;
    cfg.bits_per_sample = 32;
    cfg.sample_rate = 48000;
    cfg.speaker_active = false;
    cfg.input_device = AUDIO_HAL_ADC_INPUT_LINE2;
    kit.begin(cfg);
}

// Arduino loop - copy data
void loop() {
    copier.copy();
}

and connected the AUX output to the audio input of my Macbook, which gives image

So on my board this is working perfectly with 32 bits and 48000. So either it is an issue with your board, or it is the lag that you introduce by writing data.

pschatzmann commented 1 year ago

In order to be able to process this you need to be able to write with a speed > 48000 x 2 channels x 4 bytes = 384000 bytes per second !

Did you measure how fast your writing speed is ?

learex commented 1 year ago

No not yet. I’ll perform more tests later this month, something else with higher priority came up.