Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JPEG Lossless: Error parsing encapsulated pixel data - "unsupported JPEG feature: unknown marker" #85

Open
bbeckrest opened this issue Jun 22, 2020 · 9 comments
Labels
help wanted Extra attention is needed

Comments

@bbeckrest
Copy link

Hi Suyash,

First off, thank you for your efforts supporting/maintaining this library. I really appreciate your desire to guide future development based on user feedback.

I'm running into an issue while trying to parse encapsulated pixel data. Unfortunately, the following error is being returned:

unsupported JPEG feature: unknown marker

Here's a snippet of code I'm trying:

// NumCols returns number of pixel columns in slice image
// Tag (0028,0011) specifies this value, but is invalid for encapsulated images
func (slice *Slice) NumCols() (int, error) {
	var nCols int

        // slice.pixelData is instance of *element.PixelDataInfo
	if slice.pixelData.IsEncapsulated {
		img, err := slice.pixelData.Frames[0].GetImage() // <- this returns error
		if err != nil {
			return 0, err
		}

		imgRect := img.Bounds()
		nCols = imgRect.Dx()
	} else {
		nCols = slice.pixelData.Frames[0].NativeData.Cols
	}

	slice.imageInfo.nCols = &nCols

	return nCols, nil
}

Here's an example of failing DICOM images:
dicom.zip

I don't believe this is an issue with the dicoms themselves because I am able to view them with another dicom viewer (OsiriX). Total guess but I'm thinking it might have something to do with how the PixelData element is being parsed by go-dicom... here's a screenshot of the PixelData element value from a dicom linked above, as displayed by OsiriX:

Screen Shot 2020-06-22 at 1 56 35 PM

You'll notice that the PixelData element contains two values, with the first value empty and the second value containing the actual byte data. Conversely, other (albeit, non-compressed) dicoms I've successfully parsed contain only one value within the PixelData element.

Alternatively, I could just be using go-dicom incorrectly to parse encapsulated data.

Thanks in advance for your help.

@suyashkumar
Copy link
Owner

Hi there @bbeckrest! I can take a deeper look later this weekend if needed, but one thing that may be worth trying is index 1 for the frame, which should index into the second pixeldata value.

What happens if you try:
slice.pixelData.Frames[1].GetImage() in your code instead of slice.pixelData.Frames[0].GetImage() ?

@bbeckrest
Copy link
Author

@suyashkumar Thanks for the reply.

Trying the suggestion above causes a panic due to Frames only having a length of 1. Not sure why it's not parsing the second pixeldata value.

runtime error: index out of range [1] with length 1

@suyashkumar
Copy link
Owner

@bbeckrest apologies for the delay! Looks like this issue is coming from the jpeg.Decode call (from here). One option is to take the encapsulated pixel byte data and write out the byte data to disk to see if you can open it in a jpeg tool of your choice. Taking a look, there does seem to be data in the EncapsulatedFrame.

You'd want to try something like

fr, err := slice.pixelData.Frames[0].GetEncapsulatedFrame()
fmt.Println(fr.Data)
f, _ := os.Create("test.jpg")
f.Write(fr.Data)
f.Close()

to get the underlying data, which appears to indeed be parsed into the construct. That could be a good place to start the debug--is the data encoded in an odd way, perhaps using some of the newer JPEG codecs that maybe aren't supported by the Go stdlib?

At the very least, you should have access to the byte data to then try to render or process downstream. I think if this is an issue with the JPEG codecs, it might be good to pull in a third part library to help decode some of this jpeg data.

@bbeckrest
Copy link
Author

@suyashkumar You are correct. I'm able to get the byte data from the encapsulated frame using the code above. The issue is the JPEG encoding.

The encoding of the images is JPEG Lossless:Non-hierarchical, 1stOrderPrediction (1.2.840.10008.1.2.4.70)

After looking into this a bit, there doesn't seem to be an easy way to parse these. Apparently, very few image readers support this codec, which coincidentally you noted back in 2018 😜 . The std lib image/jpeg still does not support it either.

Have you come across any way to parse these since? I've read that cgo might be able to run a C dicom library that can handle the codec, but that's obviously not ideal.

@bbeckrest bbeckrest changed the title Error parsing encapsulated pixel data - "unsupported JPEG feature: unknown marker" JPEG Lossless: Error parsing encapsulated pixel data - "unsupported JPEG feature: unknown marker" Jul 23, 2020
@suyashkumar
Copy link
Owner

Ah, I was wondering if that was it! Great find, btw--I do remember thinking a little about this in the past as you found, but luckily never directly ran into this situation yet 😅 .

I haven't looked into how complicated the implementation of LosslessJEPG might be, but for what it's worth it might be a pretty decent contribution to the Go stdlib, but unclear how common it is in the wild (particularly outside of DICOM). I too haven't found a lot of support or implementations of it yet.

A quick search found this JS library that appears to implement lossless jpeg. Might be worth trying to port this over into Go.

In the short term to unblock your usecase, it may not be terrible to have a secondary stage of processing where you can run the JPEG-LS bytes through another parser (like this Javascript one, or a C++ one, even if it's not directly part of the same go program). If you're doing batch processing, you can probably spin up a docker container with a shell script that runs your go program (which can write the JEPG bytes to a file) and then invokes a JS or C++ program to decode the JPEG and save it out into some other lossless format for the rest of your pipeline.

Is it important that your code be encapsulated into a single go program, or are you working with a non-batch use case (e.g. a server)?

@bbeckrest
Copy link
Author

@suyashkumar It's not necessarily required to be encapsulated into a single go program but having it external would add enough complexity to where it's probably not worth handling right now. For the time being, we've decided to hold off on any development to solve this issue. We're hoping to gather more scans from customers and find out that this encoding is extremely rare - rare enough that we can simply specify in our CT specifications that we don't accept this encoding.

However, if we are alienating a significant number of scans, we will probably have to solve this. I think porting over the JPEGLosslessDecoderJS library you linked to would be the way to go in that case.

Regardless, thanks for your help! If we do end up having to solve this, I'll be sure to post here with follow-up.

@suyashkumar
Copy link
Owner

@bbeckrest thank you for the update, I too would love to get a better sense of how often these JPEG LS encoded DICOMs show up in the wild, and if you end up having to go down the route of solving this.

Ultimately, I would like to have this feature be fully supported here, which may require writing a JPEG LS decoder/encoder and submitting it to the Go stdlib (or one of the standard image packages). There's a lot of work in flight for this project (which I maintain on the side), so of course help and contributions are always appreciated!

Let me know how I can help moving forward, and best of luck!

@suyashkumar
Copy link
Owner

I will keep this issue open to track updates on this!

@suyashkumar
Copy link
Owner

Some relevant updates:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants