Clarification on I2S library

[LinusHeu]

Hi,
I'm a bit confused about the documentation and code of the I2S library.
Could you please clarify the following for me? Or just confirm if what I write is correct.
My assumptions come from reading the code, but I'm not completely sure if I understand it correctly.

1. get bytes/Samples/words available for write
int availableForWrite()

Returns the number of L/R samples that can be written without potentially blocking.

But from looking at the code it looks to me like it returns the amount of 32-bit chunks that can be written.
Not necessarily the amount of samples. For example if using 8 bit per sample there would be two L+R samples in each 32-bit chunk. When using 32 bit per sample, each 32-bit chunk would only be half a L+R sample.

2. Transfer buffer to I2S
size_t write(const uint8_t *buffer, size_t size)

Transfers number of bytes from an application buffer to the I2S output buffer. Be aware that size is in bytes* and not samples. Size must be a multiple of 4 bytes. Will not block, so check the return value to find out how many bytes were actually written.

It looks like it only transfers the lower [bitsPerSample]-Bits per 32-bit chunk.

For example, if I'm using 16 bits per sample and my buffer looks like this, it would do this:

LLRR LLRR LLRR LLRR <- my buffer
  ^^   ^^   ^^   ^^
  LL   RR   LL   RR <- what write(const uint8_t *buffer, size_t size) transfers to the I2S output buffer

Is that correct?

To properly write my buffer to I2S I could:
A. Use size_t write(int32_t val, bool sync) which would actually write all bytes to I2S.
B. Split my buffer into each L+R sample and then use size_t write16(int16_t l, int16_t r) or size_t write(uint8_t/int8_t/int16_t/int32_t)

3. Return values for write methods
The methods
size_t write(uint8_t/int8_t/int16_t/int32_t)
size_t write(int32_t val, bool sync)
size_t write8(int8_t l, int8_t r) (and write16, write24, write32)

return size_t but they actually only return 1 on success, 0 on fail.
They won't write less than the full sample, samples, or 32-bit chunk that that they're provided with.
They write all or nothing.

size_t write(const uint8_t *buffer, size_t size)
Returns amount of bytes written

Thank you so much for this making this library and the entire arduino-pico core.
Many Greetings
Linus

[earlephilhower]

1. get bytes/Samples/words available for write int availableForWrite()

Returns the number of L/R samples that can be written without potentially blocking.
But from looking at the code it looks to me like it returns the amount of 32-bit chunks that can be written. Not necessarily the amount of samples. For example if using 8 bit per sample there would be two L+R samples in each 32-bit chunk. When using 32 bit per sample, each 32-bit chunk would only be half a L+R sample.

Yes, this is a known bug. It's a little more complicated than "multiply by 4 and then divide by 2*samplewidth" because you could read out the L but not R sample so you need to look at the leftover bits. It's a lot of housekeeping, but not really complicated. Most users have while (I2S.availableForWrite() { I2S.write(l, r); } so it's worked reasonably well so far even busted.

2. Transfer buffer to I2S size_t write(const uint8_t *buffer, size_t size)

Transfers number of bytes from an application buffer to the I2S output buffer. Be aware that size is in bytes* and not samples. Size must be a multiple of 4 bytes. Will not block, so check the return value to find out how many bytes were actually written.
It looks like it only transfers the lower [bitsPerSample]-Bits per 32-bit chunk.

This should be sending 32 bit chunks (including L and R samples when at 16b, for example). If not, that's obviously something that needs to be fixed:

arduino-pico/libraries/I2S/src/I2S.cpp

Lines 383 to 393 in a851a92

	int32_t *p = (int32_t *)buffer;
	while (size) {
	if (!write((int32_t)*p)) {
	// Blocked, stop write here
	return writtenSize;
	} else {
	p++;
	size -= 4;
	writtenSize += 4;
	}
	}

3. Return values for write methods The methods size_t write(uint8_t/int8_t/int16_t/int32_t) size_t write(int32_t val, bool sync) size_t write8(int8_t l, int8_t r) (and write16, write24, write32)
return size_t but they actually only return 1 on success, 0 on fail. They won't write less than the full sample, samples, or 32-bit chunk that that they're provided with. They write all or nothing.

Yes, kind of like fwrite with a non-byte sized unit of write. 1 == 1 combined sample here (whether that's 16b, 32b, or 64b total)

size_t write(const uint8_t *buffer, size_t size) Returns amount of bytes written

This was intended to be like other writes which return # of bytes.

[LinusHeu]

Thank you so much for your very quick and helpful reply.

for 2:
The reason why I think that size_t I2S::write(const uint8_t *buffer, size_t size) is behaving as I describe:

arduino-pico/libraries/I2S/src/I2S.cpp

Lines 383 to 385 in a851a92

	int32_t *p = (int32_t *)buffer;
	while (size) {
	if (!write((int32_t)*p)) {

*buffer is typecast to int32_t and size_t write(int32_t s) is called
which typecasts it to uint32_t and calls size_t write(uint32_t s)
which then calls size_t I2S::_writeNatural(int32_t s)

arduino-pico/libraries/I2S/src/I2S.h

Lines 71 to 76 in a851a92

	size_t write(uint32_t s) {
	return _writeNatural(s);
	}
	size_t write(int32_t s) {
	return write((uint32_t)s);
	}

And _writeNatural() uses bitwise and to ignore everything except for the lowest 8/16-Bits. (If 8/16 bits per sample is used. 24/32 bps = everything is written.)

arduino-pico/libraries/I2S/src/I2S.cpp

Line 268 in a851a92

_holdWord |= s & 0xffff;

This might be a fix (haven't tested it yet):

size_t I2S::write(const uint8_t *buffer, size_t size) {
    // We can only write 32-bit chunks here
    if (size & 0x3) {
        return 0;
    }
    size_t writtenSize = 0;
    int32_t *p = (int32_t *)buffer;
    while (size) {
        if (!write((int32_t)*p,false)) {
            // Blocked, stop write here
            return writtenSize;
        } else {
            p++;
            size -= 4;
            writtenSize += 4;
        }
    }
    return writtenSize;
}

[earlephilhower]

Odd, I would have thought this method would catch the write((int32_t)):

arduino-pico/libraries/I2S/src/I2S.cpp

Lines 286 to 291 in a851a92

	size_t I2S::write(int32_t val, bool sync) {
	if (!_running || !_isOutput) {
	return 0;
	}
	return _arb->write(val, sync);
	}

Just adding the false seems OK, or directly sending to arb->write() is probably the best way here. Want to do a PR since I think you're actively trying things out?

[LinusHeu]

I think I should do a pull request once I've verified that it works. The code looks correct with arb->write() but...
The problem is, I'm using this library https://github.com/pschatzmann/arduino-audio-tools, so there are way too many variables. And the API between said library and this I2S is broken. I think it wasn't updated for your new I2S implementation. So there are a lot of other things wrong still. Updating that library to properly support your new I2S is why I'm looking into all this I2S-stuff in the first place. And I feel like I'm kind of in over my head. This is the first time that I've not only used an arduino library, but also tried to improve/fix it. :D

1. Unchanged I2S:
   Track played within 15 seconds. Sounds like 2x speed.*
   some cracking

2. updated I2S::write(const uint8_t *buffer, size_t size) with either arb->write or write((int32_t)*p,false)
   Track played within 20 seconds
   very large amount of cracking

3. Iterate over the buffer within arduino-audio-tools and call I2S::write((int32_t)*p,false) from arduino-audio-tools
   Track played within 30 seconds
   large amount of cracking

I think the track is supposed to be 30 seconds long, but I'm not sure. I have no verified working code, the track is just called starwars30.h.

*Almost matches my idea that because _naturalWrite() is used, only some samples are played.

Next, I will write a sketch that only uses I2S and no audio library.
Do you have any .h audio file that I could directly output to I2S?
https://github.com/pschatzmann/arduino-audio-tools/tree/main/examples/examples-stream/streams-memory_raw-i2s The StarWars30.h here, is used in the example without any decoder. However, it's mono, so I think I have to convert it to stereo before outputting to I2S.
https://github.com/earlephilhower/ESP8266Audio/blob/master/examples/PlayWAVFromPROGMEM/PlayWAVFromPROGMEM.ino Viola.h here is .wav. Could I still output it directly? And is it stereo?

[earlephilhower]

Try increasing the buffers and buffer sizes before I2S.begin().. I think I was too stingy with RAM for them. If you underflow, you'll get clocks and pops as silence frames are pushed out.