Open klibertowski opened 3 years ago
Hi @klibertowski thanks for reporting!
This is an interesting situation indeed, and I wonder how other DICOMs handle this in general--my guess is that for private tags like this it would probably be easiest if they were encoded in explicit transfer syntax at the start, as I'm not sure how parsers are supposed to even read these (or parse the whole dicom correctly) in the general case in an implicit transfer syntax setting without having special lookup tables as you suggest. I'm not familiar with existing opinions on how to handle things like this or what the standard suggests for private tags in an implicit transfer syntax, so need to do some reading on that.
Thoughts on a solution
Anyhow, I think this is something we should make easy for our users, even if unconventional. Assuming there isn't another opinion on how to handle these cases in general (for which I'll need to do some more reading), I think it could be reasonable for us to introduce a new ParseOption
that can take a side input of private tag information that could be used by the Parser to parse DICOMs along the lines of what you suggest. Shouldn't be too difficult, imo. I can take a look at this, perhaps after taking a quick look around to see what context there might be on this kind of thing from the parser perspective.
Hi @suyashkumar, thanks for looking into this!
I did a bit of research and it seems pydicom has recently ran into this issue as well. Their solutions was to parse UN tags with undefined length as SQ tags. This is based on this dicom standard.
This seems like a good solution since it doesn't require the user to manually input private tags.
@klibertowski fantastic, thanks so much for looking into that! Indeed I was wondering about this, and figured there would be an opinion on it--this seems like a reasonable and at the very least consistent approach! I think we should go with that. I wonder if there will ever be UN tags that are not undefined length SQ (in which case extra info would be needed to parse them correctly--but it sounds like they would be non compliant if that was the case).
Also, if there is an example DICOM with such a tag you can find or freely contribute to this repository (e.g. has the correct licensing to allow that, and is de-identified), it could be helpful to add it to the test set. We cite all the sources and licenses for any such DICOMs as well here and here. We'd have to do this after the fix goes in, but could be handy to have when kicking the tires on a fix too, in addition to unit tests.
The cancer imaging archive (TCIA) is usually a great source of DICOMs, if you happen to have run across any from there with such an unknown tag. We can always synthetically create one too if needed. I'll probably hunt around TCIA when I get a chance sometime next week to see if I can find something!
(If you don't have something handy, no worries at all--I know it can be challenging to find such dicoms that are "contributable" licensing and de-id wise, and I'll take a closer look when I can).
I tried anonymizing the DICOM I was getting the problem on using Osirix and DicomBrowser. However, when I did this I was no longer getting the error when trying to parse it. The DICOM editing programs kept changing the VL on the tag I was getting the issue on from undefined to a set value when exporting for some reason. So it seems we might need to find another DICOM for testing or make our own as you mentioned.
I did implement a quick fix based on what we discussed and it worked when testing with the original DICOM I was having issues with. So at least we know that should fix the problem.
I had some time to spend on this and was able to get an anonymized DICOM with the correct tag info by manually editing some values. The DICOM should be safe to use without any licenses.
I could write some tests and put in a PR for this issue if you'd like?
Thanks @klibertowski! hm, I think it might be safer to not use that particular DICOM without an explicit license from the owner, even if de-identified. However, writing unit tests for the fix (and confirming it works on the dicom if you run it locally) I think would be sufficient at this time.
Thanks again!
I took a tiny stab at this earlier, but need to make some adjustments and write unit tests, which I can try to do this week, or next week if I have some downtime! You are also welcome to send a PR with a fix and unit tests if you would like and have the time, but no pressure if not (though, do let me know in advance if you're going to send something so we don't overlap).
Thanks!
Hi @suyashkumar, I'll let you send the PR whenever you have time since you started working on it. Thanks for your help on this!
hey @everyone I am facing this issue while parsing dcm to jpg
package main
import ( "fmt" "image/jpeg" "os"
"github.com/suyashkumar/dicom"
"github.com/suyashkumar/dicom/pkg/tag"
)
func main() { dataset, := dicom.ParseFile("dicom/2.dcm", nil) pixelDataElement, := dataset.FindElementByTag(tag.PixelData) pixelDataInfo := dicom.MustGetPixelDataInfo(pixelDataElement.Value) for i, fr := range pixelDataInfo.Frames { img, := fr.GetImage() f, := os.Create(fmt.Sprintf("image%d.jpg", i)) = jpeg.Encode(f, img, &jpeg.Options{Quality: 100}) _ = f.Close() } }
it is the output by running the code
I used another converter online
Hi @krishpranav are you getting the EOF error reported earlier, or just an apparently dark image? The image appearing black may be working as intended, because we do not apply any pixel value auto scaling yet: https://github.com/suyashkumar/dicom/blob/87c7d7e41fca7227af3582174c5933fb91981eb7/README.md?plain=1#L57
Also, regarding the images you posted, by posting them I assume you have the license to post those and that they are de-identified (they appear to be, but that is for you to determine as the poster--if not, please delete those comments).
If you're not getting the EOF error, and it is not working-as-intended due to no auto scaling being applied, please open another issue instead of this one. Thanks!
I'm getting an error
error reading value unexpected EOF
when attempting to parse a DICOM.After doing a bit of debugging, I determined the problem is occurring because the DICOM has an implicit transfer syntax and a tag that is outside of the DICOM standard (0029,1140 ApplicationHeaderSequence SQ). This is causing the VR to be "UN" and the tag to be read as a string with a length of 4GB, which obviously fails.
Is there a way I can add a custom tag to the DICOM dict so it is correctly identified with a VR of "SQ"? Or is there another way around this problem?
I can upload the DICOM I'm getting this error on later if that will be helpful.