farvardin / 8bit_music

C64 musics, made in BASIC or other tools
21 stars 2 forks source link

Erreurs dans la boucle de lecture des données du tableau sidData #2

Closed auraes closed 5 years ago

auraes commented 9 years ago

La boucle sidPointer fait un tour de trop et la boucle sidRegister ne s'interrompt pas si l'on a atteint la fin du tableau. Proposition :

define NB_SID_REGISTER 25

void loop() { uint8_t sidRegister = 0; uint16_t sidPointer; for(sidPointer = 0; sidPointer < sidLength; sidPointer++) { mySid.set_register(sidRegister, (pgm_read_byte(&sidData[sidPointer]))); sidRegister++; if (sidRegister >= NB_SID_REGISTER) sidRegister = 0;
//digitalWrite(LED,HIGH); delay(19); //digitalWrite(LED,LOW); }; } update : J'avais sorti la déclaration des variables de la fonction ! Corrigé.

farvardin commented 9 years ago

ok mais je ne vois pas ce que ça change dans le résultat final. Si c'est pour optimiser, ok, mais de toute façon l'arduino ne peut faire qu'une seule chose, et imparfaitement vu qu'un morceau complet ne peut pas ^etre diffusé, pour cela il faudrait pouvoir lire depuis une carte sd, et je ne suis pas capable de programmer cela.

auraes commented 9 years ago

Bon ! Non, ce n'est pas pour optimiser ; le code ne fait simplement pas ce qu'il est censé faire. Je ne connais rien à l'Arduino et ses limites, mais tu peux très bien rajouter un #define ARDUINO_MAX de n, et le mettre dans ta première boucle : for(uint16_t sidPointer = 0; sidPointer < ARDUINO_MAX ; sidPointer++)

Bien, j’en reste là.

farvardin commented 9 years ago

le code est censé envoyer de la musique sous forme de dump de registres sid dans un émulateur arduino de processeur sonore MOS sid. Je ne sais pas si tu as testé ton code, mais ça ne fonctionne pas comme prévu. Ce n'est pas inintéressant comme effet, mais on a l'impression que la musique est jouée 10 fois plus lentement qu'attendue.

auraes commented 9 years ago

Je n'ai pas d'Arduino et j'ai testé mon code avec du C ; comment pourrais-je savoir ce qui est prévu ! Mon code devrait faire la même chose que ton code mais, normalement, sans les bogues — je retesterai. As-tu essayé ta routine récemment ? Il y a un delay(19) dans ton code original, donc je suppose une temporisation. Et le nombre de registres, est-ce 24 ou 25 ?

update : Après vérification de mon code, il fait la même chose que le tient sans les débordements.

farvardin commented 9 years ago

peut-être que ces débordements sont nécessaires pour le bon fonctionnement du code, à mon avis celui qui l'a écrit savait ce qu'il faisait : http://www.lemon64.com/forum/viewtopic.php?p=656715#656715

farvardin commented 9 years ago

Sinon le MOS SID 6581 a 29 registres : http://www.waitingforfriday.com/index.php/Commodore_SID_6581_Datasheet

auraes commented 9 years ago

Trouve moi un seul programmeur qui ne tombe pas de sa chaise en voyant ce genre de chose : for(uint16_t sidPointer=0;sidPointer<=sidLength;sidPointer++) { [...] sidPointer=sidPointer+24; }; Le débordement est ce qu'il y a de pire en programmation, que ce soit en lecture ou en écriture. Celui qui a fait ça, ne savait pas ce qu'il faisait (je comprends mieux le char à la place de l’unsigned char, la représentation des données en décimal dans le tableau, la structure des boucles illogiques, les débordements, etc.) Mais si cela te convient, clos ou efface la présente discussion. Je m’étonne simplement que tu rendes publique du code copier/coller non vérifié ; il y en a bien assez !

Pour les registres, il faut trouver combien sont concernés. update : Il y a bien 29 registre [0,28] ; les 25 premiers [0,24] sont write-only et les 4 derniers [25,28] sont read-only. Donc il y a bien 25 registres dans lesquels il est possible d'écrire (cela donne une image très belle : http://auraes.free.fr/tmp/600px-SID_Registers.PNG ; ça me donne une idée... )

Cordialement

farvardin commented 9 years ago

Désolé, je ne suis pas programmeur de formation, et je ne comprends rien à ce que tu racontes.

Déjà dans la structure des données, il est correct d'utiliser le type "char" et non pas "unsigned char" parce que les valeurs s'étendent de -128 à 128, et non pas de 0 à 255, cf. https://www.arduino.cc/en/Reference/Char D'ailleurs en Arduino l'unsigned char s'appelle en fait "byte" : https://www.arduino.cc/en/Reference/UnsignedChar "For consistency of Arduino programming style, the byte data type is to be preferred. ". Mais ici de toute façon cela ne s'applique pas puisque il faut pouvoir avoir des valeurs négatives.

Pour la valeur de sidPointer c'est un uint16_t donc la valeur maximale c'est 65535. Et ce sidPointer est limité au maximum à la valeur de sidLength, qui est du INT (max 32 767), donc 32 767 + 24 sera toujours inférieur à 65535, ainsi on voit qu'il n'y a pas de risque de dépassement de tampon.

auraes commented 9 years ago

Tu ne comprends pas que si tu as 20 pommes sur l'arbre, tu ne peux/dois pas en prendre 24 dans ton panier ; je ne te parle pas de la taille de ton panier !

Ta base de données est représentée en décimal, ce qui t’induit en erreur. J’ai pris la peine de t’en faire une représentation hexadécimal (test éventuellement avec le fichier hexa.)

-13 peut s’écrire : -13 décimal : 11110011 en binaire : F3 en hexadécimal

Ce qui importe ici, c’est le motif binaire du nombre : le bit 0 de ta donnée va être envoyé au registre R0 du MOS SID 6581, le bit 1 au registre R1, etc. ET LE BYTE 7 au registre D7 (regarde l’image). Ce bit 7 n’est en aucun cas un bit de signe !!! C’est pourquoi la base de donnée doit être typée en unsigned char et non en char. Mais quelque soit la représentation décimal, hexadécimal ou binaire, le motif binaire reste le même : c’est pour cela que ça fonctionne quand même.

Je ne suis pas programmeur non plus , et alors !

farvardin commented 9 years ago

En fait l'outil d'exportation des fichiers psid génère des valeurs de type char, il faut donc travailler avec ce type, la finalité c'est de produire des sons, pas d'avoir une pureté de code ou de se prendre la tête à passer par plusieurs convertisseurs. Si l'outil avait généré du format hexadécimal il aurait fallu mettre en type en accord avec ça, évidemment.