newdigate / teensy-variable-playback

Firmware library: variable playback rate for teensy audio library
MIT License
54 stars 7 forks source link

Variable Playback with SPI Flash #45

Closed positionhigh closed 2 years ago

positionhigh commented 2 years ago

I try to use variable playback with playing samples from SPI Flash. (with serialflash.h and replacing the SD card commands) It does work for a short time but after 20-30 samples i run into full memory because of a memory leak. It looks like a simple new/delete issue but so far i can not fix it. Have you (or somebody else) got this working with flash memory ?

newdigate commented 2 years ago

Hi @positionhigh. Thanks for your patience, I think you have asked some time back on pjrc forum. Sorry for the delayed response.

I have successfully managed to play from flash. However, my approach is slightly different.

I am loading the audio sample from a uSD card into flash and playing it. I use the AudioPlayArrayResmp class which can play from arbitrary memory locations, in this case from the address which maps to the flash on teensy. Have a look at this example: sampleloader.ino - I'm not sure if this will be helpful, I think I have bypassed serialflash.h by writing directly to the flash memory address

Its been a while, but from what I remember there are a number of flash locations on the teensy. which teensy are you using and which flash? I think there is external and internal flash - the external flash is one of the optional footprints on the bottom of the teensy 4.1, the internal is 7MB within the MXRT1062? Is that right? Just trying to remember

positionhigh commented 2 years ago

Hello newdigate, thank you so much for your reply.

I am using a WINBOND W25Q128FVSG SERIAL FLASH MEMORY 3V 128M-BIT that i soldered onto the audio board flash location, not to the teensy (that on the teensy uses QSPI what is not really supported well, and if, then only with the newer filesystem, that is to slow for audio)

The flash on the audio board is managed by serialflash.h, which provides an SD-Card-like interface and seems usable for audio.

I was thinking to override the library and write/directly to the flash but since it is kind of working with a slightly modified version of your sd_reader code, i was hoping to get this as an "elegant" solution so that i can use the other features of the library and don't have to handle the flash content myself.

As said, it does work for a few samples, then the memory is full and it crashes. If i remember correctly, i get the same result even with your unmodified sd_reader and (really reading from sd_card).

Playing from progmen, however , works perfectly ,without loosing memory at all.

And also, when i replace your AudioPlayResmp with the Teensy Audio Library SerialFlashPlay (that can not pitch samples but plays also from flash, with the same serialflash.h Library) , it works perfectly - withour loosing memory.

I think it could be a constructor/destuctor problem, clearly something does not get cleared after playing each sample. But everything i tried so far, made no improvement.

newdigate commented 2 years ago

I have the audio board, and somewhere I think I have that flash too.

When I get a chance, I'll try and reproduce the issue.

positionhigh commented 2 years ago

That would be amazing.

i quickly stripped down /renamed your lib to TeensyVariablePlaybackFlash so it does not interfere with the regular VariablePlayback and should allow you to reproduce the problem:

https://codeberg.org/positionhigh/MicroDexed-touch/src/branch/main/third-party/TeensyVariablePlaybackFlash

If you like to check the full project, it is at: https://codeberg.org/positionhigh/MicroDexed-touch

localhero44 commented 2 years ago

Hi, I'm facing the same problem. @newdigate do you have found time to reproduce the issue ?

newdigate commented 2 years ago

unfortunately i haven’t had a chance to reproduce this yet.

i’m not sure i will have time in the next while, i’ll spare you the boring details, but hopefully i’ll be able to find some time for this issue in the next 6 months.

in the meantime i could suggest you take the juices bits of the resampling code and plonk them somehow in the serial flash audio component. i think the core mechanism minus the indexedfilereader is pretty stable.

localhero44 commented 2 years ago

Thanks, I'll ty even if I am not very good with C++. There are some parts of the code I don't understand, for example the IndexableFile class. What is its goal ? I also see it verifies that there are always 2 channels. Does it mean it can't be used to play mono samples ?

newdigate commented 2 years ago

indexedable file reader allows you to read a file as thou it were an array, while caching blocks of data to avoid unnecessary file reads. this is handy to read files backwards without making the code horribly complex.

there is a version in history which works well without this indexable file reader but is not as efficient when playing backwards.

i’m not aware of any restriction for stereo. as far as i know this code supports any number of channels, but i stand to be corrected. which line number are you referring to?

localhero44 commented 2 years ago

I've tried to investigate for a long time now, I really think the problem is in what you have commented in the close method: //TODO: dispose _sourceBuffer properly I've put some logs in this function and here is what I get when this close method is called :

ResamplingFlashReader::close, begin MEM:216468
ResamplingFlashReader::close, after delete MEM:216468
ResamplingFlashReader::close, end MEM:216468

I would say that it isn'tnormal that the memory space stays the same after delete _sourceBuffer; has been called. Each time a file is played I see the memory loses 528 bytes. And it goes down, down. These 528 bytes per file are never released. I still try to find why.

newdigate commented 2 years ago

thanks, that is helpful. I'll have a quick look. Its looks posible that indexedfilereader could have some leaks...

newdigate commented 2 years ago

see this commit - I have tried to improve the cleanup of _sourceBuffer;

If you apply similar changes to your ResamplingFlashReader, does it improve the memory leak?

newdigate commented 2 years ago

...actually, a couple more improvements...

positionhigh commented 2 years ago

wow i think this is a improvement. i quickly modifierd your changes to flash storage, maybe i made some mistakes there. I can tell that the free memory goes down less quickly than it did before. Will look at it in more detail tomorrow.

positionhigh commented 2 years ago

since you wrote the code for SD card, i tested how it is doing there, even i know that it will not will be fast enough for 8 tracks of sample playback. The memory drop is now only occuring when a new sample get plays for the first time. So that seems to be a big improvement. It still crashes after a while, because the sd card is to slow (i asume) or other issues. i will focus on flash tomorrow because it should not have the speed limitation of the standard sd card library. Thank you so much for helping in this topic !!!

positionhigh commented 2 years ago

ps: i am not sure if you are (fully) aware of our project but i would be VERY happy if you like it and join in, in our development. https://codeberg.org/positionhigh/MicroDexed-touch

localhero44 commented 2 years ago

Hi, I confirm the memory drop is much better now. There is maybe room for another improvement somewhere but it's much much better. Same for me, I will check again tomorrow my code quickly modified. Thank you !

localhero44 commented 2 years ago

I can see a first memory drop of 1056 bytes, then the following are 792 bytes. But I don't understand when they happen, a lot of samples are played, and suddenly some bytes are gone.

_Sequence started_
ResamplingFlashReader::play phKick1.wav - MEM:217524
ResamplingFlashReader::play HhCL1-808.wav - MEM:217524
**ResamplingFlashReader::play HhCL1-808.wav - MEM:216468**
ResamplingFlashReader::play HhCL1-808.wav - MEM:216468

then a lot of samples are played without memory loss. And :

ResamplingFlashReader::play HhCL1-808.wav - MEM:216468
ResamplingFlashReader::play DMpop.wav - MEM:216468
ResamplingFlashReader::play HhCL1-808.wav - MEM:216468
**ResamplingFlashReader::play CowBell.wav - MEM:215676**
ResamplingFlashReader::play HhCL1-808.wav - MEM:215676

...

ResamplingFlashReader::play HR16Snr2.wav - MEM:215676
ResamplingFlashReader::play HhCL1-808.wav - MEM:215676
**ResamplingFlashReader::play phKick1.wav - MEM:214884**
positionhigh commented 2 years ago

@newdigate

The results from SD CARD and FLASH seem to be similar, it is playing now much longer than before and at some random time it can't open the files anymore. It then either crashes or continues to play without audio:-)

Could very well be a problem with the access times. However, as told in one of my first postings, when variable playback from flash is exchanged with the stock-audioplay from FLASH from the teensy library (without pitching), it does play and find the samples reliably with no issues.

newdigate commented 2 years ago

@positionhigh, @localhero44

Can you try with .raw files instead of .wav files? There is some overhead with reading the .wav header, which is done with a seperate open, read, close file-io cycle. eliminating this may reduce memory leak. If it turns out to help a bit, we can look at improving the wav header read cycle.

newdigate commented 2 years ago

I have committed some more changes which could possibly help, I haven't had a chance to run/test these updates on a teensy yet, but they seem to work slightly better. https://github.com/newdigate/teensy-variable-playback/commits/master

localhero44 commented 2 years ago

Thanks Nic, I am trying the changes (I had already done some of them). Isn't there a need to also add disable/enable irq around the seek/read in Indexable.h ?

localhero44 commented 2 years ago

I don't know if this is really important but at each compilation I see these warnings :


IndexableFile.h:29:12: warning:   'size_t newdigate::IndexableFile<128u, 2u>::buffer_to_index_shift' [-Wreorder]
     size_t buffer_to_index_shift;
            ^
IndexableFile.h:30:5: warning:   when initialized here [-Wreorder]
     IndexableFile(SerialFlashFile file) : 
     ^
ResamplingFlashReader.cpp: In member function 'bool ResamplingFlashReader::play(const char*)':
ResamplingFlashReader.cpp:296:49: warning: 'wav_header.wav_header::data_bytes' may be used uninitialized in this function [-Wmaybe-uninitialized]
         _loop_finish = ((wav_header.data_bytes) / 2) + _header_offset; 
                                                 ^
ResamplingFlashReader.cpp:287:25: warning: 'wav_header.wav_header::bit_depth' may be used uninitialized in this function [-Wmaybe-uninitialized]
             Serial.print(wav_header.bit_depth);
                         ^
ResamplingFlashReader.cpp:294:23: warning: 'wav_header.wav_header::num_channels' may be used uninitialized in this function [-Wmaybe-uninitialized]
         setNumChannels(wav_header.num_channels);```
localhero44 commented 2 years ago

I've checked and tried the modifications. I have also tried to remove the useless close/open IO for the wav header, and also the close/open in Indexable and some others things. Unfortunately, it still finishes in the same way as Mark has reported : at one moment some wav files cannot be opened, that "crashes" the audio output but the sequencer is still running and the next wav files are found.

ResamplingFlashReader::play HhCL1-808.wav - MEM:215148 ResamplingFlashReader::play HR16Snr2.wav - MEM:215148 ResamplingFlashReader::play HR16Snr2.wav - MEM:215148 Not able to open file: HR16Snr2.wav ResamplingFlashReader::play HhCL1-808.wav - MEM:215148 Not able to open file: HhCL1-808.wav ResamplingFlashReader::play HR16Snr2.wav - MEM:215148 ResamplingFlashReader::play HhCL1-808.wav - MEM:215148 ResamplingFlashReader::play phKick1.wav - MEM:215148

newdigate commented 2 years ago

Not sure if this is worth mentioning. Mangu seems to have tried the serialflash route, he may have some experience.

After reading this, I think if you double the pitch of the samples, then playback at half the rate, my guess is you'll be able to play from serial flash for longer.

pitching down lowers the read rates from sdcard/flash, pitching down by 1 octave, the read rate will be halved). Probably worth a try.

You could use a strategy where you save multiple pitches of a sample and you always choose a sample with a higher pitch and then play it back with a lower pitch...

positionhigh commented 2 years ago

That is a good idea to look more closely at CPU and needed transfer rates. I will test if it makes a difference without any pitching and with only lower pitching than original.

newdigate commented 2 years ago

you could also look at testing mono vs stereo. it’s the same test, basically.

positionhigh commented 2 years ago

Localhero did magic. It is now working. Regardless how many samples i throw at it, it is not crashing. I think he is the only one who can explain what was wrong - the changes are already in our repository if you would like to check it. As far as i know we do not touch your regular VaribalePlayback but made this version as replacement when compiling for flash instead of Progmem. I believe this could also now work from SD-Card, with limitations. If not, than the SD Card with it's read speed will be the problem. I thank you both so much!! Now so many new opportunities are there to do with having the Flash working.

localhero44 commented 2 years ago

Yes, I will look to report some modifications in this repo if that can help other people. After so many trials I have found there was a misfunction between the irq enable/disable used and the Start/StopUsingSPI functions. I have commented out the Start/StopUsingSPI and now problems are gone (I hope so) :)

newdigate commented 2 years ago

awesome work, thank you both. Im stoked that you got serialflash working nicely with variable rate playback. Thanks!!

@positionhigh, would you mind if I include your serial flash resampling reader in my teensy-variable-playback? If you either send me a pull request or I can make the changes myself. Thanks.

Can I close this issue?

positionhigh commented 2 years ago

I think it would make sense to incorporate the recent changes from youself and from localhero also to the SD-Card functions of your library because i believe it has the same issues. It also would be good if you take in our copycat flash reader, maybe in a more elegant approach that there is not so much redundant code. This way we could drop our version and just use your main library for everything :_) In our code we have 3 defines that determin what it gets compiled for:

//#define COMPILE_FOR_PROGMEM //#define COMPILE_FOR_FLASH //#define COMPILE_FOR_SDCARD

Maybe you can do something like this in your main library ?

And yes, you can close the issue ! Thank you so much !

newdigate commented 2 years ago

I have refactored the code and integrated LittleFS for using flash, if you like you could give it a test.... https://github.com/newdigate/teensy-variable-playback/tree/feature/generics

newdigate commented 2 years ago

@positionhigh @localhero44 - I have made an experimental branch with SerialFlash support... https://github.com/newdigate/teensy-variable-playback/tree/feature/SerialFlash

positionhigh commented 2 years ago

Hello Nic, thank you we will test it. In the mean time we also tried all the other options. Our results so far are that all are "stable" but very different in speed. Best place is PROGMEM, then serialflash. SD Card seems to be the next best thing, there we currently are experimenting if we can get better results with the SDfat beta library. Worst so far is qspi, that is so slow that it seems not to be usable. This most certainly is a issue with LittleFs. However i will try your code version, maybe you did something different. Will keep you updated!

newdigate commented 2 years ago

Awesome work chaps, @positionhigh -- really appreciate your help!

newdigate commented 2 years ago

thing is with SD cards -- there are lots of different SD storage classes, some SD cards are useless, others are super fast

As per README.md - for best performance, use SDXC UHS 30MB/sec Application Performance Class 2 (A2) class micro sd-card.

positionhigh commented 2 years ago

I tested with your littlefs implementation on PROGMEM and also on qspi chip. Results are about the same for both , it runs stable with 1-2 concurrent samples, above that there are audio glitches. Both with littlefs from PROGMEM as on qspi, no real difference what is working better.

It is a shame that both methods can not be used with the serialflash.h, which seems to have none of these latency issues. However, littlefs still might be useful to have for other scenarios, where only playing only 1 sample at a time is needed.

positionhigh commented 2 years ago

ps: so far my results with the SDfat beta is that is as good as the standard SD.h, but not any better than it. (for playing samples)

positionhigh commented 2 years ago

https://www.youtube.com/watch?v=WFc2IrTSYDQ&list=PLHTypoMU1QoGOXPli8bjR6MknPiQpubHl&index=4

newdigate commented 2 years ago

@positionhigh, I am amazed! That is awesome work!

playing .mod and .it tracker files on a 1 bit pc speaker when i was a kid was my entry point into audio programming (made my first 8-bit sound card from a parallel port with resistors) !

I'm so impressed with your UI too!!!

positionhigh commented 2 years ago

i got Floyd Steinberg to make some kind of an introduction video. He has figured out most of the things but not really got everything correctly. Nevertheless it is worth watching: https://www.youtube.com/watch?v=ZFvrbFw_cPg

positionhigh commented 2 years ago

i have seen you have tried to make kind of a multiband compressor / shaper. I experimented with the basics of that and it does look/sound promising - have you made any further progress in this ? i understand that the variablefilter with 12db ? flank is not ideal for this - however , from what is experienced so far it sounds like it could be a useful tool ? Let me know about your experience or why you gave that up a few year ago ? TNX !

newdigate commented 2 years ago

markzp made a great dynamics module. i made a few minor tweaks and i use that with the built in filter for multipression. https://github.com/MarkzP/AudioEffectDynamics

positionhigh commented 2 years ago

thx, i will look into it!