DOI-USGS / ale

Abstraction Layer for Ephemerides (ALE)
Other
13 stars 33 forks source link

Add radtan distortion #575

Closed oleg-alexandrov closed 1 year ago

oleg-alexandrov commented 1 year ago

Add support for the radial + tangential distortion model. This has 5 distortion coefficients, stored in order k1, k2, p1, p2, k3. The k1, k2, k3 coefficients are for radial distortion, and p1 and p2 for tangential distortion. This was validated with the OpenCV implementation using the function cv::projectPoints(). More background in https://github.com/DOI-USGS/usgscsm/issues/465.

Licensing

This project is mostly composed of free and unencumbered software released into the public domain, and we are unlikely to accept contributions that are not also released into the public domain. Somewhere near the top of each file should have these words:

This work is free and unencumbered software released into the public domain. In jurisdictions that recognize copyright laws, the author or authors of this software dedicate any and all copyright interest in the software to the public domain.

acpaquette commented 1 year ago

@oleg-alexandrov Is the plan for this to just inject the distortion model after you create an ISD?

oleg-alexandrov commented 1 year ago

My plan is to use this distortion model for CSM cameras I create in ASP. That because this does not correspond to any sensor that ALE supports. I only added enough logic to ensure ALE and CSM can load and process files having this distortion model.

acpaquette commented 1 year ago

Is there some driver that will eventually go into ALE that uses this model?

oleg-alexandrov commented 1 year ago

Probably not. Unless it is shown that for some sensor the radtan model is better than any existing model. My plan is to use this with Earth cameras, so outside existing driver logic.

oleg-alexandrov commented 1 year ago

If the changes are approved, can they be pulled in? There's more discussion to be had at the usgscsm repo, but these ale changes appear small enough. (And without this brought in the usgscsm build fails.)

acpaquette commented 1 year ago

@oleg-alexandrov It's approved does someone internal to the DOI-USGS still need to hit merge?

oleg-alexandrov commented 1 year ago

I don't have write access to this repo (and no need to have this access). Thanks.