esp32-i2s-sdcard-wav-player icon indicating copy to clipboard operation
esp32-i2s-sdcard-wav-player copied to clipboard

Does this code actually work?

Open MartinNohr opened this issue 1 year ago • 2 comments

I have trouble believing that this code actually works. In the code: File root; creates a global structure "root". Then there are several function like this: int readRiff(File file, wavRiff_t *wavRiff) Where file is passed an argument. The first issue is that passing a global variable into a function is kind of pointless, just use the variable without passing it in. The second issue is more serious. C was changed many years ago to allow passing structures, which means that the value of "file" inside the function is a copy of the original. Any variable changed in there will not be reflected in the global variable. In the past a reference to the structure was passed so this kind of thing would work, but today it is far safer to explicitly pass the reference. I tested this code and it does not in fact read the correct data from the wav file. This is using the latest version of the updated SDFS library. I made it work by first changing the calls like this: int readRiff(File &file, wavRiff_t *wavRiff) But since file is a global it is better to not even pass in file as an argument. Of course, even better would be to not make "file" global, and then pass in the reference to the functions. Martin Nohr (30+ years of C/C++ experience)

MartinNohr avatar Feb 15 '24 00:02 MartinNohr

Yes, you are correct. Sometimes my mind is on the sky so the code was not optimized. Thanks.

Vào Th 5, 15 thg 2, 2024 vào lúc 07:17 MartinNohr < @.***> đã viết:

I have trouble believing that this code actually works. In the code: File root; creates a global structure "root". Then there are several function like this: int readRiff(File file, wavRiff_t *wavRiff) Where file is passed an argument. The first issue is that passing a global variable into a function is kind of pointless, just use the variable without passing it in. The second issue is more serious. C was changed many years ago to allow passing structures, which means that the value of "file" inside the function is a copy of the original. Any variable changed in there will not be reflected in the global variable. In the past a reference to the structure was passed so this kind of thing would work, but today it is far safer to explicitly pass the reference. I tested this code and it does not in fact read the correct data from the wav file. This is using the latest version of the updated SDFS library. I made it work by first changing the calls like this: int readRiff(File &file, wavRiff_t *wavRiff) But since file is a global it is better to not even pass in file as an argument. Of course, even better would be to not make "file" global, and then pass in the reference to the functions. Martin Nohr (30+ years of C/C++ experience)

— Reply to this email directly, view it on GitHub https://github.com/nhatuan84/esp32-i2s-sdcard-wav-player/issues/3, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABEN6LSGYFVOLXATTBLDSJDYTVHZPAVCNFSM6AAAAABDJKSQ6SVHI2DSMVQWIX3LMV43ASLTON2WKOZSGEZTKNBQHE2TANA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

nhatuan84 avatar Feb 16 '24 00:02 nhatuan84

I'm also puzzled by something, maybe I'm missing something... This code in the built-in DAC version reads only 1 byte, but isn't each sample 2 bytes, and four if stereo? (the other code reads 4 bytes, which makes sense) case DATA: uint8_t data; n = readbyte(root, &data); i2s_write_sample_nb(data); So how can that create the correct sound?

MartinNohr avatar Feb 18 '24 05:02 MartinNohr