ImagingDataCommons / libdicom

C library for reading DICOM files
https://libdicom.readthedocs.io
MIT License
16 stars 7 forks source link

Problem with 8192 Size in Rows and Columns Being Considered as Empty #89

Closed y-baba-isb closed 3 days ago

y-baba-isb commented 1 month ago

When setting the size of Rows and Columns to 8192, they are considered empty. If I set the size to 8191, OpenSlide processes the image correctly. However, when the size is set to 8192, the following error is returned:

openslide.lowlevel.OpenSlideError: libdicom Parse error: Frame read failed - Value of 'Columns' or 'Rows' is out of range.

I reviewed the following source code: https://github.com/ImagingDataCommons/libdicom/blob/main/src/dicom-parse.c#L619

if (length > 0) {
    if (vr != DCM_VR_UI) {
        if (isspace(value[length - 1])) {
            value[length - 1] = '\0';
        }
    }
}

This code uses isspace to check whether the character is a space (0x20). Since 8192 in hexadecimal is 0x2000, could it be that this value is incorrectly being interpreted as zero?

y-baba-isb commented 3 weeks ago

I created a DICOM file with each frame having a size of 8192, and with two frames each for rows and columns. When I try to open this file with libdicom via OpenSlide, I get the error message: "libdicom Parse error: Frame read failed - Value of 'Columns' or 'Rows' is out of range." I have attached the DICOM file I created; could you please take a look at it? H00-0008k-01-01_40x.zip

y-baba-isb commented 1 week ago

Could you please let me know whether this issue is with the library itself or with the data?

jcupitt commented 1 week ago

Hello @y-baba-isb, I'm very sorry for the slow reply, your mail dropped off my todo list, unfortunately.

I'll look into your issue now.

jcupitt commented 3 days ago

You were absolutely right, we were accidentally trimming space characters from the end of numeric fields. I've made a PR and credited you.

Thank you for your patience, and thanks for reporting this!

This fix will be in 1.2, in a month or so.

jcupitt commented 3 days ago

... I'll close, let's continue any discussion in the PR.