karawin / Ka-Radio32

esp32 version of Ka-Radio (https://github.com/karawin/Ka-Radio) by jp Cocatrix
525 stars 155 forks source link

No good reason for limiting memcpy size in spiram_fifo.c ? #112

Closed har-in-air closed 5 years ago

har-in-air commented 5 years ago

Not sure if there are legacy issues which resulted in the fifo read and write being limited to 1024 bytes at a time (some other 24LC type serial ram perhaps). I cleaned up spiram_fifo.c and this code works fine for me. I have a espwrover homebrew board based internet radio and am using 2201024 sized fifo without any issues with the attached code. I limited fifo size to 220kbytes because i never saw the buffer fill up to more than ~50% when I used 4201024 as per original code

(I have a slow wifi hotspot connection to internet). spiram_fifo.c.txt

karawin commented 5 years ago

To be generic, the ram is detected and if present, the size is

define BIGMEMORY 131072

I.e 128K This is about 30seconds of play without reception. It is enough. More is disturbing the title detection. As you noticed, it is hard to fill the buffer more than that. Your solution is good for you but, only for configuration with the external ram. A clenest solution is to change the define in buffer.h.

But it not an issue for my point of view. buffer.c: ICACHE_FLASH_ATTR void initBuffer() { if (externram== false) { // BUFFER_SIZE = 12960; BUFFER_SIZE = 14080; // BUFFER_SIZE = 16000; buffer = malloc(BUFFER_SIZE); } else{ BUFFER_SIZE = BIGMEMORY; }
}

ICACHE_FLASH_ATTR void initBuffer() { if (externram== false) { // BUFFER_SIZE = 12960; BUFFER_SIZE = 14080; // BUFFER_SIZE = 16000; buffer = malloc(BUFFER_SIZE); } else{ BUFFER_SIZE = BIGMEMORY; }
}

har-in-air commented 5 years ago

Its not the size of the fifo ram i'm commenting about, just that the fifo read and writes break up the buffer into 1024 byte chunks in the existing code, and I don't see a good reason for that. Of course the existing code works fine, it just seems inefficient.

karawin commented 5 years ago

Just to limit the time and let the hand to the scheduler. the spi bus is slow and may be needed by the vs1053 and lcd if presents.

har-in-air commented 5 years ago

OK thanks. I haven't integrated an LCD yet so did not see any issues with user interface responsiveness. In any case, the PSRAM spi bus is not the same as used by the vs1053 / LCD right ? Most of the pins are in parallel with the flash and CLK and CS are pins 16/17. But I understand UI response may be affected.

karawin commented 5 years ago

Ok. A generic software is not so easy to write ;-) Thanks.