Skip to content

PNG: handle single sagittal or coronal slices correctly#2465

Closed
jdtournier wants to merge 1 commit intomasterfrom
fix_png_handling
Closed

PNG: handle single sagittal or coronal slices correctly#2465
jdtournier wants to merge 1 commit intomasterfrom
fix_png_handling

Conversation

@jdtournier
Copy link
Member

Came across this while answering a question on the forum. On current master, this happens:

$ mrconvert ~/data/anat.nii -coord 0 70 out-[].png 

mrconvert: [SYSTEM FATAL CODE: SIGSEGV (11)] Segmentation fault: Invalid memory access

Or alternatively:

$ mrconvert ~/data/anat.nii -coord 0 70 - | mrconvert - out-[].png 
mrconvert: [100%] copying from "/home/donald/data/anat.nii" to "/tmp/mrtrix-tmp-ck4o7Y.mif"
terminate called after throwing an instance of 'std::bad_alloc'
  what():  std::bad_alloc
Aborted (core dumped)

Basically any attempt to convert a single sagittal or coronal slice using the square bracket syntax. Works OK if output file is simply out.png (not using square bracket syntax). Also works OK if selecting a single axial slice.

Changes in the PR seem to fix the problem.

@jdtournier jdtournier added the bug label Apr 12, 2022
@jdtournier jdtournier added this to the 3.0.4 milestone Apr 12, 2022
@jdtournier jdtournier requested a review from a team April 12, 2022 21:11
@Lestropie
Copy link
Member

PR fails existing binary test. Being an intrinsically 2D format, there is a more complex relationship between the nature of the input data and the utilisation of square-bracket notation in the output path for PNG. I'll have to remind myself of how it works and then figure out why this edge use case fails.

@Lestropie
Copy link
Member

Lestropie commented Apr 13, 2022

Need to clarify the intended behaviour in this circumstance.

If anat.nii is a 3D image, let's say 128x128x128, and -coord 0 70 is specified, then you have an image that is 1x128x128. If you were to request to write that image to a PNG, it would detect that the first axis is of unity size, and so write a 2D image based on axes 0 and 1, which would be sagittal assuming RealignTransform is true.

But you're then specifying an output image path with the square-bracket syntax. This would logically take the last non-unity axis---the third axis in this case---and generate one PNG file per coordinate, resulting in 128 images of dimension 1x128. Suspect that this is not what the user intended, but it should nevertheless work rather than segfaulting. I don't think it makes sense that the square-bracket syntax would detect a unity dimension and "attach itself" to that dimension given that by its very nature it's intended to write multiple image files.

@Lestropie
Copy link
Member

Proposed that this be deferred from 3.0.4 to 3.1.0, given that the usage resulting in the segfault is arguably erroneous.

@Lestropie
Copy link
Member

Closing in favour of #2535.

@Lestropie Lestropie closed this Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants