TDA1543A dac not working properly with I2S library

[Sand192]

Hi!

I wanted to add bluetooth to my speaker system using 'A2DPsink' sketch. Unfortunately, it hasn't worked as expected and my attempts to fix it have only led to more issues. I don't know how to fix this issue due to my small PIO code knowledge. I purchased this dac because the local shop was out of TDA1543.

The problem is that the TDA1543A expects a 24-bit right-justified format with a fixed 48fs. The audio data format is 16-bit two's complement with MSB first. This format is referred to as 'Japanese format'. The MSB has to be repeated 8 times for this dac according to this post: https://forum.pjrc.com/index.php?threads/dual-channel-16bit-dac-pt8211.29284/post-141314

Here's how the dac behaved in my case without modifications:

When using normal I2S with 'A2DPsink', I experienced distortion and hiss when the volume was set higher than 2% on computer. Anything lower than that sounded quite clean.

#include <I2S.h>
#include <pio_i2s.pio.h>
#include <BluetoothAudio.h>

I2S i2s;
A2DPSink a2dp;

#define pBCLK 2
#define pWS (pBCLK+1)
#define pDOUT 4

volatile A2DPSink::PlaybackStatus status = A2DPSink::STOPPED;

void volumeCB(void *param, int pct) {
  (void) param;
  Serial.printf("Speaker volume changed to %d%%\n", pct);
}

void connectCB(void *param, bool connected) {
  (void) param;
  if (connected) {
    Serial.printf("A2DP connection started to %s\n", bd_addr_to_str(a2dp.getSourceAddress()));
  } else {
    Serial.printf("A2DP connection stopped\n");
  }
}

void playbackCB(void *param, A2DPSink::PlaybackStatus state) {
  (void) param;
  status = state;
}

void setup() {
  Serial.begin(115200);
  delay(3000);
  Serial.printf("Starting, connect to the PicoW and start playing music\n");
  Serial.printf("Use BOOTSEL to pause/resume playback\n");
  a2dp.setName("PicoW Boom 00:00:00:00:00:00");
  a2dp.setConsumer(new BluetoothAudioConsumerI2S(i2s));
  a2dp.onVolume(volumeCB);
  a2dp.onConnect(connectCB);
  a2dp.onPlaybackStatus(playbackCB);
  i2s.setBCLK(pBCLK);
  i2s.setDATA(pDOUT);
  i2s.setBitsPerSample(16);
  a2dp.begin();
}

char *nowPlaying = nullptr;

void loop() {
  if (BOOTSEL) {
    if (status == A2DPSink::PAUSED) {
      a2dp.play();
      Serial.printf("Resuming\n");
    } else if (status == A2DPSink::PLAYING) {
      a2dp.pause();
      Serial.printf("Pausing\n");
    }
    while (BOOTSEL);
  }
  if (!nowPlaying || strcmp(nowPlaying, a2dp.trackTitle())) {
    free(nowPlaying);
    nowPlaying = strdup(a2dp.trackTitle());
    Serial.printf("NOW PLAYING: %s\n", nowPlaying);
  }
}

In the 'SimpleTone' sketch the volume increases only in amplitude ranges 0-16000 and 32000-48000. Outside of these ranges, the amplitude started to drop.

Datasheet for tda1543A: https://semiconductors.es/datasheet/TDA1543A.html

I have attached picture from the datasheet part that shows the timing.

Tda1543A datasheet timing diagram:

Any help or suggestions would be greatly appreciated!

Best regards,

[earlephilhower]

This format is referred to as 'Japanese format'.

Err, why not try adding the i2s.setLSBJFormat() call before i2s.begin() to enable LSB-Japanese mode?

[Sand192]

Tried it but it gave same result. I can try recording the output using input on laptop and upload it to here.

[Sand192]

I recorded 440hz at 1%, 50% and 100% volume using microphone input, it sounded quite bad at 50% and 100% was even worse. 1% sounded the cleanest.

testresults.zip

[Sand192]

Here's the schematic I have used

Big thanks for the suggestion but it didnt want to work as supposed to. I even tried 'i2s.setTDMFormat();' out of curiosity which gave similar result as to I2S and LSBJ. I think the dac might be unhappy with the data aligment and is looking for 24 bit right justified with 16 bit audio data MSB first. That kind of japanese format is diffrent from LSBJ.

I sent the link of the PJRC forum post which seemed helpful to me. Pazi couldnt get proper 24 bit word, so he used 32 bit word for tda1543a by sending MSB 8 times, then sending audio bits and filling rest of the 32 bit word with zeroes. That makes me think that the dac can accept data as long as it is in 24-32 word range and the MSB is repeated 8 times in the beginning of the word. Problem is that I have no clue on how to do it since never worked with anything that complex.

Best regards,

[earlephilhower]

That's a pretty messed up interface, but I think you can make it in SW.

If you use LSBJ mode and set the bits per sample to 24 and manually in your code add the MSB replication/shift, the bits on the wire coming out would replicate the waveform it expects.

[Sand192]

Do I need to modify PIO code or I2S.cpp? I think I should try modifying the I2S::_writeNatural() by adding extra code to case24:. By the way, this format was quite common on japanese audio gear. Even Sony made chips that support it. Example would be Sony CXD2550P.

Thanks for the suggestion

[earlephilhower]

Neither, change your app's I2S feeding. As you send data into the i2s.write24(l,r) just adjust each sample. Off the top of my head

int32_t l32 = ((int32_t)l16) << 16;
l32 >>= 8; // IIRC, signed shift will replicate MSB for you
int32_t r32 = ((int32_t)r16) << 16;
r32 >>= 8;
i2s.write24(l32, r32);

Or simple if logic to set MSBs and left-align data

[Sand192]

Ok, I will give it a try and see if it works.

Thanks!

[Sand192]

I am wondering how could I add it to A2DPsink, I am using the example sketch that comes with pico core.

[earlephilhower]

Maybe you should start with something simple like the I2S tone example so there's less to debug. Once you have the sample conversion working, you can just make a new subclass of BluetoothAudioConsumer almost a carbon copy of https://github.com/earlephilhower/arduino-pico/blob/master/libraries/BluetoothAudio/src/BluetoothAudioConsumerI2S.cpp , but with expansion and tweaking of the samples in `...::fill()

[earlephilhower]

(by expansion I mean you'll end up allocating a buffer of (32-bits x samples) and for all samples expand from 16->32 and do your MSB mapping before the i2s.write(buffer,size) call.(

[Sand192]

Seems to work better now. Big thanks!!! I think I would have never achieved something like this without help since I have never used bit shifting before. I guess I learned something new today which can be useful for future projects. Is 32768 supposed to be the loudest point though? I noticed I was getting 0.927v across 4.7k load, higher values than 32768 caused drop in amplitude. Example with 34000, I was getting 0.91v and higher amplitude value caused even more drop.

/*
  This example generates a square wave based tone at a specified frequency
  and sample rate. Then outputs the data using the I2S interface to a
  MAX08357 I2S Amp Breakout board.

  created 17 November 2016
  by Sandeep Mistry
  modified for RP2040 by Earle F. Philhower, III <earlephilhower@yahoo.com>

    bool setBCLK(pin_size_t pin);
    - This assigns two adjacent pins - the pin after this one (one greater)
      is the WS (word select) signal, which toggles before the sample for
      each channel is sent

    bool setDATA(pin_size_t pin);
    - Sets the DOUT pin, can be any valid GPIO pin
*/

#include <I2S.h>

// Create the I2S port using a PIO state machine
I2S i2s(OUTPUT);

// GPIO pin numbers
#define pBCLK 2
#define pWS (pBCLK+1)
#define pDOUT 4

const int frequency = 440; // frequency of square wave in Hz
const int amplitude = 32768; // amplitude of square wave
const int sampleRate = 44100; // minimum for UDA1334A

const int halfWavelength = (sampleRate / frequency); // half wavelength of square wave

int16_t sample = amplitude; // current sample value
int count = 0;

void setup() {
  pinMode(LED_BUILTIN, OUTPUT);
  digitalWrite(LED_BUILTIN, 1);

  Serial.begin(115200);
  Serial.println("I2S simple tone");

  i2s.setBCLK(pBCLK);
  i2s.setDATA(pDOUT);
  i2s.setBitsPerSample(24);
  i2s.setLSBJFormat();

  // start I2S at the sample rate with 16-bits per sample
  if (!i2s.begin(sampleRate)) {
    Serial.println("Failed to initialize I2S!");
    while (1); // do nothing
  }

}
void sampleConverter(int l16, int r16) {
  int32_t l32 = ((int32_t)l16) << 16;
  l32 >>= 8; // IIRC, signed shift will replicate MSB for you
  int32_t r32 = ((int32_t)r16) << 16;
  r32 >>= 8;
  i2s.write24(l32, r32);
}

void loop() {
  if (count % halfWavelength == 0) {
    // invert the sample every half wavelength count multiple to generate square wave
    sample = -1 * sample;
  }
  sampleConverter(sample, sample);
  count++;
}

[earlephilhower]

I'm out now so can't run anything, but I'll give some hints.

Signed 16bit goes from -32768 to 32767. I2S is really fixed point fraction so it's really -1 to +1. Sending 38000 or doesn't make sense, it's really some negative number since you're 16 not signed.

I'd suggest starting out with simple text output %08x format sprintf to check the conversion, not audio. A 32767 in (0x7fff) should turn into a 32bit 0x007fff00. A 32768(-1) should turn into 0xffffff00. A 0x8123 should output 0xff812300. Etc. If that's off, then it needs fixing before going to actual output.

Once that's set you can do the 24 but writes out block writes.

[Sand192]

I will give this a try and will probably continue working on it tomorrow since it's getting late.

[Sand192]

This is the result I got with 32767, sprintf seemed to interfere with i2s and caused the dac to output strange noises, this is seen as a oscillation in values, i was able to see 007fff00 in serial monitor, maybe the dac is working fine after all since 32767 is max value of and I simply didnt realize, I will continue tomorrow:

[Sand192]

Sorry, the tiredness is getting to my head🤦. It seems to be fine after all. Now I have to implement it either into I2S library as i2s.setEIAJFormat which if called enables LSBJ and bit shifting logic or create separate bluetooth class called BluetoothConsumerEIAJ. I think it would be better to add it directly into I2S since it makes usage much easier for other projects. This should also work with other EIAJ dacs too as long as they accept input data with any word length like TDA1543A.

Once again, big thanks to you for helping me!!!

[earlephilhower]

Cool, no rush. A PR would be great. One note, though, this weird 8-bit MSB format will need special handling even in the high-speed write(data, len) method. Everywhere else the bits go out on the wire as they're send in (i.e. an MP3 decoder can dump 1152 samples in a block and write them w/1 call and ~0 CPU work). In this mode, you'll need to iterate over the date sent in by the app to do the MSB smearing. (I'd have the user need to use 24-bit mode so the input data will be large enough to handle this. OTW, things get really weird and rough in the handler...)

[Sand192]

I tried this modification, not the best but it works decently going from -32768 to 32767 in SimpleTone sketch:

size_t I2S::write24(int32_t l, int32_t r) {
     int32_t l32 = ((int32_t)l) << 16;
     l32 >>= 8; // IIRC, signed shift will replicate MSB for you
     int32_t r32 = ((int32_t)r) << 16;
     r32 >>= 8;
     return write32(l32, r32);
}

[Sand192]

I have also implemented part which enables EIAJ format and all of this works well, it worked even with 32 bits, I had to slightly modified the I2S::write24(); for it to know when it should use bit shifting, I used l = l32 and r = r32 to maintain support for 24 bit dacs which need i2s, this works in SimpleTone but needs some extra modifications to get it properly working in other sketches, in current state it only works if I2S.write24(); is constantly called in a loop, it could play audio only if new sample value is sent every time it loops:

bool I2S::setEIAJFormat() {
    if (_running || !_isOutput) {
        return false;
    }
    _isLSBJ = true;
    _isEIAJ = true;
    _bps = 24;
    return true;
}

size_t I2S::write24(int32_t l, int32_t r) {
     if (_isEIAJ == true) { // Enables bit shifting for eiaj support
       int32_t l32 = ((int32_t)l) << 16;
       l32 >>= 8; // IIRC, signed shift will replicate MSB for you
       l = l32;
       int32_t r32 = ((int32_t)r) << 16;
       r32 >>= 8;
       r = r32;
     }
     return write32(l, r);
}

[Sand192]

I have decided to try tampering with the 'fill();' and got similar result as before but with reduced hiss this time. The pitch being 2 times higher and buffering issue are still present and there also seemed to be slight delay between filling because the sound wasn't consistent. To be honest, i had zero clue of what these buffering functions do. Ideal solution would be to directly add it to 'I2S::write(const uint8_t *buffer, size_t size)' because that could make it work with most things that require high speed writing.

Your assistance would be greatly appreciated!

Anyway, here's the 'BluetoothAudioConsumer.cpp' buffer filling code that gave me slightly better result:

void BluetoothAudioConsumerI2S::fill() {
    int num_samples = _i2s->availableForWrite() / 2;
    int16_t buff[32 * 2];

    int32_t buff24[32 * 2]; // Buffer to hold 24-bit samples

    while (num_samples > 63) {
        _a2dpSink->playback_handler(buff, 32); 

        // Attempt to convert 16-bit samples to 24-bit samples
        for (int i = 0; i < 64; i += 2) {
            // Left channel
            int16_t sampleL = buff[i];
            int32_t l32 = ((int32_t)sampleL) << 8; // Shift left by 8 bits to align MSB
            buff24[i] = l32; // Stores in 24-bit buffer

            // Right channel
            int16_t sampleR = buff[i + 1];
            int32_t r32 = ((int32_t)sampleR) << 8; 
            buff24[i + 1] = r32; 
        }

        // Write the 24-bit samples to the I2S interface
        _i2s->write((uint8_t *)buff24, 64 * 3); 
        num_samples -= 64; 
    }
} 

[earlephilhower]

I think you're mostly there, but remember all 24-bit samples are stored in 32-bit left-aligned (since it's fixed point binary) and there's no need to handle the RHS specially. Untested edits below:

void BluetoothAudioConsumerI2S::fill() {
    int num_samples = _i2s->availableForWrite() / 2;
    int16_t buff[32 * 2];
    int32_t buff24[32 * 2]; // Buffer to hold 24-bit samples

    while (num_samples > 63) {
        _a2dpSink->playback_handler(buff, 32); 
        num_samples -= 64; 

        // Attempt to convert 16-bit samples to 24-bit samples
        for (int i = 0; i < 64; i ++) {
            int32_t l32 = ((int32_t)buff[i] << 16; // 0xabcd -> 0xabcd0000 (24-bit left aligned)
            buff24[i] = l32 >> 8; // 0xabcd0000 -> 0xffabcd00 (duplicate 8 MSBs, store in output buff)
            // TBD, volume control. See original code
        }

        // Write the 24-bit samples to the I2S interface
        _i2s->write((uint8_t *)buff24, 64 * 4); 

    }
} 

Make sure I2S is set to 24-bits in BluetoothAudioConsumerI2S::init. By default (w/o any setting) it defaults to 16.

[Sand192]

I tried it and it doesnt seem to work that well, the waveform in audacity looks strange and has delays between samples. The recorded output is on top and original is the second track. It's a very strange problem and i don't know what else would solve this issue but i don't want to give up yet because the SimpleTone works perfectly with bit shifting:

I also added zip containing original and recorded output: tda1543abttest.zip

[earlephilhower]

Recorded audio really isn't too useful here. If you have a simple USB logic analyzer and can record the I2S interface pins that would be very useful and let you double check the clock period, the # of bits between WL toggles, and that the MSBs are all smeared.

[Sand192]

Maybe scoppy might work, i'll give it a try.

[Sand192]

This is what i got with 440hz:

[Sand192]

Slightly better screenshot:

[earlephilhower]

Looks like the data's as you suggested your chip wants. I see 24 bits and the 1st 8 bits are all identical (all positive in these scans). I'd double check your wiring and any config on the unique DAC you've got. I'd triple check your chip's hookup and datasheet. Sometimes they want an MCLK, sometimes they can't actually run except at very specific frequencies (which the A2DP may not be running at), etc.

For the tone example you can set it to 24-bit mode and LSBJ mode and then use 0x007fff00 and 0xff800000 as the +ve and -ve parts of the signal. That's manually setting config and values to ones which should work and give a maximum amplitude square wave signal. If it doesn't, then there's a DAC config, timing, or wiring issue.

[Sand192]

Manual configuration worked in SimpleTone with bit shifting code, -32768 gave maximum negative amplitude and 32767 gave maximum positive amplitude, 0 gave nothing as it should. The wiring looks correct and resistors required for I/V conversion are well attached since these gave me 1k reading on all 3 of resistors.

[Sand192]

I adjusted the time div and noticed that first 3 ones look good but 3 data transfers at the end looked a bit strange to me. Are these okay?

[earlephilhower]

What exactly do you see that's wrong? These look to be negative values and again, no changes for 1st 8 cycles...

[Sand192]

I don't know, i might be a little confused. I measured these from gpio pins on the pico w running the A2DPsink sketch.

[Sand192]

Out of curiosity i tried i2s.setBitsPerSample(16); and it actually sounds cleaner but now the pitch is shifted lower by 2 times. i2s.setBitsPerSample(22); gives correct pitch with bit crushing;

Best regards,

[earlephilhower]

Setting 16b but still using the code in https://github.com/earlephilhower/arduino-pico/issues/2332#issuecomment-2289277536 means you're outputting garbage since your buffer sent in is 32b per sample. The L channel will get the upper 8 bits of the value, and the R channel will get the lower 8 bits (shfited left). It'll also play at 1/2 the expected frequency since you're taking 2 sample times to write 1 sample now.

Also, looking at the datasheets on your link, there are some variants that are normal and don't need 8 repeated MSBs. Double check your chip, maybe it was sold as a TD1543A but is really a TD1543 from Philips (with normal inputs)...

In any case, at this point there's not much I can do for you. The samples look to match this weird device, the per-sample conversion works, and that's really all that's needed. If you get clicking between samples increase the core frequency (A2DP is a compressed format so takes CPU cycles to decode). And of course don't do something silly like printf in the ::fill callback. OTW I might just recommend tossing this 33 year old ugly monster and finding something built this century by VLSI designers not on PEDs. :)

[Sand192]

Holy [redacted], i got it working with 16 bit LSBJ format, buff[i] = (((int32_t)buff[i]) * _gain) >> 16; was only what was needed. How i didn't realize something this simple. This is a very quick and dirty fix, enough for me but PR could use better solution than that. The bit shifting is still a requirement for manual writing.

[Sand192]

Well, i think i might close this issue now since i got it solved. Dumb me didn't realize that i would have had much less trouble by using something fancy and modern. At least it has been a decent learning experience for me.

All the best,