pull request #514 breaks DICOM conversion

[neurolabusc]

Hello,
Our dcm2niix github project uses openjpeg to decode JPEG2000 images embedded in DICOM files. However, version 2.2 breaks support for many images due to
#514
Therefore, we are stuck on 2.1. Does @mayeut or anyone else have a suggestion of how we can proceed. The only way we can use 2.2 is to set m_nb_tile_parts_correction_checked and recompile, but of course this would hinder others who use the library and want the patch for Adobe images. The ideal solution would seem to be an option that allows the user to set the m_nb_tile_parts_correction_checked property at run-time, depending on their needs (or an updated check that works with proprietary DCMJP2K which seems to use their own branch of openJPEG). In my mind, an ideal solution that would allow us to compile with older (2.1) as well as a future version would look something like this:

opj_set_default_decoder_parameters(&params);
#if OPJ_VERSION_MAJOR == 2
#if OPJ_VERSION_MINOR >= 3
params->m_specific_param.m_decoder.m_nb_tile_parts_correction_checked = 0;
#endif
#endif

Any other advice appreciated. Thanks for all the hard work creating this library.

[malaterre]

Could you provide an input J2K which breaks ? You can use: gdcmraw input_jp2.dcm output.j2k to create one such sample.

[neurolabusc]

Please find a sample attached as jp2k_test.zip
1.) notes.txt: a few comments
2.) jp2k1.dcm: DICOM image (where the JPEG starts 91816 bytes into the file)
3.) jp2m1.j2c: JPEG image without DICOM header
4.) nii_dicom.cpp: modified code for dcm2niix which provides more error feedback at the point where things go wrong.

To reproduce:
1.) Download dcm2niix and replace nii_dicom.cpp with version here.
2.) Compile dcm2niix, on Linux I did this with
g++ -O3 -s -I. main_console.cpp nii_dicom.cpp nifti1_io_core.cpp nii_ortho.cpp nii_dicom_batch.cpp jpg_0XC3.cpp ujpeg.cpp nii_foreign.cpp -o dcm2niix -I/usr/local/include/openjpeg-2.2 /home/research/openjpeg-master/build/bin/libopenjp2.a -lpthread
3.) Run dcm2niix, providing the folder that includes the file 'jp2k1.dcm', e.g. ./dcm2niix ~/myDICOM
4.) The new version will provide the following feedbackL

Offset 91816
FF 4F FF 51
>>OpenJPEG=2 2 0
[INFO] Start to read j2k main header (0).
[INFO] Main header has been correctly decoded.
[ERROR] opj_j2k_apply_nb_tile_parts_correction error
Error: OpenJPEG j2k_to_image failed to decode /home/research/openjpeg-master/build/bin/dcm/jp2k1.dcm

Comments
1.) opj_dump and opj_decompress can read the image, where my code can not. Perhaps the difference is that I am reading from a buffer opj_stream_create_buffer_stream while the default projects read direct from files using opj_stream_create_default_file_stream
2.) Seems related to issue 613
3.) So the question is how one reads from a stream or sets the tile defaults when reading from a stream rather than a file.
4.) The images are open source and provided with consent from my center.

[neurolabusc]

@malaterre : my simple addition of two printf() calls reveals the source of the problem, though I do not know the solution. Essentially, running this with the default program ./opj_decompress -i jp2k1.j2c -o image.pgm the stream is able to backup, whereas with my code (where I need to seek a new location in the file) the seek command is unable to backup:

if (l_current_marker != J2K_MS_SOT) {
	    printf("l_current_marker != J2K_MS_SOT\n");//CR
            /* assume all is OK */
            if (! opj_stream_seek(p_stream, l_stream_pos_backup, p_manager)) {
		printf("Unable to backup\n");//CR
                return OPJ_FALSE;
            }
	    printf("Able to backup\n");//CR
            return OPJ_TRUE;
        }

What perplexes me about this code is that it should never get to this point if the stream is not seekable, as earlier we should exit:

if (!opj_stream_has_seek(p_stream)) {
        /* We can't do much in this case, seek is needed */
        return OPJ_TRUE;
    }

So either the call to opj_stream_has_seek is faulty (as it should accurately report if a stream is seekable and exit) or the call to opj_stream_seek is faulty (as it should seek backwards after we have read a few bytes). If either of these can be fixed, this should resolve the issue.

[neurolabusc]

It seems the problem was my implementation of opj_stream_set_seek_function which assumed p_nb_bytes referred to the offset from the current position rather than the offset from the start of the file. It would be great to provide a demo program to illustrate how to use these functions, but for anyone interested, here is the working code from dcm2niix:

static OPJ_BOOL opj_seek_from_buffer(OPJ_SIZE_T p_nb_bytes, BufInfo * p_file) {
    if ((p_file->buf + p_nb_bytes < p_file->buf + p_file->len ) && (p_nb_bytes >= 0)){
        p_file->cur = p_file->buf + p_nb_bytes;
        return OPJ_TRUE;
    }
    p_file->cur = p_file->buf + p_file->len;
    return OPJ_FALSE;
} //opj_seek_from_buffer()