Skip to content
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.

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

segfault when saving image after using mapim #1971

Closed
afontenot opened this issue Jan 29, 2021 · 12 comments
Closed

segfault when saving image after using mapim #1971

afontenot opened this issue Jan 29, 2021 · 12 comments

Comments

@afontenot
Copy link

Describe the bug
Using pyvips, I reliably get a segfault when doing image.write_to_file after having previously done image = image.mapim(transform).

My transform image is the result of a complex series of functions that create a perspective transformation of the original image. There's too much code involved to be useful, however I can provide a simple FITS image that replicates the issue in a simple (5 lines) test case. Even if my transform is somehow bogus, I don't think that should cause the issue since I have looked at every pixel in the output image (using getpoint), and all the values seem sane.

In any case, it seems like segfaulting isn't the right behavior for handling an error.

To Reproduce
Steps to reproduce the behavior:

  1. Try to write an image to a file after transforming it with mapim, using certain specific transforms that seem to cause this problem. (See test case.)
  2. Observe segfault. It's probably useless but I provided the gdb backtrace.

Environment

  • OS: Arch Linux (x86_64 Linux 5.10.10)
  • Vips: libvips 8.10.2
  • Pyvips: 2.1.14
  • Python: 3.9.1

Files for test case: https://ipfs.io/ipfs/QmQks3UnxiaVoA8WT1msCY4aaSnYMBLLidY3URHE6RrtCh/testcase.zip

Backtrace: bt.txt

@afontenot afontenot added the bug label Jan 29, 2021
@afontenot
Copy link
Author

I have (sort of) figured out what's going wrong here. This happens whenever the transform contains invalid numbers, e.g. float("nan"). I'm still not clear on why this is happening, because I can use getpoint to see the value of every pixel in the output image, and they seem to be working as expected. Points in the transform that are float("nan") are mapped to black, which is exactly what I want. (There are plenty of transformations in which not every point maps to a pixel in the input image.)

So as a workaround for now, I'm doing .abs() before taking the square root in the function that creates the transformation image. I'll then need to mask out the region that is not supposed to exist after doing .mapim. If anyone knows of a better way of doing this, or can explain why vips crashes, that would be great.

@jcupitt
Copy link
Member

jcupitt commented Jan 30, 2021

Hello again, I agree it should handle things like NaN pixels gracefully. I'll have a look. Thanks for the report!

jcupitt added a commit that referenced this issue Jan 30, 2021
We were not avoiding NaN in float transform images, leading to segvs in
some cases.

Thanks afontenot

see #1971
@jcupitt
Copy link
Member

jcupitt commented Jan 30, 2021

I added NaN avoidance to mapim and credited you in the changelog. Your example runs for me now:

$ vips mapim redblue.png test.png transform.fits 

To make:

test

@jcupitt
Copy link
Member

jcupitt commented Jan 30, 2021

Off-topic, but did you find nip2? It's handy for developing stuff like this. If you save your transform image as .v you can load it into nip2 and check pixel values with a mouse. You can also do arithmetic on them, etc. etc.

https://github.com/libvips/nip2

@afontenot
Copy link
Author

Hey, @jcupitt, thanks for your reply. Sorry for not getting back to you, I was working on this for a while as part of a hobby project but ended up getting distracted. I appreciate the fix and will definitely check out nip2, it looks very cool!

Off topic, but it's cool to see that you can compose stuff like mapim with unix commands. I was trying to do everything in Python, and will probably continue, but I love it when libraries come with nice terminal interfaces like that.

@afontenot
Copy link
Author

Thanks again for the fix. I can confirm that it solves the specific issue with NaN for me. However, as soon as I applied the changes, I got another segfault. I reduced this one to a minimal test case as well. Could you have a look at it? Let me know if you want me to file a different issue for this.

https://ipfs.io/ipfs/Qmcpmo5PHSa4FGsAQf6iThCxE13QcBScHfZcQ1UaLpXffs/testcase.zip

I tested that this problem occurs with both a simple script using pyvips and with the vips tool. This one appears to show up only for large images. The transform file broken.fits never works for me. The file working.fits works about 95% of the time, the other 5% of the time I get a segfault with that one too. In this case redblue.png is 10000x5000. I can't replicate the issue with a version of it that's only 5000x2500.

The weird thing is that working.fits has illegal values (out of range for the source file), so I'm not sure why it ever works. Maybe the interpolation takes care of that. In any case all the values in broken.fits are legal, except the NaN entries.

I initially backported the changes to libvips-8.10.5, but I've now also tested with the latest master.

@jcupitt
Copy link
Member

jcupitt commented Feb 22, 2021

Hello again, sorry for the delay, I was stuck on another project.

Could you open a new issue, please? This sounds like a separate problem. I'll close this one.

@jcupitt jcupitt closed this as completed Feb 22, 2021
@jcupitt
Copy link
Member

jcupitt commented Feb 22, 2021

Actually, it's OK, it was a bug in my nan-avoidance code :(

Your second example is working in git master now:

$ vips mapim redblue.png test.png broken.fits 
$

Thanks again for reporting this. I hope it's all working now.

@afontenot
Copy link
Author

Thanks @jcupitt, it's working great now!

And no worries about the delay, as I said this was just a hobby project for me.

If you're curious about my usage of vips: there was a Twitter thread that pointed out that this image is the highest resolution image of Mars currently in existence. (It's actually a mosaic in point perspective, not a single shot.) Since it uses Viking imagery exclusively, that means the "best" image of Mars is closing in on five decades old!

I noticed that there's actually a much higher resolution (256 pixels / degree) equirectangular mosaic of the whole planet here, which could be remapped into the same perspective to create a much higher resolution image.

There's free (but closed source) software that can do this kind of thing, but it's limited to smaller images. I ended up working out the transformations myself because it's not easy to find them online, but I coming up with a tool that could implement the transformation with a reasonable amount of memory usage seemed insurmountable - until I found vips. So thanks!

@jcupitt
Copy link
Member

jcupitt commented Feb 23, 2021

That's very cool -- do you have a link to your new image?

@afontenot
Copy link
Author

I'm working on a write up and a code release for it, but here's a preview image. Direct link to 10k x 10k JPEG

Aesthetically the official one is much better. My source mosaic has obvious "passes" from the MDIM data, and also some blown out highlights. I don't really like the blending, but it's the only way to get super-high res without creating it myself from scratch using the sources. I think for the official one they must have started with the Viking source (filtered monochromatic images) without the MDIM data.

They've also got some 3D lighting going on with what looks to me like DEM-based shadows. I have no idea how to do any of that, especially in the context of trying to do a transformation from equirectangular projection. Maybe some kind of shader? IDK. I wrote this whole thing from scratch with no prior knowledge because I couldn't find the necessary transformations online, or any software to do it for me that worked with images this big. It was fun!

@jcupitt
Copy link
Member

jcupitt commented Feb 25, 2021

Oh very nice!

Yes, I guess they've lit it somehow. I tried putting a simple vingette on your reconstruction:

mars

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants