DicomJsonConverter throws for non-DicomElements

[edespong]

Describe the bug
Trying to serialize a DICOM header that contains DicomItems that are not DicomElement. This results in exceptions.

To Reproduce

Minimal program:

using FellowOakDicom;
using FellowOakDicom.Serialization;
using JsonSerializer = System.Text.Json.JsonSerializer;

DicomDataset ds = new DicomDataset(new DicomOtherByteFragment(DicomTag.PixelData));
var jsonOptions = new JsonSerializerOptions {
    WriteIndented = true,
    Converters = { new DicomJsonConverter() }
};
string jsonArray = JsonSerializer.Serialize(ds, jsonOptions);

Expected behavior
To handle serialization gracefully, according to type. There are probably variations based on the subclass of DicomItem.

Basically, the code in the serializer is:

private void WriteJsonDicomItem(Utf8JsonWriter writer, DicomItem item, JsonSerializerOptions options) {
    writer.WriteStartObject();
    writer.WriteString("vr", item.ValueRepresentation.Code);

    switch (item.ValueRepresentation.Code) {
        case "PN":
               ......
        case "OB":
        case "OD":
        case "OF":
        case "OL":
        case "OV":
        case "OW":
        case "UN":
            WriteJsonOther(writer, (DicomElement)item);                <---- throws here
            break;
        case "FL":

There is no check anywhere that the item is actually a DicomElement.

Environment
fo-dicom 5.1.2

[gofal]

Thank you for reporting this bug!

[gofal]

@mrbean-bremen I am struggling with this, maybe you could help me with your knowledge from pydicom?
When parsing pixeldata, then fo-dicom distinguishes between "plain data", where the whole data is stored into a DicomOtherByte, or fragmented, where instead a DicomOtherByteFragment is instanciated and the data is split into fragments. The difference on how to tread the binary stream is well defined in Dicom standard.
But I do not find this difference in Dicom Json definition. So it seems even in case of fragmented data, the whole pixeldata has to be serialized as huge InlineBinary.
While for the serialization from DicomDataset to Json this would be fine, I wonder what to do when deserializing. When parsing a binary Dicom file, then the length of the DicomTag is important, if it is UndefinedLength (FFFFFFFF) then the following data is fragmented, else not. But the Json does not have a Length property. So this information is lost on serialization.

How does pydicom handle this? What is the json result, when serializing the file "CT1_J2KI" from fo-dicom.Tests/TestData?

[mrbean-bremen]

Interesting question - I have to admit that I haven't thought about this. My understanding so far was that each tag (including PixelData, even if it is encapsulated) is encoded as one single base64 encoded value. Admittedly, the cases I have heard of in practice have not used InlineBinary for Pixel Data, but send it separately via BulkDataURI.

The standard only says:

If an attribute contains an "InlineBinary", this contains the base64 encoding of the enclosing attribute's Value Field.
There is a single InlineBinary value representing the entire Value Field, and not one per Value in the case where the Value Multiplicity is greater than one.

It says nothing about encapsulated data, at least as far as I can see. As far as I know, pydicom also handles it this way, e.g. as one opaque block of data.

[gofal]

Sure, I will be able to serialize the fragmented data as one block of data. But how does pydicom then handle the deserialization?

[mrbean-bremen]

As far as I can see, there is no special handling - it is just decoded from base64 and added as (byte array) value to the PixelData element, and handled as if read from a stream.
There are some roundtrip tests (encoding in json, reading back from json and check that the datasets are equal), and this works also for compressed transfer syntaxes like JPEG2000.
Note though that pydicom always handles PixelData as opaque blobs until it is needed to read, so the inner structure (e.g. the embedded sequences) are not considered at all at this level.

[gofal]

tried a lot, but still didn't manage to convert a jpeg2k compressed file with fragmented pixeldata into a json with available tools. some refused to convert.
And dcmt converted everything but the pixeldata and wrote to the output, that json inlinebinary is not suppored for compressed pixeldata.

I more and more get the impression, that by the DICOM standard DicomJson is intended for data exchange and similar to DICOM Media it is assumed that the data uns excanged uncompressed.

The currenst state, that an exception is throws, is not acceptable. As said, I try to serialize and deserialize the data in a compatible way, so that other applications still can work with the data that fo-dicom creates.
But other approaches would be, that

• first the data should be transformed to an uncompressed transfersyntax and then Json-serialized,
• or to skip the pixel data at all.

Any feedback is welcome!

[mrbean-bremen]

I more and more get the impression, that by the DICOM standard DicomJson is intended for data exchange and similar to DICOM Media it is assumed that the data uns excanged uncompressed.

I don't think so. If the data is compressed, it would not be a good idea to uncompress it before transferring it over the wire, as that that may substantially increase the needed time.

I would say that we go the dcmtk way, e.g. do not support InlineBinary for compressed data, but unfortunately, we also don't support BuldDataURI for that data, as it is handled the same way.
If there is no easy way to handle this in fo-dicom, I would propose that fo-dicom shall not write the pixel data in this case and issue a respective warning or error (and adapt the documentation, of course), at least for the current version.
This would at least avoid the crash.

Ultimately though this should be implemented, as I think this use case gets ever more common. I don't really know the code here, but I think the easiest way would probably be to write the pixel data into a buffer the same way it is written to a stream or file, and use that buffer for serialization. This would be memory intensive and not very efficient, so in the long run this should be optimized, for example by retaining the pixel data buffer as a byte array, and decoding (e.g. writing it into the fragment objects) only if needed. Or retain the pixel data buffer and have the fragment objects point to different offsets of the buffer. Or something else...
But this should probably be handled and discussed in a separate issue.