Skip to content

Nifti image conflicted names #9

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hjmjohnson merged 3 commits into from
Jan 25, 2025
Merged

Nifti image conflicted names #9

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
04b1efb
ENH: Address ambiguous situation when both .nii and .nii.gz files exist
hjmjohnson Jan 23, 2025
a6678ba
ENH: Match FSL behaviors when PIGZ is defined
hjmjohnson Jan 23, 2025
8047aee
ENH: Match FSL behavior of reject complex data
hjmjohnson Jan 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,28 @@ endif()
# current build behavior
option( BUILD_SHARED_LIBS "Toggle building shared libraries." OFF)

# Add options to mimic FSL features, Can manually specify -DFSLSTYLE:BOOL=ON for parity support with makefile
if(NOT FSLSTYLE)
# If FSL_STYLE is not defined, then
option( FSLSTYLE_NAME_CONFLICTS "FSL behavior for name conflicts (generate error if both img.nii and img.nii.gz exists)" OFF)
option( FSLSTYLE_PIGZ_SUPPORT "PIGZ: Use PIGZ parallel compression if environment includes AFNI_COMPRESSOR=PIGZ" OFF)
option( FSLSTYLE_REJECT_COMPLEX "REJECT_COMPLEX: Generate error if file is complex datatype." OFF)
else()
set(FSLSTYLE_NAME_CONFLICTS ON CACHE BOOL "FSL behavior for name conflicts (ambiguous files img.nii and img.nii.gz is an error)" FORCE)
set(FSLSTYLE_PIGZ_SUPPORT ON CACHE BOOL "PIGZ support" FORCE)
set(FSLSTYLE_REJECT_COMPLEX ON CACHE BOOL "Reject complex images." FORCE)
endif()

if(FSLSTYLE_NAME_CONFLICTS)
ADD_DEFINITIONS(-DFSLSTYLE)
endif()
if(FSLSTYLE_PIGZ_SUPPORT)
ADD_DEFINITIONS(-DPIGZ)
endif()
if(FSLSTYLE_REJECT_COMPLEX)
ADD_DEFINITIONS(-DREJECT_COMPLEX)
endif()

# Include executables as part of the build
option(NIFTI_BUILD_APPLICATIONS "Build various utility tools" ON)
mark_as_advanced(NIFTI_BUILD_APPLICATIONS)
Expand Down
6 changes: 6 additions & 0 deletions nifti2/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ CC = gcc
IFLAGS = -I. -I../niftilib -I../znzlib
CFLAGS = -Wall -std=gnu99 -pedantic $(USEZLIB) $(IFLAGS)


#run "FSLSTYLE=1 make" for JPEGLS build

@seanm seanm Jan 29, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the JPEGLS connection...

ifeq "$(FSLSTYLE)" "1"
CFLAGS += -DFSLSTYLE -DPIGZ -DREJECT_COMPLEX
endif

LLIBS = -lz -lm

MISC_OBJS = nifticdf.o znzlib.o
Expand Down
132 changes: 126 additions & 6 deletions nifti2/nifti2_io.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3740,7 +3740,23 @@ char * nifti_findhdrname(const char* fname)

strcpy(hdrname,basename);
strcat(hdrname,elist[efirst]);
#ifdef FSLSTYLE
if (nifti_fileexists(hdrname)) {
free(basename);
char *gzname = (char *)calloc(sizeof(char),strlen(hdrname)+8);
strcpy(gzname, hdrname);
strcat(gzname,extzip);
Comment on lines +3747 to +3748

@seanm seanm Jan 29, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strcpy and strcat should not be used in new code

if (nifti_fileexists(gzname)) {
fprintf(stderr,"Image Exception : Multiple possible filenames detected for basename (*.nii, *.nii.gz): %s\n", basename);
free(gzname);
exit(134);

@seanm seanm Jan 29, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why 134?

Also, it doesn't seem appropriate for library code to call exit(), nowhere else in this file calls exit(). This should probably return NULL.

@neurolabusc neurolabusc Feb 4, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compile directive FSLSTYLE emulates the beahavior of fsl tools, these terminate with these exit codes for these failures:

$fslmaths a add -0 b
Image Exception : #61 :: Multiple possible filenames detected for basename: a
libc++abi: terminating due to uncaught exception of type std::runtime_error: Multiple possible filenames detected for basename: a
/Users/chris/fsl/share/fsl/bin/fslmaths: line 2: 20227 Abort trap: 6           /Users/chris/fsl/bin/fslmaths "$@"
$ echo $?
134

@seanm seanm Feb 4, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So that's a SIGABRT. If we really need to emulate that, better to call abort().

But what would happen if we just return NULL? Could you try this: #11

}
free(gzname);
return hdrname;
}
#else
if (nifti_fileexists(hdrname)) { free(basename); return hdrname; }
#endif
#ifdef HAVE_ZLIB
strcat(hdrname,extzip);
if (nifti_fileexists(hdrname)) { free(basename); return hdrname; }
Expand Down Expand Up @@ -5288,14 +5304,14 @@ nifti_1_header * nifti_read_n1_hdr(const char * hname, int *swapped, int check)
hfile = nifti_findhdrname(hname);
if( hfile == NULL ){
if( g_opts.debug > 0 )
LNI_FERR(fname,"failed to find header file for", hname);
LNI_FERR(fname,"failed to find N1 header file for", hname);
return NULL;
} else if( g_opts.debug > 1 )
fprintf(stderr,"-d %s: found header filename '%s'\n",fname,hfile);

fp = znzopen( hfile, "rb", nifti_is_gzfile(hfile) );
if( znz_isnull(fp) ){
if( g_opts.debug > 0 ) LNI_FERR(fname,"failed to open header file",hfile);
if( g_opts.debug > 0 ) LNI_FERR(fname,"failed to open N1 header file",hfile);
free(hfile);
return NULL;
}
Expand Down Expand Up @@ -5389,7 +5405,7 @@ nifti_2_header * nifti_read_n2_hdr(const char * hname, int * swapped,
hfile = nifti_findhdrname(hname);
if( hfile == NULL ){
if( g_opts.debug > 0 )
LNI_FERR(fname,"failed to find header file for", hname);
LNI_FERR(fname,"failed to find N2 header file for", hname);
return NULL;
} else if( g_opts.debug > 1 )
fprintf(stderr,"-d %s: found N2 header filename '%s'\n",fname,hfile);
Expand Down Expand Up @@ -5731,7 +5747,7 @@ void * nifti_read_header( const char *hname, int *nver, int check )
hfile = nifti_findhdrname(hname);
if( hfile == NULL ){
if(g_opts.debug > 0)
LNI_FERR(fname,"failed to find header file for", hname);
LNI_FERR(fname,"failed to find any header file for", hname);
return NULL; /* check return */
} else if( g_opts.debug > 2 )
fprintf(stderr,"-d %s: found header filename '%s'\n",fname,hfile);
Expand All @@ -5742,7 +5758,7 @@ void * nifti_read_header( const char *hname, int *nver, int check )
/**- open file, separate reading of header, extensions and data */
fp = znzopen(hfile, "rb", nifti_is_gzfile(hfile));
if( znz_isnull(fp) ){
if( g_opts.debug > 0 ) LNI_FERR(fname,"failed to open header file",hfile);
if( g_opts.debug > 0 ) LNI_FERR(fname,"failed to open any header file",hfile);
free(hfile);
return NULL;
}
Expand Down Expand Up @@ -5885,7 +5901,7 @@ nifti_image *nifti_image_read( const char *hname , int read_data )
/**- open file, separate reading of header, extensions and data */
fp = znzopen(hfile, "rb", nifti_is_gzfile(hfile));
if( znz_isnull(fp) ){
if( g_opts.debug > 0 ) LNI_FERR(fname,"failed to open header file",hfile);
if( g_opts.debug > 0 ) LNI_FERR(fname,"failed to open a header file",hfile);
free(hfile);
return NULL;
}
Expand Down Expand Up @@ -5949,6 +5965,13 @@ nifti_image *nifti_image_read( const char *hname , int read_data )
znzclose(fp); free(hfile); return NULL;
}

#ifdef REJECT_COMPLEX
if ((nim->datatype == DT_COMPLEX64) || (nim->datatype == DT_COMPLEX128) || (nim->datatype == DT_COMPLEX256)) {
fprintf(stderr,"Image Exception Unsupported datatype (COMPLEX64): use fslcomplex to manipulate: %s\n", hname);
Comment thread
seanm marked this conversation as resolved.
exit(13);

@seanm seanm Jan 29, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why 13?

@neurolabusc neurolabusc Feb 4, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a few years ago, perhaps that was the output of fsl tools at the time. Feel free to change this to 134 as that is what I get with the current version of fslmaths (the FSL error is odd, the tools work fine with float 32, so the error really refers to the enumeration for DT_COMPLEX64 which is 32.

$ fslmaths complex.nii.gz -add 0 sfsf
Image Exception : #8 :: Fslread: DT 32 not supported
libc++abi: terminating due to uncaught exception of type std::runtime_error: Fslread: DT 32 not supported
/Users/chris/fsl/share/fsl/bin/fslmaths: line 2: 19588 Abort trap: 6 /Users/chris/fsl/bin/fslmaths "$@"
$ echo $?
134

@seanm seanm Feb 4, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And you actually need that exact error code? As I said in another comment, it's not ideal IMHO to call exit() in a library...

}
#endif

if( nim == NULL ){
znzclose( fp ) ; /* close the file */
if( g_opts.debug > 0 )
Expand Down Expand Up @@ -7473,6 +7496,10 @@ int nifti_convert_nim2n1hdr(const nifti_image * nim, nifti_1_header * hdr)
nhdr.qoffset_z = nim->qoffset_z ;
nhdr.pixdim[0] = (nim->qfac >= 0.0) ? 1.0F : -1.0F ;
}
#ifdef FSLSTYLE
else //this helps for regression testing between this library and fsl, there is no other purpose. Without this you get false alarms
nhdr.pixdim[0] = 1.0; //default if unknown and not needed
#endif

if( nim->sform_code > 0 ){
nhdr.sform_code = nim->sform_code ;
Expand Down Expand Up @@ -7584,6 +7611,10 @@ int nifti_convert_nim2n2hdr(const nifti_image * nim, nifti_2_header * hdr)
nhdr.qoffset_z = nim->qoffset_z ;
nhdr.pixdim[0] = (nim->qfac >= 0.0) ? 1.0F : -1.0F ;
}
#ifdef FSLSTYLE
else //this helps for regression testing between this library and fsl, there is no other purpose. Without this you get false alarms
nhdr.pixdim[0] = 1.0; //default if unknown and not needed
#endif

if( nim->sform_code > 0 ){
nhdr.sform_code = nim->sform_code ;
Expand Down Expand Up @@ -7807,6 +7838,73 @@ znzFile nifti_image_write_hdr_img2(nifti_image *nim, int write_opts,
if( imgfile ) *imgfile = fp; \
return 1 ; } while(0)

#ifdef PIGZ
#ifdef HAVE_ZLIB
int doPigz2(nifti_image *nim, struct nifti_2_header nhdr, const nifti_brick_list * NBL) {

@seanm seanm Jan 29, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

struct nifti_2_header is a pretty big struct, might be better to pass by reference, not value, no?

FILE *pigzPipe;
char command[768];
strcpy(command, "pigz" );

@seanm seanm Jan 29, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strcpy and strcat are war crimes and should never be used in new code. :)

strcat(command, " -n -f > \"");
strcat(command, nim->fname);
strcat(command, "\"");
#ifdef _MSC_VER
if (( pigzPipe = _popen(command, "w")) == NULL)
return -1;
#else
if (( pigzPipe = popen(command, "w")) == NULL)
return -1;
#endif
znzFile fp;
fp = (znzFile) calloc(1,sizeof(struct znzptr));
fp->zfptr = NULL;
fp->withz = 0;
fp->nzfptr = pigzPipe;
Comment thread
seanm marked this conversation as resolved.
fwrite(&nhdr, sizeof(nhdr), 1, pigzPipe);
if( nim->nifti_type != NIFTI_FTYPE_ANALYZE )
nifti_write_extensions(fp,nim);
nifti_write_all_data(fp,nim,NBL);
#ifdef _MSC_VER
_pclose(pigzPipe);
#else
pclose(pigzPipe);
#endif
free(fp);
return 0;
}

int doPigz(nifti_image *nim, struct nifti_1_header nhdr, const nifti_brick_list * NBL) {
FILE *pigzPipe;
char command[768];
strcpy(command, "pigz" );
strcat(command, " -n -f > \"");
strcat(command, nim->fname);
strcat(command, "\"");
#ifdef _MSC_VER
if (( pigzPipe = _popen(command, "w")) == NULL)
return -1;
#else
if (( pigzPipe = popen(command, "w")) == NULL)
return -1;
#endif
znzFile fp;
fp = (znzFile) calloc(1,sizeof(struct znzptr));
fp->zfptr = NULL;
fp->withz = 0;
fp->nzfptr = pigzPipe;
fwrite(&nhdr, sizeof(nhdr), 1, pigzPipe);
if( nim->nifti_type != NIFTI_FTYPE_ANALYZE )
nifti_write_extensions(fp,nim);
nifti_write_all_data(fp,nim,NBL);
#ifdef _MSC_VER
_pclose(pigzPipe);
#else
pclose(pigzPipe);
#endif
free(fp);
return 0;
}
#endif //HAVE_ZLIB
#endif //PIGZ

/* ----------------------------------------------------------------------*/
/*! This writes the header (and optionally the image data) to file
Expand Down Expand Up @@ -7915,6 +8013,28 @@ static int nifti_image_write_engine(nifti_image *nim, int write_opts,
/* we will write the header to a new file */
if( g_opts.debug > 2 )
fprintf(stderr,"+d opening output file %s [%s]\n",nim->fname,opts);

#ifdef PIGZ
#ifdef HAVE_ZLIB
if ((( nim->nifti_type == NIFTI_FTYPE_NIFTI1_1 ) || (nim->nifti_type == NIFTI_FTYPE_NIFTI2_1 )) && (nifti_is_gzfile(nim->fname)) && (!leave_open) && (write_data) ) {
const char *key = "AFNI_COMPRESSOR";
char *value;
value = getenv(key);
//export AFNI_COMPRESSOR=PIGZ
char pigzKey[5] = "PIGZ";
if ((value != NULL) && (strstr(value,pigzKey))) {
if( nver == 2 ) {
if (doPigz2(nim, n2hdr, NBL) == 0)
return 0;
} else {
if (doPigz(nim, n1hdr, NBL) == 0) //success writing with pigz
return 0;
}
}
}
#endif //HAVE_ZLIB
#endif //PIGZ

fp = znzopen( nim->fname , opts , nifti_is_gzfile(nim->fname) ) ;
if( znz_isnull(fp) ){
LNI_FERR(func,"cannot open output file",nim->fname);
Expand Down
14 changes: 7 additions & 7 deletions nifti2/nifti_tool.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4045,7 +4045,7 @@ static int convert_datatype(nifti_image * nim, nifti_brick_list * NBL,
fprintf(stderr, "++ convert datatype: %s to %s, v,f = %d,%d\n",
nifti_datatype_to_string(nim->datatype),
nifti_datatype_to_string(new_type), verify, fail_choice);
fprintf(stderr, " lossless = %d\n",
fprintf(stderr, " lossless = %d\n",
is_lossless(nim->datatype, new_type));
}

Expand Down Expand Up @@ -4205,7 +4205,7 @@ static int convert_NBL_data(nifti_brick_list * NBL, int old_type, int new_type,
/*----------------------------------------------------------------------
* perform actual data conversion (basic checks are already done)
*
* It is not clear how to do this without an NxN set of cases, since
* It is not clear how to do this without an NxN set of cases, since
* the types must be explicitly noted for each pair, even with macros.
*
* So it is all written out, even when in and out types match, so that
Expand Down Expand Up @@ -5459,7 +5459,7 @@ static int is_lossless(int old_type, int new_type)
int has_int_space; /* is there int/mati ssa space? */

/*-- invalid types are either binary (bits) or unknown --*/
if( ! nifti_datatype_is_valid(old_type, 1) ||
if( ! nifti_datatype_is_valid(old_type, 1) ||
! nifti_datatype_is_valid(new_type, 1) ) {
if( g_debug > 0 )
fprintf(stderr,"** is_lossless: invalid types %d, %d\n",
Expand All @@ -5472,7 +5472,7 @@ static int is_lossless(int old_type, int new_type)
o_signed = type_is_signed(old_type); /* signed? */
n_signed = type_is_signed(new_type);
o_inttyp = nifti_is_inttype(old_type); /* integral? only using old */

/* RGB? */
o_rgbtyp = ( old_type == NIFTI_TYPE_RGB24 || old_type == NIFTI_TYPE_RGBA32 );
n_rgbtyp = ( new_type == NIFTI_TYPE_RGB24 || new_type == NIFTI_TYPE_RGBA32 );
Expand Down Expand Up @@ -6997,7 +6997,7 @@ int nt_run_misc_nim_tests(nifti_image * nim)
}

/* test expansion of ints, as 32-bits, not usually used */
{
{
int * ilist = NULL;
const char * istr = "7,4,2..5,11..$";
ilist = nifti_get_intlist(15, istr);
Expand All @@ -7017,7 +7017,7 @@ static int nt_test_dmat44_quatern(nifti_image * nim)

float fb, fc, fd, fx, fy, fz;
float x, y, z, fac;

fputc('\n', stdout);
/* and the orthogonalized form */
printf("= sform vs. orthog sform:\n");
Expand Down Expand Up @@ -7205,7 +7205,7 @@ int act_cbl( nt_opts * opts )

/*----------------------------------------------------------------------
* copy an image to a new file
*
*
* This is a straight NIFTI copy, without cbl (so upper dims are intact).
*
* Note: nt_image_read() allows for modifications, e.g. to the datatype.
Expand Down
Loading