Perspective module outputs incorrect aspect ratio
For example, running Perspective module from Darktable 2.3.0+410~g52bbf8c4c on the following file: https://lawrence.lu/sc/DSC03576.ARW will result in a correct vertical and horizontal lines but with incorrect aspect ratio.
Darktable output: https://lawrence.lu/sc/DSC03576_01.jpg --- The image is too wide (the circular clock is stretched horizontally, the people appear fatter, etc). xmp file: https://lawrence.lu/sc/DSC03576.ARW.xmp
Same image with vertical lines corrected using Hugin: https://lawrence.lu/sc/DSC03576.jpg --- The clock is nearly perfectly circular and the image is noticeably less wide than the Darktable output.
In general ashift.c is fairly difficult to read (it appears to have inherited a lot of mysterious things from ShiftN. In theory, computing the homography from the pure rotation of the camera should be pretty straightforward: the homography is K^-1 R K where R is the 3x3 rotation matrix of the camera and K is the 3x3 intrinsic calibration. The matrix R = R_x R_y R_z can be found using Euler chained rotations. The matrix K will encapsulate the focal length, the pixel aspect ratio, shear, and image centering. Of the three rotation matricies, in ashift.c and ShiftN only the roll one is computed in a conventional manner. The other two (horizontal and vertical shift) are decomposed into two operations (shift and compression) which seem to implement H = inv(K) R_x K and V = inv(K) R_y K, respectively, but it is not obvious to me how they are derived.
#1 Updated by Daniel Lu about 1 month ago
It seems that the reason for my incorrect aspect ratio is because the focal length wasn't set correctly. For some reason, this module requires you to manually set focal length instead of acquiring it from EXIF data automatically. Even when module->dev is true and the module obtains focal length parameters from EXIF, these values are ignored in nmsfit when mode is ASHIFT_MODE_GENERIC (which is the default), due to the following line:
fit.f_length_kb = (p->mode == ASHIFT_MODE_GENERIC) ? DEFAULT_F_LENGTH : p->f_length * p->crop_factor;
This is highly unintuitive.
Moreover, I don't see any reason to set the default mode to GENERIC. Setting the focal length and crop factor is critical to making the perspective correction module correct.
#2 Updated by Ulrich Pegelow about 1 month ago
- Status changed from New to Closed: won't fix
Contrary to common belief the effective focal length cannot be detected reliably because there is no standard way of getting the image's crop factor. The generic lens model is a good compromise in many cases. Users are free to change their default settings to "specific".
#3 Updated by Daniel Lu about 1 month ago
I strongly disagree that the generic lens model is a good compromise. It gives a clearly wrong aspect ratio that silently ruins people's images without giving a warning. It is moreover unclear to the user how to fix the aspect ratio issue.
I believe that the default behaviour, for all users, should be to use "specific" and --- if the effective focal length cannot be reliably detected, then emit a warning and fallback to generic.
While there is no standard way of determining crop factor, there are many approaches that we should try first before giving up and resorting to the default lens model. For example, we can use Exif.Photo.FocalPlaneXResolution and Exif.Photo.FocalPlaneYResolution, as well as from Exif.FocalLengthIn35mmFilm.
By the way, while trying to figure out the cause of my aspect ratio woes, I had rewritten homography computation:
My method is faster (sqrt is much faster to compute than atan and sin/cos), more concise, has more descriptive variable names, and requires fewer matrix multiplications. It's not done yet --- I have to clean up the debug printfs I accidentally committed, compute the inverse intrinsics by hand (it's a super easy matrix to invert so we can avoid the slowness and numerical instability of mat3inv), and determine how to make my version work well with people's existing perspective settings from the previous version.