CMRR-C2P / MB

Support for CMRR multi-band pulse sequences
http://www.cmrr.umn.edu/multiband/
MIT License
57 stars 20 forks source link

Debug XA-30 data handling (case 2, data stored in SpectroscopyData) + decoding fix for writing log files #348

Closed marcelzwiers closed 10 months ago

marcelzwiers commented 10 months ago

This is a straightforward bugfix PR for the python code. The only thing that I was wondering about, is that the Matlab version does some typecasting that I had to skip in the python version (see also issue #347):

    % case 2: up until R017pre6, XA-line ImageType field is wrong, stores in SpectroscopyData
    elseif (isfield(dcmInfo,'ImageType') && strcmp(dcmInfo.ImageType,'ORIGINAL\PRIMARY\RAWDATA\NONE') ... % XA30 bug
        && isfield(dcmInfo,'SpectroscopyData'))
        pdata = typecast(dcmInfo.SpectroscopyData, 'uint8');

Why is the typecasting needed? I skipped it and all seems fine without it (and case 1 and case 3 also don't do the typecasting)

eauerbach commented 10 months ago

Normally I am stuffing the data into private fields Matlab doesn't know what to do with, so it just returns them as uint8 (OB, other byte). In the case where I was using the compliant spectroscopy DICOM format, however, Matlab does know what it should do with that and returns the SpectroscopyData field properly as float (OF, other float). So I had to cast back to uint8 for that case only.

marcelzwiers commented 10 months ago

Ok, thanks for the info. It seems that pydicom returns uint8 in all 3 cases, so I dropped your typecast. Have you tried the python version? Would be nice if you could confirm that the SpectroscopyData in the python code works as expected :-)

Op vr 19 jan 2024 om 01:07 schreef Eddie Auerbach @.***

:

Normally I am stuffing the data into private fields Matlab doesn't know what to do with, so it just returns them as uint8 (OB, other byte). In the case where I was using the compliant spectroscopy DICOM format, however, Matlab does know what it should do with that and returns the SpectroscopyData field properly as float (OF, other float). So I had to cast back to uint8 for that case only.

— Reply to this email directly, view it on GitHub https://github.com/CMRR-C2P/MB/pull/348#issuecomment-1899414214, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADTUGL6TASYZIJ5CNEWISW3YPG2NPAVCNFSM6AAAAABB6HFGY2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOJZGQYTIMRRGQ . You are receiving this because you authored the thread.Message ID: @.***>