fix: instance transfer syntax UID is required for decoding images #108
Conversation
|
@pieper - I will get the first PR done first, then merge it here and finish with this one. |
|
|
||
| requestInstance.open(method, url, true); | ||
| if ('responseType' in request) { | ||
| if (request.responseType) { |
There was a problem hiding this comment.
Excludes null/undefined which was causing issues.
| 'image/gif', | ||
| 'image/png', | ||
| 'image/jp2', | ||
| 'image/*', |
There was a problem hiding this comment.
image/* is a valid type, and I was trying to get the generic retrieve working, but it doesn't seem to be allowed for dcm4chee
There was a problem hiding this comment.
Some fixes for the retrieveInstanceFrames, needed since I added an actual integration test for it, and that wasn't running until I added the fixes.
| * @param {*} destination | ||
| * @param {string} headerString | ||
| */ | ||
| function addHeaders(destination, headerString) { |
There was a problem hiding this comment.
This allows for retrieving the actual transfer syntax used in the image encoding. This is allowed by Part 18, but unfortunately not required there.
| } | ||
|
|
||
| Object.defineProperty(destination, 'headers', { value: headers }); | ||
| Object.defineProperty(destination, 'contentType', { value: contentType }); |
There was a problem hiding this comment.
Having at least the contentType allows one to figure out what the encoding is close enough to allow testing for which decoder to use.
| // mediaTypes: [ {mediaType: 'image/*' }], | ||
| }; | ||
|
|
||
| const frames = dwc.retrieveInstance(options); |
There was a problem hiding this comment.
This didn't even use the await, so it was essentially an empty test, without checking if the response was valid.
| const frames = dwc.retrieveInstance(options); | ||
| }); | ||
| const frames = await dwc.retrieveInstanceFrames(options); | ||
| expect(frames instanceof Array).toBe(true); |
There was a problem hiding this comment.
Now the test at least checks that the response has an image frame, and for the given server, tests for uncompressed.
| }; | ||
|
|
||
| const metadata = await dwc.retrieveInstanceMetadata(options); | ||
| const metadata = await dwc.retrieveSeriesMetadata(options); |
There was a problem hiding this comment.
Since instance metadata is tested elsewhere, this tests just makes sure series metadata also works.
|
@pieper - I had to make a few more extensive changes for fixing the retrieveInstanceFrames as there were a number of spots that weren't being tested and weren't in fact working. However, I think it is ready for review now. I'm not sure why the server isn't returning compressed image frames - it should as far as I can tell, but it might be that the content type isn't getting specified in a way that it understands. |
pieper
left a comment
pieper
left a comment
There was a problem hiding this comment.
Okay, I think this makes good sense. Thanks for working on it 👍
|
🎉 This PR is included in version 0.11.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
The retrieveInstanceFrames works, but doesn't return the transfer syntax, making the image undecodeable in general. This PR just adds that as a hidden attribute on the buffer object being returned.