Adding support for Tranfer-Encoding: chunked streams

[yoav-klein]

Howdy Earl !

I first started using your great library just a while ago. I wanted to have a ESP8266 as a Radio player in my home since my radio reception in my home is not that good. So I did all the setup, but then I always got the lost synchronization.. error.

First googling, all I could find is "Try encrementing your buffer size", with no help.

After some research, I realized that the stream of my favorite radio station is using a Transfer-Encoding: chunked, and that what caused the loss of synchronization.

So I pulled up my sleeves and added support in the AudioFileSourceHTTPStream class to chunked Transfer-Encoding.

This could be also implemented in the ICY class as well, but for me it wasn't all that important and all the ICY thing was quite complex, so I didn't get to it.

Hope it will solve some other folks' same problem as well.

This is the radio stream I'm using if you want to check it out.

http://kanliveicy.media.kan.org.il/icy/749624_mp3

[yoav-klein]

Hi @earlephilhower , did have the chance having a look at this?

[earlephilhower]

Thanks for the PR. Sorry for the delay, I've been quite busy this past week. A few changes would make it a very good addition, I think!

[earlephilhower]

Not sure of the reason for std::size_t vs. the well-known stdint.h size_t?

[yoav-klein]

Does it really matter?

[earlephilhower]

I think the AudioFileSourceHTTPStream:: is superfluous here. We're already in class AudioFileSourceHTTPStream...

[yoav-klein]

From what I've checked, this is how it can be used. You're welcomed to further check it yourself.

[yoav-klein]

Hi @earlephilhower, fixed all needed to be fixed, please review

[DeqingSun]

@yoav-klein Thanks for providing the improvement. Just report 2 bugs I found:

1. The improvement will crash AudioFileSourceICYStream class, because the readImpl keeps uninitialized. I did some quick test to copy the open code to the AudioFileSourceICYStream. But the readInternal seems to be much more complicated and thrown a lot Couldn't read CRLF after chunk, something is wrong error. I did not fully investigate the issue. But it seems doing the icy in readInternal is quite easy to get conflict with the chunk. Maybe doing the icy in read and readNonBlock?

2. In readChunked function, if the len parameter is big, it becomes a long blocking function. In my test the len was 16K and the WDT reset was triggered, after the function run for a few seconds. A quick and dirty way is to add if (len > next_chunk) len = next_chunk;

after some tweak, here is my improvement. No loop in the function.

uint32_t AudioFileSourceHTTPStream::readChunked(void *data, uint32_t len, bool nonBlock)
{
  uint32_t bytesRead = 0;
  uint32_t pos = 0;
  
  if(len > 0)
  {
    if(len >= next_chunk)
    {
      if (next_chunk)
      {
        bytesRead = readInternal(data + pos, next_chunk, nonBlock);
        next_chunk -= bytesRead;
        pos += bytesRead;
      }
      len -= pos;
      if (!next_chunk){
        if(!verifyCrlf())
        {
          audioLogger->printf(PSTR("Couldn't read CRLF after chunk, something is wrong !!\n"));
          return 0;
        }
        next_chunk = getChunkSize();
      }
    }
    else
    {
      bytesRead = readInternal(data + pos, len, nonBlock);
      next_chunk -= bytesRead;
      len -= bytesRead;
      pos += bytesRead;
    }
  }
  return pos;
}

[DeqingSun]

And here are my functional AudioFileSourceICYStream, it can deal with both chunked or regular stream.
AudioFileSource_ICY_HTTPS.zip

[sh-user]

@DeqingSun i tested your code on ESP8266, it works but cuts off the end of the word
https://translate.google.com/translate_tts?ie=UTF-8&q=Text%20to%20Speech&tl=en&client=tw-ob

[yoav-klein]

HI @DeqingSun , thanks for the improvement!

I actually also experiencing some hiccups and also restarts every now and then. When I say now and then I mean it could go smooth for hours, but there are times that it's more frequent. I didn't get the chance to look deeply into why exactly this happens, but maybe it has something to do with what you're pointing at.

Although I find it hard to believe. It's not fresh in my memory, but as far as I remember, the reads in the original code are only one MP3 frame at a time, so how did you get to a point where the readChunked function received a len of 16K?

And I'll be pleased to know that - how do you know what function causes a reset? Would love if you could share with me some debugging skills so I could understand why I get all these hiccups and restarts.

[DeqingSun]

HI @yoav-klein,

For exceptions reset, ESP will print a Backtrace in 115200. And such info can be decoded by EspExceptionDecoder. However, it can not give useful info when you forget you return a value in function, WDT reset etc.

What I did was adding tons of print in every suspicious function, on the entry and exit part, and any point I feel suspicious. Also the print can carry info of the parameters and variables. With a lot of prints, you can see how the code flows, and see where it stuck.

If your code runs for hours before a bug, it might be helpful to get a serial logger like this one. I always want to get one but never get a chance to debug such a bug.

For the 16K problem, I think the read function in AudioFileSourceBuffer will try to fill the buffer in the first read. My code may break the read casually and return a significantly shorter response and may cause a longer delay at the beginning.

[DeqingSun]

@sh-user The key to solve the problem is to load the data as much as possible at the beginning and stop when the data stream stops. So some change to the read, the getChunkSize, needs to be done.

Try this version and use the AudioFileSourceICYStream class
AudioFileSource_ICY_HTTPS_20220205.zip
.

[sh-user]

checked, there is no longer a big delay after playback, but the last word is cut off. I noticed that if you send the text longer, then everything works fine
mp3

[DeqingSun]

@sh-user I tested on ESP32 with 16K buffer. That might be a reason.

[schmurtzm]

@sh-user I tested on ESP32 with 16K buffer. That might be a reason.

I confirm this behavior with Google TTS :

• Tested on ESP32 : it seems that the ESP32 plays the stream a little too long with a repeat of the last syllab during 1 second.
• Tested on ESP8266 : it seems that the ESP8266 cut the stream about one seconde before the end.

Still a lot better than before (with the current dev version it hangs during few minutes!)
It helps a lot for the resolution of my issue ! #475 (comment)
So thanks you for the modifications !

[DeqingSun]

@yoav-klein Another thing worth improving: The getChunkSize() using readStringUntil which is blocking. In the following case it may corrupt a lot of data.

1. The buffer is so large and it fetches all data from the wifi client
2. The network is slow
3. The Audio is decoding

When they all happens, the getChunkSize() may takes a long time to respond, and it will create a lot of hiccup and eventually kill the stream. I'll see if I can improve it.

[rebane2001]

Thank you @DeqingSun, your version seems to runs really well. I modified it to support https (through WiFiClientSecure) for a personal project and it still runs great on my ESP32!

[yoav-klein]

Hi @rebane2001 , Will your support for HTTPS will also work on ESP8266?

[rebane2001]

I'm not sure as I don't have an ESP8266 to test on, but you can see what I did in #628 if you want to try it yourself.

[yoav-klein]

@rebane2001 , can I pull your code from somewhere? Additionally, on what version of @DeqingSun were you talking about? Can you share the unified versions?

[rebane2001]

I used the version initially posted (AudioFileSource_ICY_HTTPS.zip), I didn't notice there was another one there. Here's my code at the moment:
with-https.zip

Fyi I've only tested/used this for http, not icy.