[Dcmlib] gdcm proposal (2)

[Benoit Regrain]

2eme partie (sinon ca passait pas...)


>> --------------------------------------------------------------------------
>> Maintenant voici mes remarques :
>> - le File (dérivé du Document) fait déjà une bonne partie de ce que tu
>> veux faire. Je pense qu'en effet, il est bien de garder cette structure qui
>> convient parfaitement.
> 
> c'est le terme parfaitement qui me fais sursaute. Aller je me lance dans 
> une autre de mes idee ;-P:
> Un truc que j'adore en c++ c'est le polymorphisme, tu peux par exemple 
> manipler une classe via des fonction virtuelles sans te rendre compte 
> que tu manipule un instance d'une sous classes...ainsi j'aimerais que 
> gdcm.File soit en fait:
> 
>                       gdcm.Writer
>              |           |             |             |
>      gdcm.DICOMV3  gdcm.Libido   (gdcm.ACR-NEMA) (gdcm.Papyrus)
> 
> je veux pas d'une classes fourre tout ou tu fais du code pour ACRNEMA du 
> DICOMV3 et du papyrus justeaucasou ... c'est horrible !
Voila une bonne idée toute belle qui me plait bien... 
Mais la question etait sur la donnée Document.



 
>> - le DicomDir (dérivé de Document), tu n'en parles pas. Etant donné
>> qu'il n'a pas besoin de la complexité interne du Document, garde-t-on
>> cette dérivation ? (elle est à l'heure actuelle utile car le Document 
>> contient
>> toute la partie Reader/Writer, mais je pense qu'on peut s'en abstraire
>> pour la structure que tu proposes).
> 
> 
> C'est la ou le bas blesse, je n'ai pas du tout de connaissance sur le 
> sujet. Est-ce que ca peut rentrer dans mon archi:
> 
>                         gdcm.Writer
>                             |
>                        gdcm.DICOMDIR
Oui, ca y rentre, car le DicomDir est ecrit comme les autres. 
Mais ma question portait plutot sur la structure interne du DicomDir au
sens Data (la sortie du Writer)



 
>> - Dans la création d'un Element du Document... comment on
>> différencie les ValEntry, BinEntry et SeqEntry ? A l'heure actuelle,
>> on fait cette différenciation à la création de l'élément en fonction de 
>> son VR.
>> De plus, un Element (ou DocEntry à l'heure actuelle) est rataché à un 
>> DictEntry.
> 
> 
> Faux -IMHO- c'est le gdcm.Document qui a connaissance du dict. Mais un 
> element est rattache a un Document.
> D'ailleurs au passage je ne comprends que deux type d'el un BinEntry et 
> un ValEntry. Pour moi un SeqEntry c'est un Document ... je sais pas bien 
> encore comment representer ca.
Un SeqEntry contient des SQItem qui sont eux des pseudo Document...
En fait, dans la structure actuelle, un SQItem et un Document derivent de 
ElementSet. Deux points les différencie :
 - dans le Document, on stoque les entry dans une map (pour des questions
de rapidité) et dans les SQItem on y stoque dans une liste (car moins gourmande
en place).
 - dans un Document (ou plutot un File), on a des méthodes supplémentaires
pour acceder aux caractéristiques du Document.



> Au passage, pourquoi la distinction BinEntry / ValEntry ? Pour moi tu 
> lis un petit bout de fichier tu stocke ca dans un ostringstream et 
> basta. Un ValEntry c'est juste que tu 'tautorize a interpreter le flux 
> binarie en chaine de caractere... 
La distinction BinEntry / ValEntry se fait en fonction de la VR. Pour un BinEntry,
par exemple les pixels de l'image ou d'autres données, y stoquer dans une stream
n'aurait pas vraiment de sens... et quel en serait reelement l'intérêt ?

> d'ailleurs :
> QUESTION: comment on sait qu'un element est une ValEntry dans le cas 
> d'un shadow dict. Est-ce qu'on ne pourrait pas tout lire en BinEntry et 
> unquiquement lorsqu'on sait ce qu'on fais on dynamic_cast en ValEntry -> 
> interprete la chaine de caractere et la on peut effectivement essayer de 
> lire par exemple le spacing.
> 
> Tout ca pour dire je voudrais pouvoir lire un DICOM plus vite, il y a 
> beaucoup de temps passer a je lis un petit bout de DICOM, a tiens c'est 
> un ValEntry, je fais un new ValEntry, je copie les valeurs, je passe au 
> suivant tiens c'est un BinEntry je fais new ValEntry ... c'est vraiment 
> long et je me rend pas bien compte de ce que ca apporte.
Moui, c'est une excuse tout à fait valable... A étudier. Mais ca ne resoudrait pas 
le problème des SeqEntry. Il faudra de toute manière faire le travail pour lui.
Par contre, il est clair que la transformation en chaine du ValEntry est un peu
lourde. Je pense qu'il manquerait meme une classe qui genere les NumberEntry
Ainsi on supprimerait les parsing de chaine de caractères pour au final obtenir les
nombres. Les transformation number->string et string->number sont a mon avis
plutot couteuses en temps.

On pourrait avoir une structure de DocEntry comme suit :
DocEntry (existe deja)
 - SeqEntry (existe deja)
 - ContentEntry (existe deja)
    - ValEtnry (pour tout ce qui est chaine)
    - BinEntry (pour tout ce qui est donnée non interpretable)
    - NumberEntry (pour tout ce qui est nombre)
mais ca ne résout pas le problème de passage d'un BinEntry lorsqu'on a un ShadowDict
et qu'on est en ImplicitVR (car pour le ExplicitVR, on n'a pas de problèmes).
Mais avec les ShadowDict, il reste le problème du SeqEntry... car il se peut qu'on ne
sache pas qu'on a une SeqEntry à un moment (car son groupe est privé)... ce qui 
d'ailleurs m'interpelle un peu, car l'élément suivant d'un SeqEntry serait un SQItem... et 
on devrait pouvoir le repérer lui... na ? JPR, toi qui connait tout de Dicom, qu'en penses tu ?

 
>> Tu fais ce rattachement à quel moment ? De plus lorsque le dictionnaire 
>> publique
>> ne contient pas le tuple (group, element, VR) il en crée un nouveau 
>> sauvegardé
>> dans une zone tampon... garde-t-on cela qui n'est pas tres propre ? ou 
>> place-t-on
>> le VR de l'Element dans l'Element ? 
> 
> Pourquoi exactement on a besoin d'en creer un nouveau ? Un element est 
> rattacher a un gdcm.Document et le gdcm.Document peut tout moment 
> changer de dictionaire (Siemens, Philips, ...). Donc la VR c'est jamais 
> stocker dans l'element, non ?
En ExplicitVR, on peut avoir des Entry dont la VR diffère de la VR du dictonnaire.
C'est dans la norme Dicom ca (enfin a verifer avec JPR)

 
> Dans mon idee c'est le gdcm.Document qui a l'ensemble des dictionaires 
> autorises. Donc a partir d'un element+group il doit etre capable de 
> renvoyer toutes les informations necessaires correct ? Et comme c'est 
> une std::map l'access est en O(1). Le seul probleme dans mon approche 
> c'est qu'un group+element n'est pas unique, par ex qu'il soit present 
> dans le dict public et le dict prive. Mais  meme dans ce cas je vois pas 
> comment vous pourriez le gerer.
Mais cela va ralentir lors de la recherche du nom appartenant une Entry...
certes le parsing sera plus rapide, les Entry plus petits. C'est à discuter.
Pour moi je laisse le sujet en suspend pour l'instant.

 
>>  Ce qui nous donnerait :
>> 
>> newel = gdcm.Element("OB")    // La VR est affecté au moment de la 
>> création de l'objet
> pourquoi on a besoin de la VR ?
Savoir si on est en BinEntry, ValEntry, NumberEntry, SeqEntry

 
>> newel.Set(0x1234,0x5678)       // On va rechercher le DictEntry 
>> correspondant... mais dans quel Dict ?
> Ceux attacher au gdcm.Document...
Mais comment savoir lors de la lecture d'un fichier à quel constructeur il
appartient pour ensuite prendre le bon ShadowDict...

 
>> newel.SetValue("foobar")          // Et pour une valeur binaire ou une 
>> séquence, que fait-on ?
> J'aime pas la notion de sequence ca reste pour moi qu'un gdcm.Document. 
> Mais sinon "foobar" est une valeur valide pour une entree binaire, non ? 
> Ou est la difference entre BinEntry et ValEntry que je ne vois toujours 
> pas ?
une BinEntry, c'est une VR = 'OB' ou 'OW'
une SeqEntry, c'est une VR = 'SQ'
une ValEntry, c'est tout le reste
(sauf erreur de ma part)

 
>> Mais ceci ne résoudrait pas le problème de création de l'élément en 
>> fonction de sa VR.
>> Car avec ce choix, l'utilisateur peut créer un Element de VR "SQ" et 
>> mettre des données
>> textuelles dedans. Une solution serait de mettre le constructeur en 
>> protected
>> et d'avoir un New avec une création de l'objet en fonction de la VR...
> 
>> - Concernant l'écriture des series, tu considères avoir en entrée une 
>> liste de Document
>> ou un seul Document contenant une image 3D ? Pour moi, la liste de Document
>> et nettement plus adaptée car d'un Document à l'autre, de nombreux 
>> champs de
>> l'entete peuvent varier.
> Le probleme est parfaitement souligner: je ne sais pas ce que j'ai en 
> entree ! Si je recois une image 4D, je eux pouvoir l'ecrire en serie 
> d'image 3D temporelle OU une serie 2D variant en Z direction et en temps 
> ! et je vois pas ou est le probleme d'archi la dedans.
> Il faut garder en tete que c'est une archie que je propose, si le 
> programmeur ne sait pas faire il met un 'abort()' dans le code et il 
> attends qu'un utilisateur lui envoi le patch pour pouvoir le faire. Je 
> ne veux pas avoir de limitation d'entree de jeux.
> 
>> - Une question qui ne peut se déduire du code python : la création des 
>> objets
>> se ferait par quelle méthode ? une méthode New comme en VTK, ou un new
>> habituel comme on fait actuellement ?
> 
> Reponse: new. le ::New VTK c'est pour des compilos que gdcm ne 
> supporte(ra) pas, faut pas deconner qd meme !
Le New/Delete de VTK sert aussi a proteger l'accès au constructeur et destructeur
car ce dernier utilise le reference counting... ca n'a rien a voir avec des compilos !

 
>> - Concernant les Series, tu fais un SerieReader... qui remplacerait 
>> surement
>> le SerieHelper actuel... je ne suis pas sur qu'il s'agisse d'une bonne 
>> solution
>> (d'après ce que j'ai compris de l'intérêt du SerieHelper). Le 
>> SerieHelper a pour
>> but d'ordonner les différents fichiers d'une meme série. Or 
>> l'utilisateur pourrait
>> très bien vouloir lire ses différents fichiers d'une part, puis les 
>> trier... or s'il passe
>> dans un SerieReader, il va de nouveau relire ces fichiers. Il serait 
>> mieux d'avoir
>> un SerieOrder qui prend en entrée une liste de Document (donc les 
>> fichiers sont
>> déjà lus) et ce process ne ferai qu'ordonner cette liste (suivant les 
>> critères actuels
>> du SerieHelper).
> 
> Parfaitement raisonne ! Et le SerieOrder est utiliser soit lors de la 
> lecture soit lors de l'ecriture (reutilisation de code plutot que 
> duplication).
Au moins on est d'accord sur un truc...


 
>> - Pour la classe Image, je n'ai pas très bien compris comment elle 
>> fonctionne.
>> A-t-elle une sortie ? et quel type de sortie ? ... simplement un tableau 
>> de données comme
>> le fait le FileHelper actuel lorsque tu appelle la méthode GetImage ?
>> Et lorsque l'utilisateur demande un sous-volume englobant le volume 
>> précédement lu,
>> réutilise-t-on le volume précédemment lu ?
>> 
>> image = gdcm.Image()
>> image.SetInput( reader.GetOutput())
>> image.SetSubVolume(512,512,1,3)  # SetVOI / SetExtent
>> image.Construct()
>> image.SetSubVolume(512,512,0,10)  # SetVOI / SetExtent englobant le VOI 
>> precedent.
>> image.Construct()
> 
> Ok je reconnais le nom gdcm.Image n'est pas correct. Que pense tu de 
> gdcm.Decompress. Dans ce cas gdcm.Decompress repart de zero a chaque 
> fois: relecture du fichier / seek / contruire une nouvelle image.
> C'est sans doute dommage niveau efficacite mais pour moi je vise le cas 
> du thesard qui fais sa registration et il sait quel sous volume 
> utiliser. Dans son rep il a une image de 3 Gig il sait qu'il doit 
> recaller la partie 0-100 x 0-100 x 0-100 s'il se trompe ben il part du 
> principe que ca va prendre un peu de temps a relire la bonne partie du 
> fichier.
Ca me convient. Decompress, ca serait plutot le traitement de décompression.
Mais la donnée en sortie de ce traitement ? Image ou Document avec quelque chose
en + ?


 
>> - Pour l'ecriture des images créées from scratch. Comment cela se 
>> passerait-il ?
>> Tu as dit que l'Image replacerait les champs du Document, ce qui veut 
>> dire qu'il
>> faut passer à la fois une Image et un Document au Writer ? Sur ce point, 
>> je crois qu'il
>> manque un exemple d'utilisation. J'ai aussi beaucoup de mal à voir ce 
>> que tu appelles
>> Image...
> Est-qu'on peut faire un gdcm.Document contient un gdcm.ElementSet (le 
> header) et contiendrait aussi un gdcm.Image. Donc dans le cas d'un 
> fichier DICOM raw si on force la lecture meme des element > 4096 en 
> taille et que l'utilisateur demande une gdcm.Image de la totalite alors 
> oui il aura deux fois l'image en memoire. Et non on ne pourra pas etre 
> malin en passant le meme pointeur sur l'image.
Et cela repondrait a ma question precedente sur la sortie du Decompress


 
>> Voila, les remarques sont finies. Certaines rentrent déjà dans 
>> l'implémentation, j'espere
>> donc que je ne les pose pas trop tot. Je crois sinon que j'ai fait le 
>> tour du sujet.
>> Sinon, moi aussi je ne vais pas avoir beaucoup de temps à investir dans 
>> le développement
>> de cette nouvelle mouture de gdcm... mais ce sera un problème à voir 
>> lorsqu'on aura
>> défini la structure finale de gdcm !
>> Je pense que cette fois ci, il ne faut pas commencer à développer avant 
>> d'avoir la structure
>> finale de la librairie et d'avoir vérifié que les exemples créés 
>> conviennent à tout ce
>> dont on a besoin.
> 
> Si on fait le tour des cas d'utilisation on ne peut rien manquer par 
> definition. C"est pour ca qu'il faut passer pas mal de temps a reflechir 
> a tout les cas d'utilisation. De mon experience j'ai eu pas mal de 
> feedback depuis ITK, vous de votre cote vous devrier avoir du feedback 
> de CREATIS directement.
Euhhh... a creatis, gdcm n'est pas trop utilisé directement. Les utilisateurs
sont soit resté sur Libido (tres ancienne version de gdcm dont ils se contentent
car ils veulent po passer a mieux...) soit en utilisant DaVaW qui vient tout juste
de passer à la nouvelle version de gdcm :-)... c'etait pas trop tot.
Le seul non informaticien de creatis a utiliser gdcm (sauf erreur de ma part) est
Edouardo avec Maracas. Et les plus gros problèmes qu'on a eu, c'est :
 - creer un DicomDir à partir d'une liste d'images deja lus (d'ou les convertisseurs
   Document2DicomDirImage... meme si c'est encore a revoir ca...)
 - les SerieHelper qui relisent obligatoirement les fichiers (ce qui serait resolu par 
   le SerieOrder)

 
>> Désolé pour la longueur du mail. J'ai essayé d'etre le plus constructif 
>> possible, et
>> ca prend de la place. J'espere que vous serez arrivé jusqu'ici
> J'aime les longues histoires avec plein de rebondissements :)
Tu vas etre content, j'en ai fait plein.



>> PS : Mathieu, tes exemples sont une très bonne idée. Je pense qu'il faut 
>> continuer à les
>> faire et les compléter au fur et à mesure... afin d'avoir l'utilisation 
>> complète et finale
>> de la librairie.
> Merci infiniment et merci a toi d'avoir pris le temps de me repondre, 
> c'est extremement apprecie (si si !).
C'est la partie du travail que je prefere dans la programmation :-)

Benoit
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.creatis.insa-lyon.fr/pipermail/dcmlib/attachments/20050428/51fe3774/attachment.html>