earlephilhower / ESP8266Audio

Arduino library to play MOD, WAV, FLAC, MIDI, RTTTL, MP3, and AAC files on I2S DACs or with a software emulated delta-sigma DAC on the ESP8266 and ESP32
GNU General Public License v3.0
2.05k stars 435 forks source link

Stutter after 10-15 minutes of listening StreamMP3FromHTTP.ino #13

Closed renat2985 closed 6 years ago

renat2985 commented 6 years ago

I'm using your new example StreamMP3FromHTTP.ino. Listening this radio http://jazz.streamr.ru/jazz-64.mp3 The radio works well on computer.

ESP8266 07 - Stutter after 10-15 minutes of listening. If I turn off the power and then turn it back on, 10-15 minutes will work fine. How to solve this problem?

2017 11 23-09 45 42

earlephilhower commented 6 years ago

Can you post the serial output, too?

It's probably network buffering issues between the server and your ESP. Nothing that can be done w/o adding more RAM to the ESP. You could try increasing the buffer size in the code to see if it helps.

gerardwr commented 6 years ago

I changing the buffer from 2048 to 4096, that was an improvement. Increasing it to 8192 made things worse. For me 4096 is optimal. I have little/no disruptions with the latest repo streaming Mp3.

renat2985 commented 6 years ago

I changed the buffer to 4096, did not help. Log:

Decoding error 0x0235 (bad main_data_begin pointer) at byte offset 0
Decoding error 0x0235 (bad main_data_begin pointer) at byte offset 209
Decoding error 0x0235 (bad main_data_begin pointer) at byte offset 418
Buffering...
Buffering...
Buffering...
Buffering...
Buffering...
Buffering...
Buffering...
Buffering...
Buffering...
Buffering...
Buffering...
Buffering...

I want to try. If there is buffering, turn off the flow, and start it again. Help to do this:

  if (mp3->isRunning()) {

   ///THIS LINE NOT COMPILE
   if (Serial == "Buffering...") {mp3->stop();mp3->start();}

    if (!mp3->loop()) mp3->stop();
  } else {
    Serial.printf("MP3 done\n");
    delay(1000);
  }
earlephilhower commented 6 years ago

Sorry, but this is just a network issue between the server and your ESP8266. Your PC can buffer audio for many seconds, but the ESP only has ram for <1/4 of a second. I can only suggest trying a different streaming server, or if there is a lower bitrate they can stream at, trying that.

armSeb commented 6 years ago

Please retry as the buffering system has been improved ;)

renat2985 commented 6 years ago

Work is worse now! :-1: Log for new git pull:

Buffering...
Decoding error 0x0235 (bad main_data_begin pointer) at byte offset 0
Decoding error 0x0235 (bad main_data_begin pointer) at byte offset 209
Decoding error 0x0235 (bad main_data_begin pointer) at byte offset 417
Buffering...
Decoding error 0x0101 (lost synchronization) at byte offset 4907217
Decoding error 0x0235 (bad main_data_begin pointer) at byte offset 4907258
Decoding error 0x0235 (bad main_data_begin pointer) at byte offset 4907467
Decoding error 0x0235 (bad main_data_begin pointer) at byte offset 4907676
Buffering...
Decoding error 0x0101 (lost synchronization) at byte offset 4912446
Decoding error 0x0235 (bad main_data_begin pointer) at byte offset 4912483
Decoding error 0x0235 (bad main_data_begin pointer) at byte offset 4912692
Decoding error 0x0235 (bad main_data_begin pointer) at byte offset 4912901
Buffering...
Decoding error 0x0101 (lost synchronization) at byte offset 4918710
Decoding error 0x0235 (bad main_data_begin pointer) at byte offset 4918752
Decoding error 0x0235 (bad main_data_begin pointer) at byte offset 4918961
Decoding error 0x0235 (bad main_data_begin pointer) at byte offset 4919170
Buffering...
Decoding error 0x0101 (lost synchronization) at byte offset 4927905
Decoding error 0x0235 (bad main_data_begin pointer) at byte offset 4927947
Decoding error 0x0235 (bad main_data_begin pointer) at byte offset 4928156
Decoding error 0x0235 (bad main_data_begin pointer) at byte offset 4928365
Buffering...
Decoding error 0x0101 (lost synchronization) at byte offset 4931254
Decoding error 0x0235 (bad main_data_begin pointer) at byte offset 4931291
Decoding error 0x0235 (bad main_data_begin pointer) at byte offset 4931500
Decoding error 0x0235 (bad main_data_begin pointer) at byte offset 4931709
Buffering...
Decoding error 0x0101 (lost synchronization) at byte offset 4947759
Decoding error 0x0235 (bad main_data_begin pointer) at byte offset 4947800
Decoding error 0x0235 (bad main_data_begin pointer) at byte offset 4948009
Decoding error 0x0235 (bad main_data_begin pointer) at byte offset 4948218
gerardwr commented 6 years ago

Had this problem with previous repos too, see issue #9. With the latest repo I get a clean audio signal with 44khz mp3 streams, without disruptions.

You are compiling for Arduino settings 160Mhz and do NOT use lwip V2, right?

I use lwip Prebuilt source and that's OK, but lwip V2 gives many decoding errors.

renat2985 commented 6 years ago

@gerardwr, please show me your sketch. Works well, only 10-15 minutes. Then begins the Buffering.

2017 11 25-18 43 53

gerardwr commented 6 years ago

I use the standard StreamMP3FromHTTP from this library.

Info on lwip on wikipedia : https://en.wikipedia.org/wiki/LwIP

My settings on Arduino 1.8.4: schermafbeelding 2017-11-25 om 19 45 38

renat2985 commented 6 years ago

I want to do something this? Tell me how to do it right?

if (err == "lost synchronization") {
  mp3->stop();
  mp3->start();
}
armSeb commented 6 years ago

I noticed that the ESP8266 is very sensitive to wifi reception. I soldered a little piece of wire on the antenna, the signal quality was improved, so less buffer refill issue.

earlephilhower commented 6 years ago

@renat2985 There isn't a way to do that yet. Error handling and reporting is pretty bad (not there at all, really) as my initial use case was totally embedded with no UI. I could add a "GetLastError()" method to the AudioGenerators, but every generator is going to have a different error code (they're different libraries) and as far as the MP3 decoder is concerned, lost sync is more of a warning than an error as it'll just dump bits until it finds the next syncword and continue happily.

gerardwr commented 6 years ago

@earlephilhower :

I THINK I have a similar requirement as @renat2985. If you play a HTTP stream, you want to play it "forever". Currently the software reports "MP3 Done" in case of serious (connection) problems, and the stream is disrupted and you get a looping "MP3 Done" in the serial monitor. As a simple user you do not want to stare at the repeating "MP3 Done" messages, but you would like the code to restart the stream if possible.

Please allow me 2 thoughts for inspiration only , respect for your excellent work!

Thought 1

You have already added the "SetReconnect(nubmer_of_retries,delay)" function to address temporary connection problems. But in the current code I assume reconnecting stops AFTER the "number_of_retries". Possibly this function could be changed, if you set the "number_of_retries" to 0/zero the code retries connecting indefinitely. "SetReconnect(0,1000)" would then mean that a reconnection is retried indefinitely every seconds till the stream is picked up agian.

Thought 2

How about replacing the "MP3 Done" by setting up the stream all over again, as now done in "Setup".

So suppose you would change "loop" from

void loop()
{
  if (mp3->isRunning()) {
    if (!mp3->loop()) mp3->stop();
  } else {
    Serial.printf("MP3 done\n");
    delay(1000);
  }
}

to something like this

void loop()
{
  if (mp3->isRunning()) {
    if (!mp3->loop()) mp3->stop();
  } else {
    Serial.printf("MP3 stream interrupted, retrying to re-start\n");
    file = new AudioFileSourceHTTPStream(URL);
    buff = new AudioFileSourceBuffer(file, 4096);     // you can increase default 2048 bufffersize to 4096 (seems better) or 8192 (seems much worse)
    out = new AudioOutputI2SNoDAC();
    mp3 = new AudioGeneratorMP3();
    file->SetReconnect(3, 500);
    mp3->begin(buff, out);
  }
}

would that not result in a full restart of the stream.

I didn't have the time yet to try it out myself, sorry.

earlephilhower commented 6 years ago

@gerardwr your 1st idea, if implemented the way I'm doing now, would lock up the ESP8266 UI indefintely. I'm busy-waiting (well, not really since I'm delay()ing which lets the WiFi stack work, just not user code). It'd be possible to do it statefully, though. There'd still want to be some indication to the app that things are bad so it can display "buffering" or "retrying connection", though.

2 looks fine, but you need to delete all the old objects before creating new ones, or you'll leak memory and it will immediately crash (out of mem error).

earlephilhower commented 6 years ago

I've just pushed a commit that adds a status and metadata callback which can let you @gerardwr and @renat2985 detect events and act on them in your own application space. Give the example HTTPStream example a look, it shows how simple processing would be done.

renat2985 commented 6 years ago

@earlephilhower new pull StreamMP3FromHTTP.ino not compile. Please help. 2017 12 07-09 11 01

armSeb commented 6 years ago

Hello,

Is your SPIRam library installed from here: https://github.com/earlephilhower/ESP8266_Spiram/blob/master/src/ESP8266Spiram.cpp ?

The original SPIRam version will not compile as the arguments in class constructor are not the same.

renat2985 commented 6 years ago

@armSeb NO! Work, thanks!

Log:

Running for 1163168 ms...
Running for 1164169 ms...
Running for 1165170 ms...
Running for 1166171 ms...
STATUS(buffer) '3' = 'Buffer underflow'
STATUS(buffer) '2' = 'Refilling buffer'
STATUS(mp3) '257' = 'Decoding error 'lost synchronization' at byte offset 9095377'
STATUS(mp3) '259' = 'Decoding error 'forbidden bitrate value' at byte offset 9095384'
STATUS(mp3) '259' = 'Decoding error 'forbidden bitrate value' at byte offset 9095385'
STATUS(mp3) '259' = 'Decoding error 'forbidden bitrate value' at byte offset 9095386'
STATUS(mp3) '259' = 'Decoding error 'forbidden bitrate value' at byte offset 9095387'
STATUS(mp3) '259' = 'Decoding error 'forbidden bitrate value' at byte offset 9095388'
STATUS(mp3) '259' = 'Decoding error 'forbidden bitrate value' at byte offset 9095389'
STATUS(mp3) '565' = 'Decoding error 'bad main_data_begin pointer' at byte offset 9095419'
STATUS(mp3) '565' = 'Decoding error 'bad main_data_begin pointer' at byte offset 9095628'
STATUS(mp3) '565' = 'Decoding error 'bad main_data_begin pointer' at byte offset 9095837'
STATUS(buffer) '3' = 'Buffer underflow'
Running for 1167172 ms...
STATUS(buffer) '2' = 'Refilling buffer'
STATUS(mp3) '257' = 'Decoding error 'lost synchronization' at byte offset 9100606'
STATUS(mp3) '565' = 'Decoding error 'bad main_data_begin pointer' at byte offset 9100643'
STATUS(mp3) '565' = 'Decoding error 'bad main_data_begin pointer' at byte offset 9100852'
STATUS(mp3) '565' = 'Decoding error 'bad main_data_begin pointer' at byte offset 9101061'
STATUS(buffer) '3' = 'Buffer underflow'
STATUS(buffer) '2' = 'Refilling buffer'
Running for 1168197 ms...
STATUS(mp3) '257' = 'Decoding error 'lost synchronization' at byte offset 9106871'
STATUS(mp3) '565' = 'Decoding error 'bad main_data_begin pointer' at byte offset 9106913'
STATUS(mp3) '565' = 'Decoding error 'bad main_data_begin pointer' at byte offset 9107121'
STATUS(mp3) '565' = 'Decoding error 'bad main_data_begin pointer' at byte offset 9107330'
STATUS(buffer) '3' = 'Buffer underflow'
STATUS(buffer) '2' = 'Refilling buffer'
STATUS(mp3) '257' = 'Decoding error 'lost synchronization' at byte offset 9112100'
STATUS(mp3) '565' = 'Decoding error 'bad main_data_begin pointer' at byte offset 9112137'
STATUS(mp3) '565' = 'Decoding error 'bad main_data_begin pointer' at byte offset 9112346'
STATUS(mp3) '565' = 'Decoding error 'bad main_data_begin pointer' at byte offset 9112555'
Running for 1169198 ms...

Auto reconnect:

void StatusCallback(void *cbData, int code, const char *string)
{
  const char *ptr = reinterpret_cast<const char *>(cbData);
  Serial.printf("STATUS(%s) '%d' = '%s'\n", ptr, code, string);
 if (code == 3) {
    Serial.printf("Reconnect NUM!!");
    mp3->stop();
    file = new AudioFileSourceICYStream(URL);
    buff = new AudioFileSourceBuffer(file, 2048);
    out = new AudioOutputI2SNoDAC();
    mp3 = new AudioGeneratorMP3();
    mp3->begin(buff, out);
 }
  Serial.flush();
}

New log:

Running for 1167591 ms...
Running for 1168592 ms...
Running for 1169593 ms...
STATUS(buffer) '3' = 'Buffer underflow'
Reconnect NUM!!stream.error != mad_Err_bufflen
DNF failed
G1S failed

MP3 done
MP3 done
MP3 done
MP3 done

Exception (0):
epc1=0x40212c70 epc2=0x00000000 epc3=0x00000000 excvaddr=0x00000000 depc=0x00000000

ctx: sys 
sp: 3ffffc60 end: 3fffffb0 offset: 01a0

>>>stack>>>
3ffffe00:  00000001 00000000 00000002 40101ae6  
3ffffe10:  40004376 00000030 00000018 ffffffff  
3ffffe20:  60000200 00000008 7c037c02 80000000  
3ffffe30:  20000000 3fff1768 80000000 200fc180  
3ffffe40:  00000000 3fffc6fc 3ffee778 3fff176c  
3ffffe50:  00000174 000fc180 60000600 00000030  
3ffffe60:  00000123 40103d2d 00040000 00000001  
3ffffe70:  40102e63 00080000 3ffee150 00000030  
3ffffe80:  00000000 15bc5659 00000000 4000050c  
3ffffe90:  00000000 00000000 0000001f 401055d5  
3ffffea0:  4000050c 40212c70 3fffc270 4000050c  
3ffffeb0:  40000f68 00000030 0000001a ffffffff  
3ffffec0:  40000f58 00000000 00000020 00000000  
3ffffed0:  3fff0b4c 4021782c 00000000 00000000  
3ffffee0:  ffffffff 3ffe93d4 3ffee778 3fffdab0  
3ffffef0:  00000000 3fffdcb0 3ffee7b0 00000030  
3fffff00:  00000000 400042db 00000064 60000600  
3fffff10:  40004b31 3fff15ec 000002f4 000fc000  
3fffff20:  40105b3a 3ffee7a0 3ffed840 4010711c  
3fffff30:  402172fd 3ffed840 3ffee7a0 46017797  
3fffff40:  3fff15ec 00001000 40217792 00000008  
3fffff50:  401062f8 00000000 4021783f 3ffed8f4  
3fffff60:  3ffee7a0 15a03020 3ffee7a0 60000600  
3fffff70:  40228f05 3ffed8f4 3ffee7a0 46015387  
3fffff80:  40228f4a 3fffdab0 00000000 3fffdcb0  
3fffff90:  3ffee7b8 00000000 40000f65 3fffdab0  
3fffffa0:  40000f49 00006be9 3fffdab0 40000f49  
<<<stack<<<

 ets Jan  8 2013,rst cause:2, boot mode:(1,7)

 ets Jan  8 2013,rst cause:4, boot mode:(1,7)

wdt reset

How to reconnect without restarting?

armSeb commented 6 years ago

The AudioFileSourceHTTPStream class already handles the auto-reconnect feature, just call this method:

file = new AudioFileSourceHTTPStream(URL);
    buff = new AudioFileSourceBuffer(file, 4096);     // you can increase default 2048 bufffersize to 4096 (seems better) or 8192 (seems much worse)
    out = new AudioOutputI2SNoDAC();
    mp3 = new AudioGeneratorMP3();
    file->SetReconnect(3, 500);
    mp3->begin(buff, out);

Creating new classes instances will cause a crash as there is not enough RAM.

earlephilhower commented 6 years ago

You can't destroy and recreate the playing objects in the callback, either. They're calling you, and will continue to do stuff after you return. If they were deleted and then returned to, you'll crash on the first memory allocation or access.

If you really want to restart when it buffers, then set a global flag in your statusCB. In your main loop, check for it and if set do the delete/new cycle.

pseudo-code:

bool forceRebuild = false;

statusCB() {
  if (!strcmp(data, "buffer") && code==AudioFileSourceBuffer::STATUS_UNDERFLOW) {
    forceRebuild = true;
  }
...
}

loop() {
  if (mp3->running) {
    if (forceRebuild) {
      forcerebuild = false;
      ...delete and remake the objects...
    }
    mp3->loop();
  }
...
}
gerardwr commented 6 years ago

After installing the SPIRam library (as @armSeb commented) the example sketch StreamMP3FromHTTP compiles and runs OK.

Beware : Arduino defaulted to again to "LwIP-V2" and this reulted in tons of "decoding errors", but after switching to "lwIP Prebuilt Source" it runs fine.

This stream http://stream.morow.com:8000/morow.mp3 (128kbps 44.1kHz)

This stream http://icecast.omroep.nl/radio1-bb-mp3 (128kbps 48.0kHz)

gerardwr commented 6 years ago

And it resets :-(


Running for 2286359 ms...
Running for 2287362 ms...
STATUS(buffer) '3' = 'Buffer underflow'
STATUS(buffer) '2' = 'Refilling buffer'
STATUS(mp3) '257' = 'Decoding error 'lost synchronization' at byte offset 35806480'
STATUS(mp3) '257' = 'Decoding error 'lost synchronization' at byte offset 35806487'

 ets Jan  8 2013,rst cause:4, boot mode:(1,6)

wdt reset
ÄÄÄ
earlephilhower commented 6 years ago

That's a watchdog reset. No code in the actual guts was changed, only replacement of the Serial.* with callbacks. Whatever was causing that is probably down to either the MP3 decode logic or the IP stuff. Wish it had a stack trace, that would be very useful.

The other thing I'm seeing is the decode errors after buffering. Given that HTTP connections use TCP, a "lossless" protocol, a refill should not cause any changes to the actual bitstream received by the decode engine. Either the HTTP server itself is catching the TCP-NAKs and substituting the present-time output (and really causing a lost synchronization), or the buffer refill logic is causing a hiccup in the bitstream delivered to the MP3 decoder. That needs a looking into...

gerardwr commented 6 years ago

I rebooted after the "watchdog reset", and it's playing for almost 30 minutes now without problems as before, fingers crossed.

And it halted, but for another reason:

Running for 1756160 ms...
Running for 1757166 ms...
STATUS(buffer) '3' = 'Buffer underflow'
Running for 1792239 ms...
STATUS(buffer) '2' = 'Refilling buffer'
STATUS(buffer) '3' = 'Buffer underflow'
MP3 stop, len==0
DNF failed
G1S failed

MP3 done
MP3 done

Can't do a lot more for you than just testing the sketch behaviour right now, the internals of the connection code is "out of my leaque", sorry.

earlephilhower commented 6 years ago

I think the buffer errors are related to the LAME decoder pipeline. It sends a read request and if it comes back short (as in the case when it times out after 500ms) it still tries to decode but poops its pants since there's not enough data in its buffer. And then on the next pass it gets a full buffer but it's thrown out the first part of the packet when it pooped its pants on the last loop. So you get a second error.

I'll have to see if I can short-circuit that in the pipeline to allow a buffer refill.

The error you're showing now says that the server sent back 0 bytes to a read request. Whether it really sent a 0-byte response, or timed out in the TCP stack on the ESP8266...I'd put my money on the ESP8266 TCP stack going weird...

gerardwr commented 6 years ago

Sorry if my "help" gives you more worries. Tell me if you want me to stop "helping" ;-)

earlephilhower commented 6 years ago

Eh, no sweat. I actually only use this library in my kids' alarm clocks so it's good to have other folks who really use it extensively.

For the 48KHz station, how about dropping the sample rate to 24KHz by using the FilterAndDecimate stage. Have a 2-element filter and 1/2 and 1/2 as coefficients, then decimate by 2:1. That would pinpoint the pips as being timing related due to 48KHz underflow in the I2S subsystem (very possible since if the WiFi stack takes >~8ms you're out of I2S buffers to send). If the pops continue, it's not the I2S underflow but something else. If they stop, then there's just nothing for it other than larger I2S buffers (not the buffer you make in your code, the ones in the Arduino core libs).

gerardwr commented 6 years ago

"Everything for the kids", right ;-)

Understand that dropping the sample rate of the 48KHz station to 24kHz could have an influence on the "pips". I'll have look at the FilterAndDecimate stage to see if I can handle this.

Thx.

gerardwr commented 6 years ago

This stream http://stream.morow.com:8000/morow.mp3 (128kbps 44.1kHz) has been playing all day (from 9:00 till 22:40) without disconnecting with "Mp3 Done".

Problems I reported yesterday did not occur, and I assume they were due problems with the stream-server or the connection to it.

A "pip" once in a while (in any case at every METADA(ICY) message), but otherwise fine. No serial connection so I have no idea on "buffer" or "decoding error" issues.

The library has performed fine today :-)

Had a 30 sec peek at the code in "AudioOutputFilterDecimate.cpp" but inserting this in my sketch seemed like quite a challenge, so it have left it at that FTTB.

armSeb commented 6 years ago

Hello,

I have the same hiccups when using the ICY file source. These hiccups are random frequency and duration. I guess this is an output buffer underflow, as there is no error when this issue happens. Using normal file source (not ICY) solve the issue. It seems there is a function which consumes too many cpu cycles in the ICY receive method.

gerardwr commented 6 years ago

Hi @armSeb , So when I use

file = new AudioFileSourceHTTPStream(URL);

instead of

  file = new AudioFileSourceICYStream(URL);

"all" hickups are gone?

Can't test it myself ATM, not at my computer ATM.

armSeb commented 6 years ago

Yeah this is the case, and I found a workaround. When the read hits an ICY block, the read is splitted into three reads:

Instead of that, I modified the method. When the read hits an ICY block, I just make first and second reads (first part of mp3 data and ICY block). If the read doesn't hit an ICY block, I call the third read (which is a normal read). I think this code can be optimized a lot, but for now I just modified the code as this:

            AudioFileSourcePROGMEM afsp(value, strlen(value));
            AudioFileStream afs(&afsp, strlen(value));
            cb.md(type, false, &afs);
          }
          do {
            ret = mdr.read(&c, 1);
          } while ((c !=';') && (c!=0) && !mdr.eof());
        }
      }
    }
    icyByteCount = 0;
  } else {

    int ret = stream->readBytes(reinterpret_cast<uint8_t*>(data), len);
    read += ret;
    pos += ret;
    icyByteCount += ret;
    //Serial.printf("Read: %u\n");
  }
  return read;
}

I just added an else statement to call the second read only when there is not icy block. So, when the ICY block hit happens, the returned data is less than available in the socket, but the function returns faster the mp3 data.

With this modification, the random hiccups are gone.

armSeb commented 6 years ago

I think that any called function must be quick to execute and return, to let enough time to mp3 decoder to do his job. In the future, we should be able to increase the audio output buffer, which will make the code less sensitive to mp3 decoding latencies. The mp3::loop function have to be called very often.

earlephilhower commented 6 years ago

@armSeb I'll look at your ICY changes a little later. The real problem, I think, is that the slow bits are in the ESP8266 core and LWIP. :(

One suggestion would be to drop the METADATA() print completely. Serial output is slow, and it's not needed here. That might be the whole cause of the pips, waiting for the Serial port to finish.

@gerardwr Happy to hear it worked for so long, but I know it's not stable by any means yet. If the stream is perfect, the MAD decoder seems to run fine. If errors are introduced, bad stuff seems to happen up to and including hangs. This could be my fault in some of the code I've written, or it could be out-of-memory errors (we're running at the razor-edge now).

I just ported the HELIX MP3 library and will spend some time trying to memory-optimize it as it doesn't need dynamic allocations in the main decode loop, which would make it stabler. Right now, if you get some memory fragmentation (say, from the TCP stack so it's not even visible or repeatable) you might end up blowing up. Even though there's enough free memory, it's not contiguous enough to meet the malloc requirements.

earlephilhower commented 6 years ago

D'oh. The stuttering w/ICY is most probably due to the Serial.flush() in the MetadataCB and StatusCB functions. That will busy-wait for however long it takes to dump the new title out at a slow baud rate...removing that may help significantly w/ICY hiccups.

armSeb commented 6 years ago

The hiccups doesn't happen when printing informations on the serial port, but at any time. Doing one read instead of two removed the random hiccups.

earlephilhower commented 6 years ago

Weird. I'm only using the internal buffer and had it running for a while debugging the WebRadio.ino example and didn't notice any hiccups while listening, neither when printing out the MD() lines, nor when I had them removed and just set the web page internal variable. What LWIP version are you using? It seems that if that's the only change (1 less call to the LWIP stack in your method, assuming that the MD block fits in the buffer, if I understand it correctly) then the stack has a lot of latency on each invocation. :(

I think the bigger issue is memory reduction to avoid the random hangs more than hiccups in the case of errors. It seems like its borderline w/o the external SRAM buffer. If only the RealPlayer decoder was faster. I can't believe how it can't hit realtime on a 160MHz 32-bit integer core. See the AudioGeneratorMP3a() object for details. If you find anything glaringly wrong I'd be ecstatic. Those kinds of bugs are easier to fix than the subtle little ones...

armSeb commented 6 years ago

Unfortunately the AudioGeneratorMP3a class doesn't compile:

src/libhelix-mp3/imdct.c:519: undefined reference tostack'` etc...

I'm using the lwip from stable arduino library (not git version).

EDIT: the ICY parsing function is too big, it creates a big hiccup when receiving the song title. It can be better (to my mind) to just read the block into a pointer and set a flag, then the client can request parsing later.

earlephilhower commented 6 years ago

Thanks for the heads-up on the debug stack() calls. I've removed them (I have an unchecked-in debug file w/the call so I didn't see it in my builds). No need to test, as I was able to get a testbed running and helix-mp3 is ~2.7x slower than libmad but does take several KB less RAM.

There are a few messages corroborating that on an unoptimized AVR PIC32 (his results were almost exactly mine). Unfortunately the Xtensa in the ESP8266 doesn't have the 1 instruction needed to make the multiply fast (signed mult32x32, upper-32bit result) and it takes four (slowish) multiplies and software-handled 64-bit extensions and adds to calculate the fixed-point mults that are the core of the codec.

armSeb commented 6 years ago

Regarding the hiccup when getting the ICY title, it is caused by this function:

stream->readString();

When I call it, this causes systematically a hiccup, even if I don't display the title.

EDIT: The Helix MP3 library is very slow and doesn't work as well:

*WM: 
*WM: AutoConnect
*WM: Connecting as wifi client...
*WM: Using last saved values, should be faster
*WM: Connection result: 
*WM: 3
*WM: IP Address:
*WM: 192.168.1.2
SPI RAM buffer size: 131072 Bytes
6448
Buffering...
Filling Done !
STATUS(mp3) '-2' = 'MP3 decode error -2'
STATUS(mp3) '-9' = 'MP3 decode error -9'
STATUS(mp3) '-9' = 'MP3 decode error -9'
STATUS(mp3) '-6' = 'MP3 decode error -6'
STATUS(mp3) '-6' = 'MP3 decode error -6'
STATUS(mp3) '-9' = 'MP3 decode error -9'
STATUS(mp3) '-9' = 'MP3 decode error -9'
armSeb commented 6 years ago

With the original library:

Connecting to WiFi
*WM: 
*WM: AutoConnect
*WM: Connecting as wifi client...
*WM: Using last saved values, should be faster
*WM: Connection result: 
*WM: 3
*WM: IP Address:
*WM: 192.168.1.2
SPI RAM buffer size: 131072 Bytes
20944
Buffering...
Filling Done !
STATUS(mp3) '565' = 'Decoding error 'bad main_data_begin pointer' at byte offset 0'

It seems the free heap is more important than the Helix library

earlephilhower commented 6 years ago

Yes, but the Helix library seems to use less memory so it helps with the heap. In any case when I moved the example to a custom, non memory hog web server, I was able to get several streams stopped and started through the interface.

If you have better parsing code that doesn't use extra memory for ICY, I'd be happy to see it. As I don't have much to go on, I tried to make the existing code snippet handle almost any case. If you are guaranteed to get only one line with the same MD then it can be made way simpler.

armSeb commented 6 years ago

Hello,

The issue is passing by AudioFileSourcePROGMEM and AudioFileStream. I guess you are doing that to avoid storing the block into RAM.

Without this code, playing is fine. I just pass the value to my own callback function.

earlephilhower commented 6 years ago

Got it. I only had the Stream interface to help with the ID3 decoding anyway.

I've replaced the Stream with a simple char* callback value which should alleviate the problem. I've updated the MP3/ID3 and WebRadio examples and moved a bunch of strings to PROGMEM.

With those changes and the removal of the ESP8266WebServer I'm able to allocate a 4K RAM buffer on an un-augmented ESP8266 and still have ~11K free in the main loop() which seems to make things massively more stable.

armSeb commented 6 years ago

Did you use the git version of Arduino-esp8266 ?

error: 'class HardwareSerial' has no member named 'printf_P'

earlephilhower commented 6 years ago

I'm using GIT head, give or take. Are you using 2.3 release? I believe 2.4-rc has the _P methods, and it's recommended to use that (at least, the bug reports all get responded to as "try the -rc version!")

armSeb commented 6 years ago

I used 2.3 release. Switched to 2.4-rc2 solved the issue. Please add this requirement in the README to avoid compilation issues ;)

earlephilhower commented 6 years ago

Done, thx!

earlephilhower commented 6 years ago

Closing as I think we're good now. Stuttering == needs more buffering, either go ESP32 or go ESP8266 + SPISRAM.