Add coverimage support for ogg, vorbis, opus (and some flac) files in Web-UI [base64 codiert]

sfields · 10. März 2024 um 09:34

Bis vor kurzem wurden im Web-UI nur cover von mp3s angezeigt. @Wolle ist vor ein paar Wochen meiner Anregung nachgekommen, dies auch für andere Formate wie flac, ogg, opus bibliothekseitig zur Verfügung zu stellen. Vielen Dank für diese Arbeit!

Die Anzeige von nativen flac covern habe ich dann espuinoseitig umgesetzt, tueddy hat dies im Code übernommen und @tueddy hat selbst noch die Anzeige von m4a Covern umgesetzt.

Nun fehlt jetzt noch die Anzeige von ogg, vorbis, opus covern. Diese sind im Unterschied zu den obigen Dateiformaten base64 codiert und liegen nicht in einem Stück, sondern in mehreren Segmenten innerhalb der Datei vor (auch nicht-native flac cover haben diese Eigenschaft).

Ich habe bereits mit tueddy gesprochen, von seiner Seite aus spricht nichts gegen die Übernahme meiner Code-Änderungen (s.u.). Aber er hat mir vorgeschlagen, meinen Änderungsvorschlag zunächst hier im Forum vorzustellen. Das will ich hiermit machen.

Analog zu audio_id3image habe ich eine audio_oggimage geschrieben, in der

das base64 codierte cover chunkweise decodiert wird,
das decodierte cover im SD-Karten Ordner /.cache/file.path() gecacht wird (decodiert wird natürlich nur, falls das cover nicht bereits gecacht vorliegt)

zu 1: „Dekodieren“

die Segmente werden in Chunks der Größe 2048 (kann man beliebig wählen, muss aber base64 kompatibel sein) dekodiert. Da das Ganze auch funktionieren soll, wenn man eine chunkSize wählt, die größer als ein Segment ist und die Segmente ohnehin die base64-Blöcke oft mittendrin trennen, ist das auch kein Einzeiler. Wer das chunkweise und mit ganzen Blöcken einfacher decodieren kann, möge gern einen Vorschlag posten.
Natürlich wäre der Code viel kürzer, wenn man zunächst alle Segmente einliest und an einem Stück decodieren lässt. Das ging bei meinem Board (D32 pro LiFePO4) auch mit riesigen Covern (über 1MB) problemlos, aber ggf. nicht bei allen anderen Boards, die ESPuino unterstützt. tueddy hat mir zu einer chunkweisen Dekodierung geraten. Zeitlich dauert das chunkweise Dekodieren nur minimal länger als das Dekodieren in einem Stück, die chunkSize sollte aber mindestens 1024 Bytes sein, für noch kleinere Chunks dauert es dann doch etwas länger.
mittels
char *encodedChunk = (char *)x_malloc(chunkSize);
wird Speicher im SPIRAM allokiert, falls vorhanden, dadurch dauert das Dekodieren insgesamt etwa 10% länger, als wenn man im heap allokiert, aber ich denke das ist verkraftbar
ich habe den base64 Decoder von Base64 decode snippet in C++ - Stack Overflow (keine copyright claims gefunden, „fastest algorithm out there AFAIK“) verwendet, da dieser mehr als 10 mal schneller ist als die standard base64 decoder, die man auf dem esp32 benutzt, wie z.B. mbedtls. Dieser base64 Decoder steht derzeit einfach vor der geschriebenen audio_oggimage. Sollte man diesen noch z.B. in common.h schreiben, oder gar eine eigene .h Datei für machen?
[Zur zeitlichen Info: Das Dekodieren inkl. Abspeichern der decodierten Bilddatei dauert bei normalen covern der Größe 40-80kB um die 250ms, die anderen Decoder brauchen dann schon um die 3 Sekunden, bei größeren Bildern entsprechend länger]

zu 2: „Cachen“

zunächst ist abzuwägen, ob überhaupt gecacht werden soll.
Das Cachen hat den Vorteil, dass ein einmal decodiertes Cover bei erneutem Abspielen der zugehörigen Datei nicht nochmal decodiert werden muss, also Rechenzeit spart. Allerdings braucht es etwas Speicherplatz auf der SD-Karte und der Code ist etwas länger, insbesondere wenn man in einem eigenen Ordner /.cache die Coverbilder speichert.
dann ist abzuwägen, wo gecacht werden soll. Man könnte das decodierte Coverbild im selben Ordner speichern, entweder unter demselben Namen (+.img) oder mit demselben Namen (+.img) aber führendem Punkt „.“, sodass die Datei unsichtbar ist für den Anwender. Da in der Web-UI Dateien mit führendem Punkt jedoch angezeigt werden, und ich das für den Anwender unschön finde, wird die decodierte Datei jetzt für den Anwender ungemerkt in /.cache/file.path() gespeichert, ähnlich wie z.B. auch thumbnails auf einem PC gespeichert werden. Da der Pfad dorthin ggf. noch nicht existiert, muss er erstellt werden, einen command wie mkdir_p() scheint es leider nicht zu geben (?), daher etwas umständlicher (geht das einfacher?).

Falls ihr euch den Code anschaut, bedenkt bitte, dass ich kein Programmierer bin, sondern das Programmieren in der Schule gelernt habe.

Was haltet ihr von dem Vorschlag? An welchen Stellen gibt es Verbesserungspotential oder alternative Herangehensweisen? Gerade von der Wahl der besten Variablenarten oder Speicherverwaltung habe ich ehrlich gesagt keine Ahnung.

Leider bin ich (wie @laszloh und viele andere auch) vor kurzem auf github gesperrt worden, kann so also derzeit kein PR zur Diskussion reinstellen. Deshalb hier ein Patch mit allen Änderungen, den ihr z.B. einfach mittels
git apply <.patch file>
in den aktuellen dev applien könnt (Endung .txt vorher entfernen):
0001-Add-coverimage-support-for-ogg-vorbis-opus-files-in-.patch.txt (6,7 KB)

Vielen Dank fürs Durchlesen und Kommentieren.

JHB · 11. März 2024 um 07:17

Finde Cache gut, in den Zeiten von „fast unendlichem“ Speicherplatz…

Finde auch gut den nicht in den „eigenen“ Ordner zu speichern sondern zentral.
Doppelte Namen sollte es nicht geben da, file.path() ?!

zu mkdir hab ich auf die Schnelle nur das gefunden:

Das sieht mir fast nach einem rekursiven Aufruf auf für mkdir (bei dir)… bzw. man könnte so einen dafür bauen vielleicht…

PS: krass das der base64 Algo soviele schneller ist, weißt du zufällig warum?

freddy · 11. März 2024 um 13:36

zum thema mkdir_p: Laut doku sollte es eigentlich rekusiv fehlende Ordner erstellen, funktioniert aber nicht.
beim File.open() gibt es aber einen „geheimen“ create parameter, welcher dafür sorgt, das fehlende Ordner angelegt werden. Wird z.B. hier verwendet: ESPuino/src/Web.cpp at a2c112d7529499d86bef8d071a3e75295c16bcb0 · biologist79/ESPuino · GitHub

Grundsätzlich würde ich vorschlagen das file erstmal in ein temporäres Verzeichnis oder Datei zu schreiben und es nach Abschluss mit renname() zu verschieben/umzubennenen). Wenn etwas crasht oder der Schreibvorgang abbricht, verhinderst du damit, das du korrupte Daten im Cache Ordner hast.

Bei Verwendung eines seperaten Cache Ordners würde ich auch nur den Dateinamen verwenden /.cache/covers/abc.mp3). Ohne extra .img dran zu hängen. Mit dem Ansatz musst du dir keine Gedanken machen, was passiert wenn man z.B. eine .mp3 Datei mit einem 255 Zeichen (Maximale Dateinamen Länge) langen Namen hat und du da dann noch ein .img dran hängen möchtest.

Bei deinem chunkSize Kommentar „must be base64 compatible“ schreib direkt dran, was das bedeutet: „must be a multiple of 4“.

Der base64 decoder allkokiert für den output string nochmal Speicher auf dem heap. So wie es aktuell verwendet wird würde ich das umbauen, das er den output string in den buffer des input strings speichert, das sollte beim dekodieren entsprechend kein Problem sein.

base64 Dekoder bitte allgemeinverwendbar in eigene Dateien bzw. common.h.

Zur Performance Optimierung könnte man ggf. den verfügbaren HEAP Speicher prüfen und abhängig davon den Buffer allokieren.

Bonus Wünsche von mir: Die audioI2S cover API so umbauen, das es auch für Streams funktioniert. Aktuell werden die File Handle übergeben, was es auf lokale Dateien beschränkt. Bei einer Audio Datei, die z.B. via HTTP gestreamt wird, kann das Logo aktuell nicht extrahiert werden. Mein Vorschlag wäre es, die audioI2S API so umzubauen, das man die covers aus dem audioI2S buffer ließt und nicht direkt aus einer Datei. Das dürfte ingesammt effizienter sein, als die Daten zweimal aus der Datei lesen zu müssen.

sfields · 11. März 2024 um 20:14

Schonmal danke für die ersten Rückmeldungen.

Perfekt, habe ich so umgesetzt und funktioniert. Spart uns einiges an Code.

Gute Idee. Ist zwar nie vorgekommen, aber könnte prinzipiell passieren, wenn der ESP während des Dekodierens oder Schreiben in file rebooted. Habe ich umgesetzt, wird jetzt erst in filename.tmp geschrieben und dann am Ende in filename umbenannt. Das rename braucht nochmal 25ms, aber ich denke, das ist es wert.

Danke, hab das .img jetzt weggelassen. Jedoch würde ich die files gerne mit dem ganzen filepath in /.cache speichern. Angenommen man hat in mehreren Ordnern die Datei 01.opus (aber es sind natürlich andere Daten darin gespeichert), dann würde sonst das falsche Coverbild geladen. Oder habe ich dich falsch verstanden?

OK, angepasst.

Ohje (das übertrifft wohl meine Kenntnisse), ich habs versucht, hat aber nicht geklappt. Weiß aber auch nicht, ob ich dich richtig verstanden habe. Du willst auf die Variable decodedChunk komplett verzichten und encodedChunk mit dem Ergebnis aus dem Dekoder überschreiben? Ich habe dazu den Dekoder von const str::string auf char* umgeschrieben mit minimalen Anpassungen im Dekoder, aber bekomme dann beim Dekodieren die Fehlermeldung

assert failed: block_trim_free heap_tlsf.c:371 (block_is_free(block) && "block must be free")

Wärst du so nett und könntest den Dekoder und die zwei Zeilen in der Schleife nach deiner Vorstellung anpassen? Das wäre super!

Meinst du damit die chunkSize? Sobald die in der Größenordnung von den hier verwendeten 2048Bytes ist macht eine weitere Vergrößerung kaum einen Geschwindigkeitsvorteil aus.

Vielen Dank für die guten Hinweise.

tueddy · 11. März 2024 um 21:45

Ich habe es getestet & alle Coverbilder werden korrekt angzeigt, @sfields Vielen Dank schon mal für Deine Arbeit!

Base64-Dekodierung im selben Speicher wäre zwar fein, ich glaube aber nicht das es funktioniert, die Assertion deutet schon darauf hin. Versuch war’s wert

Eine Sicherheitsabfrage würde ich noch einbauen, hier:

		File coverFile = gFSystem.open(coverFilename, FILE_WRITE);
	    if (!coverFile) {
		  return;
	    }

coverFile könnte Null sein wenn z.B. die Audiobibliothek irgendwann die Coverbilder auch über Webstream liefert & gleichzeitig mit NO_SDCARD kompiliert wird.

Ansonsten passt es für mich.

JHB · 12. März 2024 um 07:31

Achtung, der führende Punkt für das Verstecken gilt nur für Linux/Unixoide OS, ein Windows zeigt auch Ordner mit Punkt ganz normal an…
grafik

Ich würde daher auf einen extra Ordner gehen (vlt sogar _ImageCache) durch den _ wird der als erstes ganz oben angezeigt…

sfields · 12. März 2024 um 07:53

Danke für den Hinweis. Da die Sd-Karte zu 99% im SD-Kartenleser des ESPuinos ist dachte ich eher aus der Perspektive des Benutzers des Web-UI. Und dort werden im Verzeichnisbaum Ordner mit führendem „.“ (zumindest auf root-Ebene) ausgeblendet. Aus dieser Perspektive finde ich den Ansatz auch weiterhin richtig. Natürlich können wir das aber auch so machen, wie du vorschlägst, wenn die andern das ebenfalls so sehen.

JHB · 12. März 2024 um 07:55

stimmt das hatte ich gerade nicht auf dem Schirm, wenn es da ausgeblendet wird dann gerne mit .

btw ich halte es generellt kritisch Sachen vor dem „mündigen“ Benutzer zu verstecken, aber das ist ein anderes Thema

freddy · 12. März 2024 um 09:36

Hier ist eine Variante mit Variablen input/output buffer:

static const int B64index[256] = {
    0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
    0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
    0,  0,  0,  0,  0,  0,  0,  62, 63, 62, 62, 63, 52, 53, 54, 55, 56, 57,
    58, 59, 60, 61, 0,  0,  0,  0,  0,  0,  0,  0,  1,  2,  3,  4,  5,  6,
    7,  8,  9,  10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24,
    25, 0,  0,  0,  0,  63, 0,  26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36,
    37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51};

/// @brief decodes a base64 string
/// @param input_buffer pointer to the base64 string buffer
/// @param output_buffer pointer to the output buffer (can be the same as the
/// input buffer)
/// @param input_length length of the base64 string (without \0 terminator byte)
/// @param output_size size of the output buffer
/// @return number of bytes written to the output buffer
size_t b64decode(const void *input_buffer, void *output_buffer,
                 const size_t input_length, const size_t output_size) {
    if (input_length == 0)
        return 0;

    unsigned char *p = (unsigned char *)input_buffer;
    size_t j = 0, pad1 = input_length % 4 || p[input_length - 1] == '=',
           pad2 = pad1 && (input_length % 4 > 2 || p[input_length - 2] != '=');
    const size_t last = (input_length - pad1) / 4 << 2;
    //size_t output_length = last / 4 * 3 + pad1 + pad2;
    unsigned char *str = (unsigned char *)output_buffer;

    for (size_t i = 0; i < last && j + 2 < output_size; i += 4) {
        int n = B64index[p[i]] << 18 | B64index[p[i + 1]] << 12 |
                B64index[p[i + 2]] << 6 | B64index[p[i + 3]];
        str[j++] = n >> 16;
        str[j++] = n >> 8 & 0xFF;
        str[j++] = n & 0xFF;
    }
    if (pad1 && j < output_size) {
        int n = B64index[p[last]] << 18 | B64index[p[last + 1]] << 12;
        str[j++] = n >> 16;
        if (pad2 && j < output_size) {
            n |= B64index[p[last + 2]] << 6;
            str[j++] = n >> 8 & 0xFF;
        }
    }
    
    return j;
}

Dürfte potenziell nochmal ein wennig schneller sein (kein string [] operator overhead mehr).

freddy · 12. März 2024 um 10:36

hast du richtig verstanden, den filepath würd eich (mit entsprechendem .cache prefix auch speichern um eine 1:1 Zuordnung zu gewährleisten.)

Da die audioI2S API ohnehin keine parallele Nutzung ermöglicht, kannst du als temp file immer einen festen Pfad nehmen (z.B. „/.cache/.temp“). Damit sparst du die den string concat und auch Ärger mit der Dateinamen Länge. Außerdem musst du dir keine Gedanken machen „alte“ .tmp Dateien zu löschen.

Aktuell schreibst du in deine ogg cache Dateien noch ein „fLaC“ prefix damit es dann in der Web.cpp richtig behandelt wird. Ich würde vorschlagen ein einheitliches Format für die cache Datein zu wählen damit es beim ausliefern in der Web.cpp nurnoch einen Fall gibt.
z.B. solch eine header:

uint8_t version;
uint8_t mime_type_length;
uint8_t mime_type[mime_type_length];

Grundsätzlich müsste man sich auch mal Gedanken machen was passiert wenn eine Audio Datei mehrere Bilder beinhaltet. Ggf. sollte man direkt einen image_type mit in den Header aufnehmen.

freddy · 12. März 2024 um 10:47

Um sich einen custom header, etc. sparen zu können und mehrere Dateien zu unterstützen macht es ggf. Sinn anstelle das Bild in einer gleichnahmigen Datei zu speichern, diese einfach in einem Ordner zu speichern:

/.cache/path/to/example.ogg/0 - CoverFront.jpg
/.cache/path/to/example.ogg/1 - Coverback.png

dateiname = nummer und ggf. image type
mime-type entsprechend über die Datei Erweiterung.

Nachteil dieser Lösung ist sicher das zusätzliche Iterieren über die Dateien im Ordner. Müsste man sich mal angucken wieviel mehr Aufwand das ist.
Vorteil: einfach und maximale Kompatibilität der metadaten.
ggf. könnte man dann auch eine /.cache/path/to/example.ogg/metadata.json einführen, in der sonstige metadaten (titel, artist, etc.) gecacht werden können.

sfields · 12. März 2024 um 15:16

Vielen Dank für alle Rückmeldungen. Ich habe fast alle Vorschläge umgesetzt.

Hier ist der aktuelle Stand als Patch (mein github ist noch gesperrt):
0001-Add-coverimage-support-for-ogg-vorbis-opus-files-in-.patch.txt (7,3 KB)

Ein paar Anmerkungen:

Zum umgeschriebenen Decoder:
Habe hier das Argument output_size entfernt weil ich nicht wusste, was ich hier als Größe übergeben soll, denn die Größe wird doch (wenn überhaupt) erst im Dekoder berechnet. output_size zu übergeben macht doch nur Sinn, wenn ich den Chunk nicht vollständig dekodieren will, aber der soll doch vollständig dekodiert werden. Entsprechend habe ich auch die drei Bedingungen mit output_size aus dem Dekoder entfernt. Wahrscheinlich habe ich es einfach nicht verstanden?
Ansonsten wird hoffentlich alles so gemacht wie gewünscht, der dekodierte Chunk wird jetzt in den encodiertenChunk geschrieben.
Ich habe dann als temporären CoverFilename doch
String tmpCoverFilename = coverFilename.substring(0, coverFilename.lastIndexOf('/') + 1) + ".tmp";
gewählt, da
String tmpCoverFilename = "/.cache/.tmp";
den Nachteil hat, dass dann das rename nur geht, wenn die Ordnerstruktur bereits existiert, sowas wie
gFSystem.rename(tmpCoverFilename, coverFilename, true);
gibt es nicht. Alternative wäre statt einem einfachen rename nach dem Codieren zusätzlich:

File coverFile = gFSystem.open(CoverFilename, FILE_WRITE, true); 
coverFile.close();
gFSystem.rename(tmpCoverFilename, coverFilename);

Oder siehst du eine bessere Lösung?

Was ich von den Vorschlägen nicht umgesetzt habe:

Zur Performance Optimierung könnte man ggf. den verfügbaren HEAP Speicher prüfen und abhängig davon den Buffer allokieren.
Bonus Wünsche von @freddy : Die audioI2S cover API so umbauen, das es auch für Streams funktioniert. Aktuell werden die File Handle übergeben, was es auf lokale Dateien beschränkt. Bei einer Audio Datei, die z.B. via HTTP gestreamt wird, kann das Logo aktuell nicht extrahiert werden. Mein Vorschlag wäre es, die audioI2S API so umzubauen, das man die covers aus dem audioI2S buffer ließt und nicht direkt aus einer Datei. Das dürfte ingesammt effizienter sein, als die Daten zweimal aus der Datei lesen zu müssen. Das geht wohl an @Wolle ?
das Schreiben eines anderen Headers und das alternative Abspeichern, wie im vorherigen Beitrag vorgeschlagen

Was vor einem Übernehmen des Codes noch zu tun wäre (aus meiner Sicht):

dass jemand nochmal schaut, ob ich immer die richtige Variablentypen genommen habe, so verwende ich in der Dekodierschleife immer int, oft verwenden wir aber uint32_t und andere Arten
ist das mit dem reinterpret_cast<const uint8_t*> nach Anpassung noch richtig und wichtig?

freddy · 12. März 2024 um 16:57

output_size ist die Größe des output buffers, die checks sorgen dafür das man nicht darüber hinaus schreibt.
In deinem Fall müsstest du chunkSize übergeben.

Es sollte möglich sein, deinen for (int chunk = 0; chunk < numChunks; chunk++) loop so umzubauen, dass der auch das lesen des Rests (code nach dem loop) mit erledigt (Aktuell hast du da sehr ähnlichen Code doppelt).

String coverFilename = "/.cache" + String(file.path());

baust du so um (Spart einen String malloc)

String coverFilename = "/.cache";
coverFilename.concat(file.path());

In der Web.cpp machst du es analog dazu.

Aktuell setzt du gPlayProperties.coverFilePos = 1.
Das war für mich erstmal nicht nachvollziehbar (hätte 4 erwartet um den „fLaC“ Marker zu skippen).
Habs mir aber nun genauer angeguckt. flac und ogg nutzen beide dieses biary format für bilder: FLAC - Format
Bei flac Datein bekommen wir von der audioI2S aber ein pointer zu einem 3 byte längen Feld vor dieser Datenstruktur.
Die Routine in der Web.cpp geht entsprechend von einem 3 byte längen Feld am Anfang aus. Damit das dort trotzdem funktioniert muss man coverFilePos entsprechend auf 4-3=1 setzen.

Wie gesagt, ich find das Speichern von pseudo flac Dateien noch ein wenig Problematisch. Da müssten aber denke ich mindestens noch Kommentare rein, die den Workaround/hack erklären. Da haben aber natürlich @tueddy bzw. @biologist das letzte Wort.

freddy · 12. März 2024 um 17:33

Alle int’s die du hast würde ich zu size_t machen.

reinterpret_cast sollte man als letztes nutzen. Bei dir wäre ein static_cast ausreichend.
Wenn du dir unsicher bist am besten ganz vermeiden:
Den flac Marker kannst du auch so schreiben:

constexpr uint8_t flacMarker[] = "fLaC";
coverFile.write(flacMarker, std::char_traits<uint8_t>::length(flacMarker));

Wenn du dein encodedChunk direkt als uint8 deklarierst sparst du dir auch die anderen casts:

uint8_t *encodedChunk = (uint8_t *) x_malloc(2048);

Habe gesehen das du die b64decode mit einem inline versehen hast, bringt das einen nennenswerten Performance Vorteil? Ansonsten würde ich die an der Stelle nicht Zwangs-Inlinen.

sfields · 12. März 2024 um 19:57

Danke für deine Antwort.

ok, ist die input_size aber nicht immer größer als die output_size bei der base64 Dekodierung? Von daher sollte das doch automatisch immer passen, wenn wir den dekodierten Chunk in encodedChunk schreiben, so wie wir es tun. Wenn man auf die (daher in meinen Augen überflüssigen) Checks in jedem Schleifendurchlauf verzichtet spart man etwas Rechenzeit. Oder ich verstehe es noch nicht.

Danke für den Hinweis, habe ich gemacht.

Hab noch einen Kommentar zur Erklärung ergänzt.

ok, gemacht.

Danke, gemacht.

Wenn ich reinterpret_cast durch static_cast ersetze bekomme ich den Fehler invalid static_cast from type 'char*' to type 'const uint8_t*' {aka 'const unsigned char*'}

Dann habe ich deinen anderen Vorschlag versucht:

Wenn ich das so mache, bekomme ich in der folgenden Zeile einen Fehler

file.readBytes(&encodedChunk[remainder], chunkSize - remainder);

(invalid conversion from 'uint8_t*' {aka 'unsigned char*'} to 'char*' [-fpermissive]) den ich z.B. beheben kann mit

file.readBytes(reinterpret_cast<char*>(&encodedChunk[remainder]), chunkSize - remainder);

Aber ist dadurch wirklich was gewonnen?

Ich bekam sonst den Fehler: multiple definition of b64decode. Ich wusste nicht wie ich den Fehler ohne inline lösen kann?

freddy · 13. März 2024 um 09:19

In deinem Fall ist das korrekt, der b64decoder ist aber allgemein gehalten, der output buffer wird als extra Argument angegeben und muss nicht zwingend der input buffer sein. Entweder müsste man jetzt die maximal performance optimierte Version mit nur einem input/output buffer als Parameter bauen oder die minimal langsammere allgemeine Version mit output buffer size checks verwenden.

In die Common.h baust du nur die deklaration ein:

size_t b64decode(const void *input_buffer, void *output_buffer, const size_t input_length);

Must dann eine Common.cpp anlegen mit einem #include <Common.h> und dem code drin. Vermutlich dürften dann beim Kompilieren die fehlenden includes in der Common.h auffallen, dann müsste ein #include <Arduino.h> ausreichend sein.

Laut Arduino Doku sollte readBytes() auch byte/uint8_t akzeptieren, tut es aber nicht… Da kommst du dann um einen reinterpret_cast nicht drumherum.

Würde encodedChunk aber als uint8_t deklariert lassen, für binär Daten ist uint8_t mit den wenigsten Überraschungen verbunden. Kannst es auch als Arduino byte Typ deklarieren (Alias für uint8_t). Um den cast bei readBytes kommst du aber leider nicht drumherum.

sfields · 14. März 2024 um 20:51

Ok, habe ich gemacht. Sollte man dann (ganz unabhängig hiervon) nicht auch die ganzen alten inline Funktionen aus der Common.h ohne inline nach Common.cpp schieben?

Ok, habe den encodedChunk jetzt statt char als uint8_t deklariert, und entsprechend in file.readBytes() einen reinterpret_cast<char*> hinzugefügt.

Ich denke von meiner Seite aus kann das jetzt so bleiben und im Idealfall übernommen werden. Ich bin ohnehin mit meinem Latein (oder besser gesagt mit meinen Programmierkenntnissen) am Ende

Ich bedanke mich nochmals für die Arbeit von @Wolle und die Hilfe von @tueddy und @freddy.

Hier mein finaler Vorschlag:
0001-Add-coverimage-support-for-ogg-vorbis-opus-files-in-.patch.txt (8,3 KB)

tueddy · 14. März 2024 um 21:40

Ich habe den aktuellen Stand nochmal getestet & es sieht von meiner Seite Alles gut aus. Code & Style passt, die Base64-Coverbilder werden richtig und auch recht schnell angezeigt, werde es so übernehmen.

@sfields Danke für Deine Arbeit & auch @freddy für die Review