One more memory leak in the Vorbis Decoder

[ma261065]

I think this is the last one.

I have printed out all the memory allocations and frees, and after your recent modifications, these are the last ones left where there is an allocation without a matching free.

In VORBISDecoder_FreeBuffers() there is a call to vorbis_dsp_destroy(), which is supposed to loop through the v->work and v->mdctright instances, based on how many channels there are, and free them.

However, in VorbisDecode(), there is this line:
if(s_f_oggLastPage && !s_vorbisSegmentTableSize) {VORBISsetDefaults();}
which gets called before VORBISDecoder_FreeBuffers()

Unfortunately VORBISsetDefaults() sets s_vorbisChannels to zero, which means when vorbis_dsp_destroy() gets called the two loops in there do zero iterations and the v->work and v->mdctright instances don't get freed.

Two other things:

1. This line (and others like it) if(v){free(v); v = NULL; doesn't do what I think you are expecting, as v is passed in by value, so setting it to NULL in the function doesn't alter it outside of the functions. I don't know if that breaks anything, but it might be good to fix.

2. You missed one of the "double if's" in VORBISDecoder_FreeBuffers() in your last fix - the one for s_mode_param is still there. No big deal, but would make for neater code if it wasn't there.

Thank you so much for this amazing decoder. It works extremely well for me, and with these final tweaks I think it will be even better.

[schreibfaul1]

Thank you for your detailed explanations, which enable me to follow your thoughts. If you have any further suggestions or improvements, please feel free to let me know. best regards
Wolle

[ma261065]

@schreibfaul1 thank you for the fixes.

I have two comments:

1. Did you mean to comment out s_vorbisChannels = 0; in VORBISsetDefaults()? It would still need to be set to 0 after the call to {vorbis_dsp_destroy(), wouldn't it?

2. My point about the v = NULL wasn't meant to imply that you should take out the free(v) code. Now you have just introduced a new memory leak for v haven't you? The reason for mentioning it was that setting v to NULL will have no effect outside of the function, and I'm not sure whether that is relied on in other parts of the code. But you should still free v.

[mmar22]

@schreibfaul1 @ma261065 now i test ogg station long time and seems somethink still eat SPIRAM after 20min i see -200kB
info VORBISDecoder has been initialized, free Heap: 122504 bytes, free stack 14368 DWORDs info syncword found at pos 0 info Zuzana Smatanová - Sľub, že prežijem info Channels: 2 info SampleRate: 44100 info BitsPerSample: 16 info BitRate: 128000 info Robin Schulz featuring Jasmine Thompson - Sun Goes Down [User] (14.585, +11121) WM8805_Update: WMPLL192 (in wolfson.c line #237) [User] (14.690, +105) WM8805_Update: WMPLL 176k4 (in wolfson.c line #238) [User] (14.794, +104) WM8805_Update: WMPLL192 (in wolfson.c line #237) [User] (14.898, +104) WM8805_Update: WMPLL 176k4 (in wolfson.c line #238) [User] (15.002, +104) WM8805_Update: WMPLL192 (in wolfson.c line #237) [User] (15.104, +102) WM8805_Update: WMPLL 176k4 (in wolfson.c line #238) 1310708 loop bezi rssi a memdma -72 110580 info HITY, KTORE MILUJETE info Pink - Runaway 1277940 loop bezi rssi a memdma -72 110580 1277940 loop bezi rssi a memdma -72 110580 info HITY, KTORE MILUJETE info HITY, KTORE MILUJETE 1245172 loop bezi rssi a memdma -72 110580 info Glass Animals - Heat Waves 1245172 loop bezi rssi a memdma -74 110580 info HITY, KTORE MILUJETE info James Arthur - Blindside 1212404 loop bezi rssi a memdma -72 110580 1212404 loop bezi rssi a memdma -72 110580 info HITY, KTORE MILUJETE info Kesha - Tik Tok 1179636 loop bezi rssi a memdma -72 110580 1179636 loop bezi rssi a memdma -74 110580 info HITY, KTORE MILUJETE info Rag'n'Bone Man - Skin 1146868 loop bezi rssi a memdma -73 110580 1146868 loop bezi rssi a memdma -73 110580 info HITY, KTORE MILUJETE info HITY, KTORE MILUJETE 1114100 loop bezi rssi a memdma -74 110580 info OneRepublic - Runaway 1081332 loop bezi rssi a memdma -73 110580 info HITY, KTORE MILUJETE info Felix Jaehn & Ray Dalton - Call It Love 1048564 loop bezi rssi a memdma -73 110580 1048564 loop bezi rssi a memdma -75 110580 info HITY, KTORE MILUJETE info ASDIS - Angel Eyes 1015796 loop bezi rssi a memdma -73 110580 info HITY, KTORE MILUJETE 999412 loop bezi rssi a memdma -75 110580

logline is from every 100sec
if(1 && (looper5%20000)==0) { Serial.print(heap_caps_get_largest_free_block(MALLOC_CAP_8BIT)); Serial.print(" loop bezi rssi a memdma "); Serial.printf("%d ",WiFi.RSSI()); Serial.println(heap_caps_get_largest_free_block(MALLOC_CAP_DMA)); }

[mmar22]

and after hour SPIRAM 0
110580 loop bezi rssi a memdma -72 110580
or better say
heap_caps_get_largest_free_block(MALLOC_CAP_8BIT) = heap_caps_get_largest_free_block(MALLOC_CAP_DMA)

next minutes
81908 loop bezi rssi a memdma -74 81908
73716 loop bezi rssi a memdma -72 73716
info HITY, KTORE MILUJETE
63476 loop bezi rssi a memdma -73 63476
...
info Nico Santos - Number 1
29684 loop bezi rssi a memdma -72 29684
info HITY, KTORE MILUJETE
12788 loop bezi rssi a memdma -73 12788
Guru Meditation Error: Core 0 panic'ed (LoadProhibited). Exception was unhandled.

Core 0 register dump:
PC : 0x42105569 PS : 0x00060430 A0 : 0x8205de38 A1 : 0x3fccd250
A2 : 0x00000040 A3 : 0x00000004 A4 : 0x600ff198 A5 : 0x00000000
A6 : 0x00000001 A7 : 0x00000001 A8 : 0x00000000 A9 : 0x3fce4ae4
A10 : 0x00000000 A11 : 0x00000000 A12 : 0x00000000 A13 : 0x00000000
A14 : 0x3fcc8ca8 A15 : 0x00000000 SAR : 0x00000001 EXCCAUSE: 0x0000001c
EXCVADDR: 0x00000004 LBEG : 0x400556d5 LEND : 0x400556e5 LCOUNT : 0xfffffffe

Backtrace: 0x42105566:0x3fccd250 0x4205de35:0x3fccd270 0x4205e529:0x3fccd290 0x420480f9:0x3fccd2c0 0x420483ff:0x3fccd300 0x4204c738:0x3fccd320 0x4204d5f7:0x3fccd350 0x420043b8:0x3fccd370

#0 0x42105566:0x3fccd250 in mdct_shift_right(int, int*, int*) at .pio/libdeps/esp32s3mykorvo/ESP32-audioI2S-master/src/vorbis_decoder/vorbis_decoder.cpp:1745 (discriminator 3)
#1 0x4205de35:0x3fccd270 in vorbis_dsp_synthesis(unsigned char*, unsigned short, short*) at .pio/libdeps/esp32s3mykorvo/ESP32-audioI2S-master/src/vorbis_decoder/vorbis_decoder.cpp:1714 (discriminator 2)
#2 0x4205e529:0x3fccd290 in VORBISDecode(unsigned char*, int*, short*) at .pio/libdeps/esp32s3mykorvo/ESP32-audioI2S-master/src/vorbis_decoder/vorbis_decoder.cpp:281
#3 0x420480f9:0x3fccd2c0 in Audio::sendBytes(unsigned char*, unsigned int) at .pio/libdeps/esp32s3mykorvo/ESP32-audioI2S-master/src/Audio.cpp:4250
#4 0x420483ff:0x3fccd300 in Audio::playAudioData() at .pio/libdeps/esp32s3mykorvo/ESP32-audioI2S-master/src/Audio.cpp:3443
#5 0x4204c738:0x3fccd320 in Audio::processWebStream() at .pio/libdeps/esp32s3mykorvo/ESP32-audioI2S-master/src/Audio.cpp:3069
#6 0x4204d5f7:0x3fccd350 in Audio::loop() at .pio/libdeps/esp32s3mykorvo/ESP32-audioI2S-master/src/Audio.cpp:2215 (discriminator 1)
#7 0x420043b8:0x3fccd370 in task_AAloop() at src/main.cpp:322 (discriminator 1)

[ma261065]

I wonder if the memory leak is in audio.cpp rather than the vorbis decoder?

I am using this decoder in a project where it is called from MicroPython, and it runs for days without showing a memory leak. Prior to the latest fixes there were definitely memory leaks, but not any more.

Obviously by using this decoder in such a way I am not using audio.cpp, which is what makes me think the memory leak might be there...

[mmar22]

Primary crashlog is result point of memory out no point of leak.
Secondary i test mp3 aac and all is ok. Only ogg station leak & crash ...
@ma261065 if you can try locate (you are more in code) test station "stream.funradio.sk:8000/fun128.ogg.m3u"

[schreibfaul1]

Hello mmar22,
memory leaks are hard to find, I will run "Funradio" for a long time and log the free memory every minute. I'll know more tomorrow.

[mmar22]

My tip leak is in tag info receive. When no info about track memory seems stay ok.

1277940 loop bezi rssi a memdma -72 110580
1277940 loop bezi rssi a memdma -72 110580
info HITY, KTORE MILUJETE
info HITY, KTORE MILUJETE
1245172 loop bezi rssi a memdma -72 110580
info Glass Animals - Heat Waves
1245172 loop bezi rssi a memdma -74 110580
info HITY, KTORE MILUJETE
info James Arthur - Blindside
1212404 loop bezi rssi a memdma -72 110580
1212404 loop bezi rssi a memdma -72 110580

seems two info no one, but maybe other

[schreibfaul1]

In the stream, a new codebook, residues, floors etc. is created for each song. This means that ~4KB are lost for each song. I corrected that, the old decoder configurations will be deleted beforehand.

[mmar22]

Now seems streaming ogg works fine. I see you after vorbis cpp change create many changes in audio code etc. But this changes make hard handle my spdif changes ...