Encoding to JPEG transfer syntaxes #303
Conversation
There was a problem hiding this comment.
Thank you for working on this. Please follow the suggestions inline to resolve the current issues.
Also, the format does not look standard. Running cargo fmt on the file should fix this.
As for the testing, we are missing a PixelDataObject implementation in this context. Let's devise a struct that keeps the necessary attributes in memory, then we can think of a way to construct them from test files.
|
Right now I just need to finish the unit tests, but since I am encountering some errors and I am still a beginner in rust, I'm taking longer to complete the rest of this pr (sorry for that); but if at any point there is a need to more quickly finish this, I can commit my work up until this point. |
|
Hey @Almeida-a, don't worry. As I intend to change the pixel data adapter API soon, it might be worth stalling this a bit more while I set up the new design, then we can work together on migrating the implementations. I hope that this works for you. |
|
Update: #361 is what I have in mind. When you have the time, please rebase this branch against it and try to adjust the implementation. Feel free to ask if you need any help. |
|
I took the liberty of rebasing this PR on top of #361 to start trying things out. I also took care of error handling in the encoder. Next I believe that this would call for a test suite to cover converting some pixel data to JPEG and back. |
|
New stuff:
I found a few more things worth addressing at #361, but the changes to be made here should be minimal. I will take care of rebasing afterwards. |
|
@Almeida-a Can you have a look and see if the new changes make sense to you? It should hopefully be ready to merge soon. |
|
On it! |
|
Looks good, thank you for your assistance! |
- this is the only supported TS for encoding right now - leave note that support for other JPEG TSes is not complete
- fixes overflow on large resolutions
- add adapter utils and TestPixelData - fixes overflow on large resolutions - add dicom-test-files as a dev dependency - add tests/jpeg.rs test suite - reading jpeg baseline and jpeg lossless - write & read cycle
- to include dependency of dicom-transfer-syntax-registry on dicom-test-files
|
Great! Thank you for the initiative and commitment, @Almeida-a! |
This is a first version of an implementation regarding issue #286.
It it not tested yet, however, any suggestion regarding that, or other mistakes, would be appreciated!
Depends on #361