Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

About examples->WebRadioDemo #47

Open
nopnop2002 opened this issue Mar 9, 2020 · 100 comments
Open

About examples->WebRadioDemo #47

nopnop2002 opened this issue Mar 9, 2020 · 100 comments

Comments

@nopnop2002
Copy link

nopnop2002 commented Mar 9, 2020

Hello.

client.read() also reads the HTTP header.

There is no fatal problem, but you can exclude the HTTP header as follows.

My Code:

void HexDump(uint8_t * mp3buff, uint8_t bytesread) {
  int loop = (bytesread + 9) / 10;
  uint8_t index = 0;
  char str[20];
  //Serial.print("bytesread=");
  //Serial.println(bytesread);
  Serial.println();
  for (int i=0;i<loop;i++) {
    for(int j=0;j<10;j++) {
      if (index < bytesread) {
        if (mp3buff[index] < 0x10) Serial.print("0");
        Serial.print(mp3buff[index], HEX);
        Serial.print(" ");
        if (mp3buff[index] >= 0x20 && mp3buff[index] <= 0x7F) {
          str[j] = mp3buff[index]; 
        } else {
          str[j] = 0x20;
        }
        str[j+1] = 0;
        index++;
      }
    }
    Serial.println();
    Serial.println(str);
    delay(1);
  }
}



uint8_t IsHeaderEnd(uint8_t * mp3buff, uint8_t bytesread) {
  static char previus = 0;
  static int status = 0;
  for(int i=0;i<bytesread;i++) {
    if (mp3buff[i] == 0x0D) {
      Serial.print("0D i=");
      Serial.print(i);
      Serial.print(" previus=");
      Serial.println(previus,HEX);
      if (previus != 0x0A) status=0;
      if (previus == 0x0A) status++;
    }
    if (mp3buff[i] == 0x0A) {
      Serial.print("0A i=");
      Serial.print(i);
      Serial.print(" status=");
      Serial.print(status);
      Serial.print(" previus=");
      Serial.println(previus,HEX);
      if (previus == 0x0D) status++;
      if (status == 3) return i+1;
    }
    previus = mp3buff[i];
  }
  return 0;
}


void loop() {
    static uint8_t startData;
    static bool detectedHeader = false;
    
    if(!client.connected()){
      Serial.println("Reconnecting...");
      if(client.connect(host, httpPort)){
        client.print(String("GET ") + path + " HTTP/1.1\r\n" +
                  "Host: " + host + "\r\n" + 
                  "Connection: close\r\n\r\n");
      }
    }
  
    if(client.available() > 0){
      // The buffer size 64 seems to be optimal. At 32 and 128 the sound might be brassy.
      uint8_t bytesread = client.read(mp3buff, 64);
      startData = 0;
      if (!detectedHeader) {
        HexDump(mp3buff, bytesread);
        startData = IsHeaderEnd(mp3buff, bytesread);
        Serial.print("startData=");
        Serial.println(startData);
        if (startData > 0) detectedHeader = true;
      }

      player.playChunk(&mp3buff[startData], bytesread);
    }
}

Test result:

Simple Radio Node WiFi Radio
Connecting to SSID aterm-d5a4ee-g
.....WiFi connected
IP address: 
192.168.10.121
connecting to ice2.somafm.com
Requesting stream: /seventies-128-mp3

48 54 54 50 2F 31 2E 31 20 32 
HTTP/1.1 2
30 30 20 4F 4B 0D 0A 43 6F 6E 
00 OK  Con
74 65 6E 74 2D 54 79 70 65 3A 
tent-Type:
20 61 75 64 69 6F 2F 6D 70 65 
 audio/mpe
67 0D 0A 44 61 74 65 3A 20 4D 
g  Date: M
6F 6E 2C 20 30 39 20 4D 61 72 
on, 09 Mar
20 32 30 32 
 202
0D i=15 previus=4B
0A i=16 status=0 previus=D
0D i=41 previus=67
0A i=42 status=0 previus=D
startData=0                    ------> Not detect end of HTTP header

30 20 31 30 3A 35 38 3A 33 36 
0 10:58:36
20 47 4D 54 0D 0A 69 63 79 2D 
 GMT  icy-
62 72 3A 31 32 38 0D 0A 69 63 
br:128  ic
79 2D 67 65 6E 72 65 3A 4D 65 
y-genre:Me
6C 6C 6F 77 20 37 30 73 0D 0A 
llow 70s  
69 63 79 2D 6E 61 6D 65 3A 4C 
icy-name:L
65 66 74 20 
eft 
0D i=14 previus=54
0A i=15 status=0 previus=D
0D i=26 previus=38
0A i=27 status=0 previus=D
0D i=48 previus=73
0A i=49 status=0 previus=D
startData=0                    ------> Not detect end of HTTP header

43 6F 61 73 74 20 37 30 73 3A 
Coast 70s:
20 4D 65 6C 6C 6F 77 20 61 6C 
 Mellow al
62 75 6D 20 72 6F 63 6B 20 66 
bum rock f
72 6F 6D 20 74 68 65 20 53 65 
rom the Se
76 65 6E 74 69 65 73 2E 20 59 
venties. Y
61 63 68 74 20 66 72 69 65 6E 
acht frien
64 6C 79 2E 
dly.
startData=0                    ------> Not detect end of HTTP header

20 5B 53 6F 6D 61 46 4D 5D 0D 
 [SomaFM] 
0A 69 63 79 2D 6E 6F 74 69 63 
 icy-notic
65 31 3A 3C 42 52 3E 54 68 69 
e1:<BR>Thi
73 20 73 74 72 65 61 6D 20 72 
s stream r
65 71 75 69 72 65 73 20 3C 61 
equires <a
20 68 72 65 66 3D 22 68 74 74 
 href="htt
70 3A 2F 2F 
p://
0D i=9 previus=5D
0A i=10 status=0 previus=D
startData=0                    ------> Not detect end of HTTP header

77 77 77 2E 77 69 6E 61 6D 70 
www.winamp
2E 63 6F 6D 2F 22 3E 57 69 6E 
.com/">Win
61 6D 70 3C 2F 61 3E 3C 42 52 
amp</a><BR
3E 0D 0A 69 63 79 2D 6E 6F 74 
>  icy-not
69 63 65 32 3A 53 48 4F 55 54 
ice2:SHOUT
63 61 73 74 20 44 69 73 74 72 
cast Distr
69 62 75 74 
ibut
0D i=31 previus=3E
0A i=32 status=0 previus=D
startData=0                    ------> Not detect end of HTTP header

65 64 20 4E 65 74 77 6F 72 6B 
ed Network
20 41 75 64 69 6F 20 53 65 72 
 Audio Ser
76 65 72 2F 4C 69 6E 75 78 20 
ver/Linux 
76 31 2E 39 2E 35 3C 42 52 3E 
v1.9.5<BR>
0D 0A 69 63 79 2D 70 75 62 3A 
  icy-pub:
30 0D 0A 69 63 79 2D 75 72 6C 
0  icy-url
3A 68 74 74 
:htt
0D i=40 previus=3E
0A i=41 status=0 previus=D
0D i=51 previus=30
0A i=52 status=0 previus=D
startData=0                    ------> Not detect end of HTTP header

70 3A 2F 2F 73 6F 6D 61 66 6D 
p://somafm
2E 63 6F 6D 0D 0A 53 65 72 76 
.com  Serv
65 72 3A 20 49 63 65 63 61 73 
er: Icecas
74 20 32 2E 34 2E 30 2D 6B 68 
t 2.4.0-kh
31 30 0D 0A 43 61 63 68 65 2D 
10  Cache-
43 6F 6E 74 72 6F 6C 3A 20 6E 
Control: n
6F 2D 63 61 
o-ca
0D i=14 previus=6D
0A i=15 status=0 previus=D
0D i=42 previus=30
0A i=43 status=0 previus=D
startData=0                    ------> Not detect end of HTTP header

63 68 65 2C 20 6E 6F 2D 73 74 
che, no-st
6F 72 65 0D 0A 41 63 63 65 73 
ore  Acces
73 2D 43 6F 6E 74 72 6F 6C 2D 
s-Control-
41 6C 6C 6F 77 2D 4F 72 69 67 
Allow-Orig
69 6E 3A 20 2A 0D 0A 41 63 63 
in: *  Acc
65 73 73 2D 43 6F 6E 74 72 6F 
ess-Contro
6C 2D 41 6C 
l-Al
0D i=13 previus=65
0A i=14 status=0 previus=D
0D i=45 previus=2A
0A i=46 status=0 previus=D
startData=0                    ------> Not detect end of HTTP header

6C 6F 77 2D 48 65 61 64 65 72 
low-Header
73 3A 20 4F 72 69 67 69 6E 2C 
s: Origin,
20 41 63 63 65 70 74 2C 20 58 
 Accept, X
2D 52 65 71 75 65 73 74 65 64 
-Requested
2D 57 69 74 68 2C 20 43 6F 6E 
-With, Con
74 65 6E 74 2D 54 79 70 65 0D 
tent-Type 
0A 41 63 63 
 Acc
0D i=59 previus=65
0A i=60 status=0 previus=D
startData=0                    ------> Not detect end of HTTP header

65 73 73 2D 43 6F 6E 74 72 6F 
ess-Contro
6C 2D 41 6C 6C 6F 77 2D 4D 65 
l-Allow-Me
74 68 6F 64 73 3A 20 47 45 54 
thods: GET
2C 20 4F 50 54 49 4F 4E 53 2C 
, OPTIONS,
20 48 45 41 44 0D 0A 43 6F 6E 
 HEAD  Con
6E 65 63 74 69 6F 6E 3A 20 43 
nection: C
6C 6F 73 65 
lose
0D i=45 previus=44
0A i=46 status=0 previus=D
startData=0                    ------> Not detect end of HTTP header

0D 0A 45 78 70 69 72 65 73 3A 
  Expires:
20 4D 6F 6E 2C 20 32 36 20 4A 
 Mon, 26 J
75 6C 20 31 39 39 37 20 30 35 
ul 1997 05
3A 30 30 3A 30 30 20 47 4D 54 
:00:00 GMT
0D 0A 0D 0A FF FB 92 04 ED 0F -------> 0D 0A 0D 0A is end of HTTP header
          
F3 32 3F 55 83 38 42 E0 65 E7 
 2?U 8B e 
EA B0 65 E8 
  e 
0D i=0 previus=65
0A i=1 status=0 previus=D
0D i=40 previus=54
0A i=41 status=0 previus=D
0D i=42 previus=A
0A i=43 status=2 previus=D
startData=44                 -------> Detect end of HTTP header
@nopnop2002
Copy link
Author

nopnop2002 commented Mar 9, 2020

I don't think it needs to be fixed.

It works fine without any changes.

Please close after reading this issue.

@baldram
Copy link
Owner

baldram commented Mar 10, 2020

Hi @nopnop2002! Thank you.
The community delivered the radio example. The author's idea was to provide a simple example. While people can extend it according to their needs, I think it's important to emphasise what are the potential issues to handle.
I will add a comment to the example pointing to this thread as well as the other one regarding buffer.

@nopnop2002
Copy link
Author

Yes
I think it's important to provide a simple example.

@fabitom
Copy link

fabitom commented Mar 10, 2020

Yup, metadata while streaming are disabled by default on most servers. You can control this by "Icy-MetaData: 0" header.
Unhandled metadata will cause clicking. But support for metadata is not simple, you need additional buffer or ringbuffer, grap icy-metaint, skip metadata chunks, parse icy-* headers...

@philippedc
Copy link

philippedc commented Mar 13, 2020

Hi, I made several tries and I understand now why sometime the sound is brassy, and sometime it is clicking. Sorry but for my experience ring buffer does not solve my problem.

First: For some radios the sound is brassy. This is solve by increasing the buffer size : the brassy effect seems to be the routine is too low compare to the streaming. By increasing the buffer the client.read function is done less time, so it is faster.

Second: For some stations that make many clicking effect after a moment (few seconds to few minutes) the VS1053 crash, I must reboot to come back to make it running again.
To solve this problem, I add some delays: the SPI.writeBytes seems to need a while between 2 calls. So I modify the VS1053.cpp to the following:

void VS1053::sdi_send_buffer(uint8_t *data, size_t len) {
size_t chunk_length; // Length of chunk 32 byte or shorter

data_mode_on();
while (len >= vs1053_chunk_size ) // More to do?
{
    await_data_request(); // Wait for space available
    len -= vs1053_chunk_size;
    SPI.writeBytes(data, vs1053_chunk_size);
    delay(1);
    data += vs1053_chunk_size;
}
data_mode_off();

}

Third: I have told in another thread that when I compare a radio reception from the device and a PC, I noticed that the streaming from this web radio is much faster that on the PC. After a while the web radio either looses the signal, either crashes. Depending of the signal bitrate, the client.read should be cadenced at the right time. I do not know how to do, because for the moment I do not know how to automatically define how long client.read takes to fill its buffer.
However I test this code that makes to web radio working much better:

static byte tempo = 0;
if( tempo %2 == 0 ) { // it is not use to run each time
ArduinoOTA.handle(); // for code update available from wifi
server.handleClient(); // call to web server
}

// if signal is lost
if( !client.connected() ) {
WiFi.status(); // wifi connexion checkup
Serial.println("Tentative de reconnexion...");
if( client.connect(urlStation, portStation.toInt()) ) {
client.print(String("GET ") + pathStation + " HTTP/1.1\r\n" +
"Host: " + urlStation + "\r\n" +
"Connection: close\r\n\r\n");
}
return;
} // end of !client.connected() test

// if connected to the radio station :)
//if( client.available() > 0 ) { // not usefull
if( !pause && tempo%10 == 0 ) {
if( client.read(mp3buff, 256) == 256 ) {
delay(1);
player.playChunk(mp3buff, 256);
}
else player.playChunk(sampleMp3, sizeof(sampleMp3)); // for debugging if buffer is not full
}
tempo++;
} // end of loop

This code uses very short time of 'pause' that synchronize the streaming. To improve this code, tempo%10 == 0 should be define automatically depending of the data flow speed.

@fabitom
Copy link

fabitom commented Mar 13, 2020

Whey you send 256 buffer to VS1053 then sdi_send_buffer will split that buffer to 32 (vs1053_chunk_size) and will send to VS1053 in loop. sdi_send_buffer already have proper delay in await_data_request() function. When Delay(1) in sdi_send_buffer helps you then I don't understand why...
await_data_request checking dreq pin, in other words asking vs1053 if we can send new data.
Hm... it looks like something is not making proper clock speed.
Can you test only mono output ?

  write_register(0x07, 0x1e09);
  write_register(0x06, 0x0001);

@philippedc
Copy link

@fabitom include now in the VS1053::begin() function, however I don't notice significant improvement.

@fabitom
Copy link

fabitom commented Mar 13, 2020

Ok, you can try this:

if( client.available() > 0 ) {
...
} else {
Serial.print('.');
}

and watch if problems occour when client don't have data.

@philippedc
Copy link

philippedc commented Mar 13, 2020

I did similar yet:
// if connected to the radio station :)
if( client.available() > 0 ) {
if( !pause && tempo == 0 ) {
bytesread = client.read(mp3buff, 512);
delay(1);
player.playChunk(mp3buff, bytesread);
}
}
else player.playChunk(sampleMp3, sizeof(sampleMp3)); // for debugging if buffer is not full
tempo++;
if( tempo > 10 ) tempo=0;

I never hear the specific sampleMp3 when a radio is listened. So the client.available is always true, and in fact I do not understand this test must be performed each loop.
However tempo is required for these clicking radios can play for more than 10 minutes. I improve duration with 30, 40, 50 instead of 10, but in this case most of other radios become brassy....

@philippedc
Copy link

.... I cannot see the way Edzelf has coded the equivalent of the client.read in its Esp-radio....

@baldram
Copy link
Owner

baldram commented Mar 13, 2020

@philippedc Thanks for this analysis.

To solve this problem, I add some delays: the SPI.writeBytes seems to need a while between 2
calls. So I modify the VS1053.cpp

Are you using ESP8266? Right?
I wonder whether instead of adding delay(1), the yield() would do the job?
Would you please check and adjust your VS1053.cpp modification?

yield

AFIK the "yield" is designed to solve such issues (As described here, the reason for Yielding is preventing from various issues. The ESP8266 runs a lot of utility functions in the background – keeping WiFi connected, managing the TCP/IP stack, and performing other duties. Blocking these functions from running can cause the ESP8266 to crash and reset itself).

If the yield works fine, I would add it to VS1053.cpp before SPI.writeBytes like you did.

@fabitom
Copy link

fabitom commented Mar 13, 2020

.... I cannot see the way Edzelf has coded the equivalent of the client.read in its Esp-radio....

Esp-radio client.read

@philippedc
Copy link

@baldram it sounds much better !

If the yield works fine, I would add it to VS1053.cpp before SPI.writeBytes like you did.

il is after. I've also had a yield in the loop:
if( client.available() > 0 ) {
bytesread = client.read(mp3buff, 256);
yield();
player.playChunk(mp3buff, bytesread);
}

It make twenty minutes the web radio did not crash.
I have a remark: most of the time the crash consists of the infinite repetition of the a noise that could be the playChunk buffer. Why in this case the ESP8266 does not perform a watchdog reset as the loop is stopped ?

@philippedc
Copy link

.... there is no WD reset because of the yield() in await_data_request(); I suppose ?

@nopnop2002
Copy link
Author

I will release the version corresponding to ESP-IDF.

https://github.com/nopnop2002/esp-idf-vs1053

It use RingBuffer.

But sometimes it lose network connection.

I don't know why.

Your modifications are welcome.

@baldram
Copy link
Owner

baldram commented Mar 14, 2020

@philippedc

it sounds much better!

I'm not sure I follow. Does this yield help when it's put after SPI.writeBytes(...)?

I've also had a yield in the loop
It makes twenty minutes the web radio did not crash.

Do I understand correct?
It crashes after 20 minutes when putting yield in the loop. Right?
But when putting after SPI.writeBytes, everything is fine like with dealy(1). Right?

I try to clarify whether to add yield or delay after SPI.writeBytes to the library's VS1053.cpp.

I have a remark: most of the time the crash consists of the infinite repetition of the a noise that
could be the playChunk buffer. Why in this case the ESP8266 does not perform a watchdog reset > as the loop is stopped ?
.... there is no WD reset because of the yield() in await_data_request(); I suppose ?

To be honest I don't have an answer to this question, unfortunately.
But I guess the indirect yield might help too.

@philippedc
Copy link

@baldram I have 3 of these web radios at home. So it is easy to compare different code version. I had the idea of a delay(1) after the SPI.writeBytes just like it is for an anlogRead(), just to let the time for the device to settle... In fact I did not test with the yield() before SPI.writeBytes , I will test this afternoon.
When I compare the new version of web radio with these yield() with the original code - they are identical except the yield(), I have less clicks - but there are - with the use of yield(), and if the streaming still takes some advance, this advance is very less than with the original version.
More over the clicking effect - the couiiiik should be the best expression - happens neither at the same streaming moment, nor at the same time. So I suppose it is a internal problem of the radio.

@baldram
Copy link
Owner

baldram commented Mar 14, 2020

@philippedc

I had the idea of a delay(1) after the SPI.writeBytes just like it is for an anlogRead(), just to let the time for the device to settle... In fact I did not test with the yield() before SPI.writeBytes , I will test this afternoon.

I mean using yield after the SPI.writeBytes only. Not before.
So I would like to clarify only whether modifying your code snippet with yield() like showed below is better?
Or maybe it's better to keep it as it is with delay(1)?
I mean your modification to VS1053.cpp.

yield

I will add the final solution to the library as it should not harm, but help in some cases, like yours.

So I suppose it is an internal problem of the radio.

It could be too, indeed.

@nopnop2002
Copy link
Author

nopnop2002 commented Mar 14, 2020

If you are developing an ESP32 application in an Arduino environment, it will be built with a single task and usually scheduled on Core 1, unless you use xTaskCreate ().

delay () returns control to the scheduler and grants execution rights to other tasks waiting to run, but has no effect if core1 has no other tasks.

On the other hand, SPI transfer requests are temporarily stored in the system ring buffer and processed by the system task, but the system task is executed by the core 0.

In the case of ESP32, Delay () is not effective in a single task environment.

This is my personal opinion.

@fabitom
Copy link

fabitom commented Mar 14, 2020

IMHO putting Delay() between SPI.BeginTransaction and SPI.EndTransaction is quite risky.
From what I see esp8266 need yield in main loop Adafruit VS1053
as example

        bytesread = client.read(mp3buff, 32);
        while (!player.data_request()) {
          yield();
        }
        player.playChunk(mp3buff, bytesread);

or after playChunk like here: frokats-project

@fabitom
Copy link

fabitom commented Mar 14, 2020

But sometimes it lose network connection.

I don't know why.

Your modifications are welcome.

Maybe wifi power save mode. I saw some similar problems. esp_wifi_set_ps

@nopnop2002
Copy link
Author

@fabitom
Thank you.
I'll try.

@nopnop2002
Copy link
Author

I added esp_wifi_set_ps, but still sometimes lose network connection.

There are other causes.

@baldram
Copy link
Owner

baldram commented Mar 15, 2020

@fabitom

IMHO putting Delay() between SPI.BeginTransaction and SPI.EndTransaction is quite risky.
From what I see esp8266 need yield in the main loop Adafruit VS1053 as example

Thanks.
I see something similar in Edzelf's radio: https://github.com/Edzelf/Esp-radio/blob/e1639010a80a6516191cc15a900599f8a6794dd4/Esp_radio.ino#L369-L375

Now, looking at the ESP_VS1053_Library it was correctly extracted and has this part too:
https://github.com/baldram/ESP_VS1053_Library/blob/master/src/VS1053.h#L71-L74

I wonder whether anything else is needed or just the client code might deal with yield additionally if required? I would put then comment to web radio example that in case of any issues it's worth to experiment in different ways, like @philippedc in own source code.

@philippedc
Copy link

@nopnop2002 does it lose the signal by the same way with all stations?

@philippedc
Copy link

philippedc commented Mar 15, 2020

@baldram I have left the idea of a yield or a delay in the library. I'm sure now the problem of clicking comes from client.read for some radio stations, unfortunately it happens for the one I listen the most.
It is not a question of speed, this station is 64k, therefore some 128k reception are ok.
The strangest thing is that reception is good at the beginning, the clicking effect grows slowly after a while (about 10 seconds)

@baldram
Copy link
Owner

baldram commented Mar 15, 2020

@philippedc

I have left the idea of a yield or a delay in the library
problem of clicking comes from client.read for some radio stations,

Thank you for clarification.

unfortunately, it happens for the one I listen the most

Sadly to hear that.

PS: On the other hand, looking at Edzelf's main radio loop, he is yielding a lot in the method mentioned below. So it's worth to keep this ESP feature in mind for some cases.
https://github.com/Edzelf/Esp-radio/blob/e1639010a80a6516191cc15a900599f8a6794dd4/Esp_radio.ino#L2069-L2070

@fabitom
Copy link

fabitom commented Mar 15, 2020

@philippedc please send me address of that 64k station. I will check on my radio.

@philippedc
Copy link

@fabitom here is the clicking station:
http://icecast.radiofrance.fr/franceculture-lofi.mp3 -> 64k
or http://icecast.radiofrance.fr/franceculture-midfi.mp3 -> the same in 128K, worst reception, crash after few minutes

This one is loosing signal from time to time, to get it back I change of station then I come back to it:
http://live02.rfi.fr/rfiafrique-64.mp3
or http://live-reflector.ice.infomaniak.ch/rfiafrique-64.mp3 -> the same radio station with the same loosing signal.

I have not yet studied in detail how Edzelf get the streaming, may be its method is more accurate.

@fabitom
Copy link

fabitom commented Mar 15, 2020

@philippedc All good here. Streams are ok.

@fabitom
Copy link

fabitom commented Mar 20, 2020

@nopnop2002 hm... idk, you need to test this. Imho its not problem with stream bitrate but TCP/IP stack implementation.
https://ivyknob.com/blog/esp8266-solving-random-connection-problems/

@baldram
Copy link
Owner

baldram commented Mar 20, 2020

build_flags = -D PIO_FRAMEWORK_ARDUINO_LWIP2_HIGHER_BANDWIDTH
Do I need this build_flags to listen to 128K bit rate radio stations?

As @fabitom already told, it is needed. It was even mentioned in a comment to the basic WebRadio example.
AFIK dealing with the proper LwIP (lightweight IP) variant is needed in general to reduce resource usage and handle performance issues for such low-cost devices like ESP8266.

The article mentioned by @fabitom shows how to enable it by those who use ArduinoIDE (you have to select it manually from the "tools" menu). Now you see btw. why PlatformIO is so good.
You don't need to care about such things, doing stuff manually (and forgetting something). Just simply set a build flag, and you're done.

Happy to see a release candidate of the code provided by @wmarkow. Would @philippedc you please test it to check whether it works well on your side?

@nopnop2002
Copy link
Author

nopnop2002 commented Mar 20, 2020

Unlike ESP8266, ESP32 may not be effective.

With build_flags = -D PIO_FRAMEWORK_ARDUINO_LWIP2_HIGHER_BANDWIDTH:

ESP-IDF version:v3.2.3-14-gd3e562907
lwIP version:2-0-3-255

Without build_flags = -D PIO_FRAMEWORK_ARDUINO_LWIP2_HIGHER_BANDWIDTH:

ESP-IDF version:v3.2.3-14-gd3e562907
lwIP version:2-0-3-255

My code:

#include "freertos/FreeRTOS.h"
#include "freertos/task.h"
#include "Arduino.h"
#include "lwip/init.h"

void setup() {
    Serial.begin(115200);
    Serial.print("ESP-IDF version:");
    Serial.println(esp_get_idf_version());
    Serial.print("lwIP version:");
    Serial.print(LWIP_VERSION_MAJOR);
    Serial.print("-");
    Serial.print(LWIP_VERSION_MINOR);
    Serial.print("-");
    Serial.print(LWIP_VERSION_REVISION);
    Serial.print("-");
    Serial.println(LWIP_VERSION_RC);
}

void loop() { // Never run
}

Not an important issue.

@baldram
Copy link
Owner

baldram commented Mar 20, 2020

@nopnop2002 If you use ESP32, this flag is not relevant.
See here: https://docs.platformio.org/en/latest/platforms/espressif8266.html
and here, it's not specified: https://docs.platformio.org/en/latest/platforms/espressif32.html .
(This is also not PlatformIO specific, it's just after the official ESP specification).

@philippedc
Copy link

void loop() {

ArduinoOTA.handle();        // allow update on air
server.handleClient();      // call to the server

// if signal is lost
if( !client.connected() ) {
WiFi.status(); // verify wifi
Serial.println("Tentative de reconnexion...");
if( client.connect(urlStation, portStation.toInt()) ) {
client.print(String("GET ") + pathStation + " HTTP/1.1\r\n" +
"Host: " + urlStation + "\r\n" +
"Connection: close\r\n\r\n");
}
} // end of !client.connected() test

// if station is connected :)
byte i=0;
byte noise=0;

if( client.available() && !pause ) {
while( i < 32 ) {
mp3buff[i] = client.read();

  if( i>noise && mp3buff[i] == '\n' && mp3buff[i-1] == '\r' ) {
    if( noise == 0 ) noise =i;
    else if( (i-noise)<10 ) {
      //Serial.print(i); Serial.print(":"); Serial.println(i-noise); 
      i=i-(i-noise);
    }
    i--;
  }
  else i++;
}

player.playChunk(mp3buff, 32); 
yield();

}
} // end of loop

The web radio is playing France culture for 10 hours now, without any abort and surely 99% of clicks off. Of course it is very less elegant than a ring buffer, however it can compile with Arduino IDE and core ESP8266 2.4.2

@fabitom
Copy link

fabitom commented Mar 20, 2020

@philippedc good for you ;) your example checking every byte of stream, does not matter if stream is chunked or not, I prefer doing this only if "transfer-encoding: chunked" header exists. But after all if you don't need CPU for other things then thats ok. After all need to check diffirence on esp load.
BTW CRLF can occour on last and first index.

@baldram
Copy link
Owner

baldram commented Mar 20, 2020

@philippedc Sounds good that it works.
Would you please find a time to check this code from @wmarkow : https://github.com/wmarkow/ESP_VS1053_Library/blob/web-radio-chunked-demo/examples/WebRadioChunkedDemo/WebRadioChunkedDemo.ino ?
Maybe it works too with good results for your fav radio station? As I understood, it was also prepared with Arduino IDE.

@philippedc
Copy link

philippedc commented Mar 21, 2020

@fabitom

BTW CRLF can occour on last and first index.

yes it is not elegant. Increasing buffer size from 32 to 128 helps for a lower probability to happen.

@baldram I tested all ring buffer examples available in this discussion. I do not know the reason, each time the program compiles, run a while but the sound is not displayed.

It is the same for the @wmarkow code example : https://github.com/wmarkow/ESP_VS1053_Library/blob/web-radio-chunked-demo/examples/WebRadioChunkedDemo/WebRadioChunkedDemo.ino

The program runs:
Annotation 2020-03-21 115952

but nothing is playing.

Please note I use a different pins connection for ESP8266:
#define VS1053_CS D0
#define VS1053_DCS D3
#define VS1053_DREQ D4
I cannot imagine it is the reason why ring buffer does not work.

@fabitom
Copy link

fabitom commented Mar 21, 2020

both examples (my, @wmarkow) works here on arduino 1.8.12 with esp8266 framework 2.6.3
Settings below:
obraz

@wmarkow
Copy link

wmarkow commented Mar 21, 2020

@baldram , PR ready. To keep the chunked transfer demo simple, I do not use the ring buffer there - as it is not required to correctly handle the audio stream. Only an 8 bytes length buffer is introduced to remove the chunk control data.

PS: Btw. what editor do you use?

I'm using Sloeber which is based on Eclipse CDT. The default formatter is not good enough as it uses TABs as indentation but in my PR I have used some custom formatter which is more or less compatible with Arduino IDE.

@philippedc
Copy link

both examples (my, @wmarkow) works here on arduino 1.8.12 with esp8266 framework 2.6.3
Settings below:
obraz

I use a new clean PC, install Arduino 1.8.12, ESP8266 2.6.3, etc...
It compiles without error, it is running, but no sound

@philippedc
Copy link

@wmarkow I test your code https://github.com/wmarkow/ESP_VS1053_Library/blob/web-radio-chunked-demo/examples/WebRadioChunkedDemo/WebRadioChunkedDemo.ino
There is a small error line 215:
for (index = 0; index < lengt; index++) {
it must be lenght

by the way, if the code gives sound, the quality is awful with much more glitches than ever :(

@baldram
Copy link
Owner

baldram commented Mar 22, 2020

@philippedc Thank you for testing the code.
Also, please feel free to comment on a PR here https://github.com/baldram/ESP_VS1053_Library/pull/48/files.

There is a small error line 215:
by the way, if the code gives sound, the quality is awful with much more glitches than ever :(

I will add your remarks.

@wmarkow
Thank you for the PR.

PS:

I'm using Sloeber which is based on Eclipse CDT. The default formatter is not good enough as it uses TABs as indentation but in my PR I have used some custom formatter which is more or less compatible with Arduino IDE.

Arduino IDE is not any reference from my point of view ;-) PlatformIO allows you to use an editor of your choice. There are several great IDEs, and one of them is Eclipse. I asked about the editor since I'm considering adding .editorconfig file to the project defining the project's formatting (there is not any at the moment so that everything will be reformatted). The configuration will be handled automatically by IDE for the project. The Editorconfig is compatible with various editors, e.g., CLion, VS Code, Eclipse, and more. Some of them through the plugin. Here is some randomly googled example .editorconfig file. It will be something like this example, and also I see it's quite similar to the formatting you proposed. The two spaces are correct also from my perspective.

@wmarkow
Copy link

wmarkow commented Mar 22, 2020

@baldram, the thing related to PR I have commented in the #48.

I didn't know about Editorconfig. Thanks for the tip. I will check how it works with Eclipse.
Code formatting is always a pain, so it would be good to have a common tool for this.

@baldram
Copy link
Owner

baldram commented Mar 22, 2020

@wmarkow I will add editorconfig separately for whole project and it looks like it will be compatible with your PR formatting.

@wmarkow
Copy link

wmarkow commented Mar 22, 2020

OK, please add the editorconfig for the whole projects. I will use in my PR.

@wmarkow
Copy link

wmarkow commented Mar 22, 2020

@philippedc, you wrote:

The program runs:
Annotation 2020-03-21 115952
but nothing is playing.

I just had the same situation; nothing was played in the headphones. Then I have restarted the board and everything was correct; the music was there. The code was the same: first the audio was not there (but I saw some printouts on the console), aftere reset everything was fine.
Weired situation that I do not like.

@baldram
Copy link
Owner

baldram commented Mar 24, 2020

@wmarkow I've just added coding style settings for the project.
Finally, I followed ESP-IDF style.
The other reason was that the project had already similar settings after Edzelf and Maniacbug.
So I kept it consistent. I hope you won't be unhappy. It means that the project's indent is 4 spaces.
Here is the code style settings file for the project.

Enabling the EditorConfig support should be easy. I use CLion, and it was just a matter of installing a plugin. Same if about Eclipse editor. Some editors like VS Code have the support enabled out of the box. Finally, the formatting just works for the project. My IDE simply sees the .editorconfig file and enables proper formatting for the current project. Works like a charm.

@fabitom
Copy link

fabitom commented Apr 14, 2020

There was problem with my ring buffer example, it didn't really work ;( There is PR with fixed version. Please test it. #54

@baldram
Copy link
Owner

baldram commented Jun 12, 2021

@philippedc Did you have a chance to test the latest version?

@philippedc
Copy link

hi @baldram sorry for my late answer. I've done the tests for several months.

Note I request the web radio url in http 1.0 instead of http1.1

when I update the firmware I lose in stability. The radio stops working after few minutes of use. I need to reboot to be able to listen back.

@baldram
Copy link
Owner

baldram commented Mar 22, 2022

@philippedc Thanks for reply.
Have you seen the latest improvements?
The possibility to load patches (discussion: #66 and PR: #69).

This might have a significant impact on improving stability.
Have you had chance to test this approach?

@philippedc
Copy link

Hi @baldram, yes I've followed the discussion. One question: the firmware update is done once, or does it needed at every reboot ?
If it is done once, yes the patches improve a lot the quality ! I'm not loosing reception anymore.
What I was meaning in my above message, I was loading patches at every reboot. After few weeks the webradio had difficulties to start. So I comment the line of code that performs the update. As it is still working better than ever, I'm figuring the update is permanent.

@baldram
Copy link
Owner

baldram commented Mar 22, 2022

Hi @philippedc

the firmware update is done once, or does it needed at every reboot ?

Every time.
Here we have even an interesting discussion, whether the invalid initialization sequence (switchToMp3Mode after loading the patch) will not erase it.

I'm figuring the update is permanent.

For sure not. The documentation says that patch must be re-loaded after each hardware or software reset.

@philippedc
Copy link

ok, in this case I do not perform the firmware update anymore.

@philippedc
Copy link

Hi @baldram I didn't notice I own a VS1003. So I've ordered a VS1053, to be aac compliant and to test the patches. I will be able to answer in few weeks....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants