[dicom-dump] Add json output format #440
Conversation
* Change option from output to format * Pretty print to writer instead of to string
|
So I took the liberty of continuing with this pull request:
Alas, this might have hit a very weird bug in Rust, because it started raising absurd errors in another crate. I might need to look for advice with members of the Rust team on how to diagnose this. |
|
Thanks for working on this, sorry I've been AWOL, my day job has been very busy! Just checking, is there anything I can do here to help? I still plan on working on my other PR as well |
|
Thank you for the heads-up. I managed to get things sorted out in this PR. I suspect at this point that the dependency changes may have introduced trait implementations which ambiguated the I would certainly be grateful if you did another iteration on #437, so we can complete that one as well. |
|
Further testing shows that the tool chokes a bit on files with encapsulated pixel data, whereas it would be nicer if it just ignored the pixel data with a warning. I suspect that this would complicate the implementation though. |
|
Sorry for the delay on this one, kinda forgot about it! I added a simple serializer for |
| let offset_table = self.inner().offset_table(); | ||
| let fragments = self.inner().fragments(); | ||
| let mut map = serializer.serialize_map(Some(2))?; | ||
| map.serialize_entry("Offset Table", offset_table)?; |
There was a problem hiding this comment.
I have mixed feelings about this. :) It may be interesting for encapsulated pixel data serialization to be supported in some way, but it may also be surprising for this to exist as a non-standard extension provided specifically by this implementation.
I think we'd be able to serve both worlds if we did the following in this order:
- At dicom-dump, we can check whether Pixel Data is encapsulated, raise a warning that it will be ignored, and remove it from the dataset before the dump.
- Later on, we can make it possible to provide parameters to the serialization process, so that the decision of whether to serialize encapsulated pixel data or not can be explicitly stated by the API user or the
dicom-dumpuser.- IIRC we don't quite have a way to do this yet. Intuitively I would make this a field of
DicomJson, but since it is defined as a single item tuple, adding new fields is likely breaking change (which could probably be mitigated through some tricker, but a breaking change nevertheless).
- IIRC we don't quite have a way to do this yet. Intuitively I would make this a field of
There was a problem hiding this comment.
Done. I left the serialize implementation for PixelFragmentSequence for when/if we do want to support that, but I can remove if you don't want unused code
Add flag to output in dicom json format