Das sollte erst gefixt werden. Das hat auf jeden Fall funktioniert, als ich das damals implementiert habe.
Klar, kann ich machen. @tueddy kannst du die M3U hochladen, dann schaue ich es mir morgen an.
Ich habe das auch, dachte es liegt an meiner Playlist.
Ich hätte noch ein Idee , wenn du schonmal dabei bist.
Durchschalten über die Endpunkte der Liste hinaus. Also von Ende wieder nach 1 und von 1 wieder ans Ende. Ich hatte das vor einiger Zeit nach @biologist ´s Tipp implementiert. Das ist bei Webradio ganz praktisch. Das war echt einfach, ich muss mal danach suchen.
Hier die Playlist (Das könnte sogar die von @compactflash gewesen sein). Hatte mal stack-canary exception weil hier auch SSL verwendet wird. Dazu habe zum Test die Stackgröße des Audiotask erhöht. Aber auch dann beendet sich die Playlist nach Sender 4:
#EXTM3U
# http://web.ard.de/radio/radionet/
# http://www.wdr.de/wdrlive/wdr2player.html
#EXTINF:0,103.7 UnserDing (MP3)
http://streaming01.sr-online.de/unserding_2.m3u
#EXTINF:0,1LIVE (MP3)
http://www.wdr.de/wdrlive/media/einslive.m3u
#EXTINF:0,1LIVE diggi (MP3)
http://www.wdr.de/wdrlive/media/einslivedigi.m3u
#EXTINF:0,1LIVE Kassettendeck (MP3)
http://www.wdr.de/wdrlive/media/1LIVE_kassettendeck.m3u
#EXTINF:0,Radio Bremen vier (http)
http://icecast.radiobremen.de/rb/bremenvier/live/mp3/128/stream.mp3
#EXTINF:0,Radio Bremen vier (https)
https://icecast.radiobremen.de/rb/bremenvier/live/mp3/64/stream.mp3
nee, hier ist meine
Webradioliste1.txt (662 Bytes)
Hm, hab den Absturz. Liegt an wdr.de, die leiten den nicht gefundenen link http://www.wdr.de/wdrlive/media/1LIVE_kassettendeck.m3u
auf eine https 404-Tut-uns-Leid Seite um. Mein mp3play stirbt mit einem Stack canary
Hab den Fehler gefunden: https://github.com/biologist79/ESPuino/blob/894c6d8392b4f1aa92d3c73bf10635e8b4b59e1b/src/AudioPlayer.cpp#L809
Das Problem ist, dass die Verbindung zum Server funktioniert, aber audio->isRunning()
ist false. Damit kommt es zu einem Timeout mit settleCount
um die Playlist wird abgebrochen.
Ich bin am Fixen
edit
Die Änderung ist hier. Ich habe auch die Stack Größe um knapp 2k erhöht um keine Ausfälle bei https streams zu haben. Sollte das nicht gewpnscht sein, kann ich das natürlich zurück ändern.
Danke für die Fehleranalyse & den Fix, ich probiers gleich aus!
Ja HTTPS verbraucht enorm viel Speicher! Empfehlung für Webstreams, sucht Euch immer die http-Adresse ohne SSL, meist bieten die Sender beides an.
Wir haben letztens die Stackgröße schonmal angehoben. Ich meine es steht dadurch auch weniger Heapspeicher zur Verfügung? Frage ist halt wie weit man es hier treibt…
Ich bin mir unsicher bzgl. Stackerhöhung. Vielleicht lassen wir das im Dev-Branch einfach mal laufen…
Edit:
Der Bugfix für einen fehlerhaften Eintrag in der M3U-Playlist klappt hervorragend!
Es wird bis zum Timeout gewartet und dann auf den nächsten Track gewechselt. So kann man in der Weboberfläche auch sehen warum das ungültig ist, im Titel steht dann z.B. „(5/13): HTTP/1.0 404 Not Found“.
Ja, also ich wäre da auch vorsichtig, das schon wieder anzuheben. Vor allem gleich um so viel.
Ansonsten, wenn das gescheit läuft, dann sollte man vielleicht den aktuellen DEV in den Master mergen. Und erst DANACH die Playlist-Optimierung in den DEV übernehmen.
Ich habe den letzten Commit mit dem M3U Fix extrahiert und eine neue PR gestartet (dabei gleich auch die Stackgröße wieder auf 6k stellen). Denn diese Funktioniert auch ohne den restlichen Commits für die Playlist Optimierung.
Danke für deine Arbeit!
das funktioniert nicht im „Rückwärtsgang“ , da wird dann wieder der nächste Track vorwärts angesprungen, Man kann jetzt jedoch mit nochmaligem Tastendruck rüberspringen , was vorher nicht ging.
Ich bekomme jetzt nach dem Fix sporadisch entweder die Meldung „Keine Playlist“ order einen Crash beim Auflegen einer Karte (Hörbuch von SD):
D [59121] Gebe Speicher der alten Playlist frei (Freier Speicher: 74032 Bytes)
D [59121] Freier Speicher nach Aufräumen: 74032 Bytes
D [59143] Freier Speicher: 73880 Bytes
N [59143] Playlist-Generierung
N [59149] Anzahl gültiger Files/Webstreams: 2
N [59150] Modus: Spiele alle Tracks (alphabetisch sortiert) des Ordners '/2 kleine WoeGuru Meditation Error: Core 1 panic'ed (LoadProhibited). Exception was unhandled.
Core 1 register dump:
PC : 0x400949ca PS : 0x00060230 A0 : 0x800d403e A1 : 0x3ffda8a0
A2 : 0x00000000 A3 : 0x030b0031 A4 : 0x00000003 A5 : 0x00000000
A6 : 0x00000000 A7 : 0xff000000 A8 : 0x030b0031 A9 : 0x00000000
A10 : 0x00000000 A11 : 0x00000068 A12 : 0x3f401330 A13 : 0x00000001
A14 : 0x00060220 A15 : 0x00000001 SAR : 0x0000000a EXCCAUSE: 0x0000001c
EXCVADDR: 0x030b0031 LBEG : 0x40085755 LEND : 0x4008575d LCOUNT : 0x00000027
Backtrace: 0x400949c7:0x3ffda8a0 0x400d403b:0x3ffda8c0
#0 0x400949c7:0x3ffda8a0 in strncmp at /builds/idf/crosstool-NG/.build/HOST-x86_64-w64-mingw32/xtensa-esp32-elf/src/newlib/newlib/libc/string/strncmp.c:65
#1 0x400d403b:0x3ffda8c0 in AudioPlayer_Task(void*) at src/AudioPlayer.cpp:680
ELF file SHA256: 19d587ebfe3061d0
E (5532) esp_core_dump_flash: Core dump flash config is corrupted! CRC=0x7bd5c66f instead of 0x0
Rebooting...
Es scheint als ob die Playlist unterm Hintern weggezogen und NULL wird. Dann crasht es im Audioplayer Zeile 680 beim Stringvergleich auf „http“?
Kommentiere ich den Fix aus klappt es wieder. Könnt Ihr das bestätigen?
Ja, kann ich nachstellen. Es scheint eine Race Condition zwischen dem neuen Timeout im AudioPlayer_TrackQueueDispatcher
und AudioPlayer_TrackQueueDispatcher
zu sein. Ich vermute, dass gPlayProperties.trackFinished
durch den Timeout gesetzt wird und damit beim Empfangen der neuen Playlist der erste Track sofort übersprungen wird
Könnt ihr prüfen, ob der folgende Ptach abhilfe schafft?
diff --git a/src/AudioPlayer.cpp b/src/AudioPlayer.cpp
index 5d9b888..fc9792b 100644
--- a/src/AudioPlayer.cpp
+++ b/src/AudioPlayer.cpp
@@ -348,7 +348,7 @@ void AudioPlayer_Task(void *parameter) {
#endif
constexpr uint32_t playbackTimeout = 2000;
- uint32_t playbackTimeoutStart = 0;
+ uint32_t playbackTimeoutStart = millis();
AudioPlayer_CurrentVolume = AudioPlayer_GetInitVolume();
audio->setPinout(I2S_BCLK, I2S_LRC, I2S_DOUT);
@@ -404,12 +404,19 @@ void AudioPlayer_Task(void *parameter) {
trackQStatus = xQueueReceive(gTrackQueue, &gPlayProperties.playlist, 0);
if (trackQStatus == pdPASS || gPlayProperties.trackFinished || trackCommand != NO_ACTION) {
if (trackQStatus == pdPASS) {
- if (gPlayProperties.pausePlay) {
- gPlayProperties.pausePlay = false;
- }
audio->stopSong();
Log_Printf(LOGLEVEL_NOTICE, newPlaylistReceived, gPlayProperties.numberOfTracks);
Log_Printf(LOGLEVEL_DEBUG, "Free heap: %u", ESP.getFreeHeap());
+ playbackTimeoutStart = millis();
+ gPlayProperties.pausePlay = false;
+ gPlayProperties.repeatCurrentTrack = false;
+ gPlayProperties.repeatPlaylist = false;
+ gPlayProperties.sleepAfterCurrentTrack = false;
+ gPlayProperties.sleepAfterPlaylist = false;
+ gPlayProperties.saveLastPlayPosition = false;
+ gPlayProperties.playUntilTrackNumber = 0;
+ gPlayProperties.trackFinished = false;
+ gPlayProperties.playlistFinished = false;
#ifdef MQTT_ENABLE
publishMqtt(topicPlaymodeState, gPlayProperties.playMode, false);
@@ -985,12 +992,6 @@ void AudioPlayer_TrackQueueDispatcher(const char *_itemToPlay, const uint32_t _l
gPlayProperties.playMode = _playMode;
gPlayProperties.numberOfTracks = strtoul(*(musicFiles - 1), NULL, 10);
// Set some default-values
- gPlayProperties.repeatCurrentTrack = false;
- gPlayProperties.repeatPlaylist = false;
- gPlayProperties.sleepAfterCurrentTrack = false;
- gPlayProperties.sleepAfterPlaylist = false;
- gPlayProperties.saveLastPlayPosition = false;
- gPlayProperties.playUntilTrackNumber = 0;
#ifdef PLAY_LAST_RFID_AFTER_REBOOT
// Store last RFID-tag to NVS
Bei mir läuft es mit der Änderung ohne Fehler. Ich muss schauen, ob es eine schönere Lösung gibt.
Ja, das behebt bei mir die Exception . Wäre das durch den anderen Playlist-PR überhaupt noch relevant oder ist das unabhängig davon?
Update:
funktioniert leider doch nicht in allen Fällen. Habe zum testen den Audio-Task mal auf Core 0 zugewiesen, jetzt kommt die exakt selbe Exception wieder auch mit dem Fix. Zugegeben etwas ein Grenzfall
Grml… Hab das Problem. Wir setzten playMode in Zeile 985, potentiel lange bevor wir die Playlist in die Queue zB in Zeile 1055 schieben. Hier schlägt leider Multithreading (bzw SMP) zu, je mehr Dateien in dem Ordner sind und je mehr operationen ausgeführt werden müssen, desto öfters kann ich den Fehler erzwingen ( also zB wenn alle Dateien Random angespielt werden sollen). Wenn wir lange brauchen, kann der mp3 Thread zum Zug kommen (der auch höhere Prio hat).
Wird der Dispatcher zwischen Zeile 985 und 1025 unterbrochen und der AudioTask läuft, ist mit dem gesetzten playMode & numberOfTracks ist die Bedingung für die Audio Timeout gegeben. Der setzt gPlayProperties.trackFinished
und wir versuchen zu dem 2. Track zu springen, die Playlist ist aber zu dem Zeitpunkt noch immer ein nullptr
. Damit exlodiert strncmp
in Zeile 680.
diff --git a/src/AudioPlayer.cpp b/src/AudioPlayer.cpp
index 5d9b888..8538be4 100644
--- a/src/AudioPlayer.cpp
+++ b/src/AudioPlayer.cpp
@@ -348,7 +348,7 @@ void AudioPlayer_Task(void *parameter) {
#endif
constexpr uint32_t playbackTimeout = 2000;
- uint32_t playbackTimeoutStart = 0;
+ uint32_t playbackTimeoutStart = millis();
AudioPlayer_CurrentVolume = AudioPlayer_GetInitVolume();
audio->setPinout(I2S_BCLK, I2S_LRC, I2S_DOUT);
@@ -404,12 +404,19 @@ void AudioPlayer_Task(void *parameter) {
trackQStatus = xQueueReceive(gTrackQueue, &gPlayProperties.playlist, 0);
if (trackQStatus == pdPASS || gPlayProperties.trackFinished || trackCommand != NO_ACTION) {
if (trackQStatus == pdPASS) {
- if (gPlayProperties.pausePlay) {
- gPlayProperties.pausePlay = false;
- }
audio->stopSong();
Log_Printf(LOGLEVEL_NOTICE, newPlaylistReceived, gPlayProperties.numberOfTracks);
Log_Printf(LOGLEVEL_DEBUG, "Free heap: %u", ESP.getFreeHeap());
+ playbackTimeoutStart = millis();
+ gPlayProperties.pausePlay = false;
+ gPlayProperties.repeatCurrentTrack = false;
+ gPlayProperties.repeatPlaylist = false;
+ gPlayProperties.sleepAfterCurrentTrack = false;
+ gPlayProperties.sleepAfterPlaylist = false;
+ gPlayProperties.saveLastPlayPosition = false;
+ gPlayProperties.playUntilTrackNumber = 0;
+ gPlayProperties.trackFinished = false;
+ gPlayProperties.playlistFinished = false;
#ifdef MQTT_ENABLE
publishMqtt(topicPlaymodeState, gPlayProperties.playMode, false);
@@ -424,7 +431,7 @@ void AudioPlayer_Task(void *parameter) {
}
if (gPlayProperties.trackFinished) {
gPlayProperties.trackFinished = false;
- if (gPlayProperties.playMode == NO_PLAYLIST) {
+ if (gPlayProperties.playMode == NO_PLAYLIST || gPlayProperties.playlist == nullptr) {
gPlayProperties.playlistFinished = true;
continue;
}
@@ -985,12 +992,6 @@ void AudioPlayer_TrackQueueDispatcher(const char *_itemToPlay, const uint32_t _l
gPlayProperties.playMode = _playMode;
gPlayProperties.numberOfTracks = strtoul(*(musicFiles - 1), NULL, 10);
// Set some default-values
- gPlayProperties.repeatCurrentTrack = false;
- gPlayProperties.repeatPlaylist = false;
- gPlayProperties.sleepAfterCurrentTrack = false;
- gPlayProperties.sleepAfterPlaylist = false;
- gPlayProperties.saveLastPlayPosition = false;
- gPlayProperties.playUntilTrackNumber = 0;
#ifdef PLAY_LAST_RFID_AFTER_REBOOT
// Store last RFID-tag to NVS
Hier der neue Patch, wichtig ist die Änderung in Zeile 427 if (gPlayProperties.playMode == NO_PLAYLIST || gPlayProperties.playlist == nullptr)
. Damit brechen wir auch ab, wenn die Playlist noch nicht „da“ ist.
Das ist nur ein schneller Bugfix, die saubere Lösung wäre alle Daten zu der neuen Playlist gleichzeitig zu übertragen, die Race Condition bleibt weiterhin bestehen (wir kleben nur ein Pflaster drüber). Alternativ können wir auch 0db1e86 reverten. Dass sollte das Problem auch wieder unter den Teppich kehren .
Das Problem wäre aktuell mit dem 3. Patch (Umstellung auf die Playlist Klasse) verschwunden, da die Playlist alle Informationen bündelt. Werde ich aber für Patch 2 vorsehen (Umstellung von Queue auf Mutex + Smart Pointer).
Sehr cool! Danke dir!
Aber dann wäre ich eher für die saubere Lösung in deinem schon bestehenden PR und für ein revert von dem Bugfix-commit im aktuellen dev-branch.
Wird sonst vermutlich auch immer schwerer das wieder zusammen zu mergen.
Dann teste ich lieber gleich auf der finalen Lösung intensiver, das schau ich mir morgen dann Mal genauer an
@laszloh Das passt & Vielen Dank für Deine Mühe (Obwohl das eigentlich gar nicht Deine Baustelle ist). Hab’s jetzt eingecheckt, hoffe ich hab jetzt nix vergessen.
Das sind genau die Probleme die für ein Refactoring der Playlist sprechen. Aber Plan ist jetzt mal erst die Verbesserungen aus dem DEV-Branch in den Master zu übernehmen & dann die neue Playlist im DEV weiter zu entwickeln.
Wie weit ist den der Pull Request Playlist improvements by laszloh · Pull Request #269 · biologist79/ESPuino · GitHub ?
Ich baue auch gerade an ein paar Features und stolpere auch über die aktuelle Playlist Implementierung.
Der Umbau auf Playlist Klassen würde da einige unschöne Hacks vermeiden.
Wir hatten das Thema zurückgestellt, weil plötzlich noch verschiedene Probleme mit Audiorucklern aufgetreten sind.
Aus meiner Sicht kann das Thema angegangen werden. Was mir aber ehrlich gesagt nicht gefällt, das ist der „Trend“ im ESPuino-Code, dass immer weniger dokumentiert wird (generell; der PR hier ist jetzt für mich nur der Aufhänger, das mal zu äußern). Also wenn ich mir zB die cpp.h anschaue, dann habe ich ehrlich gesagt nicht die geringste Ahnung, was da passiert.
Als ich ESPuino damals alleine geschrieben habe, hatte zB jede (oder nahezu jede) Funktion eine Beschreibung darüber, was sie macht. Da braucht’s (für mich) jetzt auch nicht die große Doxygen-Keule, sondern ich hätte gerne, dass über jeder Methode, die mehr als ein GETer oder SETer ist, eine Kurzbeschreibung steht (vielleicht 1-3 Zeilen), in der steht, was gemacht wird.
Kommentarzeilen in Funktionen/Methoden finde ich auch gut, aber hier sehe ich eigentlich auch keinen Engpass.
Ich weiß, dass ich diesen Zielzustand so nie öffentlich formuliert habe, insofern bin ich vermutlich ein Stück weit selbst Schuld, dass es so ist, wie es ist. Aber ich formuliere hiermit, dass ich gerne mehr Doku hätte .
Jupp, die PR ist erstmal ruhend. Wir hatten uns geeinigt, dass das in 3 PR’s aufeteilt wird. Vorstellung Playlist Optimierung - #17 von laszloh
Nr 1 ist schon da, aber wir hatten, wie biologist geschrieben hat, Probleme bei der Wiedergabe und wollten keine großen Änderungen einführen, bis das Problem gelöst war. Ich halte gerade die PR up-to-date zu der dev-branch.
Bezüglich Kommentierung werde ich mich bessern, vor allem wenn es Richtung der 3. PR geht. Ich muss auch sagen, mit der Version der cpp.h bin ich auch nicht zufrieden. Ich denke, die neue cpp.h ist bezüglich Kommentierung besser (bzw, ich verlinke einfach auf die Doku der Funktion, die ich damit implementiere). Kommentierung in der ist sparse, weil der Code 1:1 copy paste aus der c++20 stdlib ist (backport von Funktionen, die ich verwende/mag, die es aber in unserer gcc nicht existiert).