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

Incorrect file size on export #3757

Closed
juhavlintula opened this issue Dec 14, 2019 · 12 comments
Closed

Incorrect file size on export #3757

juhavlintula opened this issue Dec 14, 2019 · 12 comments

Comments

@juhavlintula
Copy link

Describe the bug
A vertical image that has been cropped to 16:9 and that I want to export as 1440×2560 is exported as 1439×2560 (or as 1439×2559)

To Reproduce
Steps to reproduce the behavior:

  1. Crop image to 9x16
  2. Define max dimensions as 1440×2560
  3. Export JPG
  4. The exported image is not 1440×2560

Platform (please complete the following information):

  • OS: Win10 & Ubuntu 19.10
  • Version: 3.0rc2 and newer

Additional context

  • works correctly on 2.7.0
  • tested on Olympus RAWs
@jade-nl
Copy link

jade-nl commented Dec 15, 2019

Confirmed and not Olympus specific.

Tested and confirmed with: Mamiya, Nikon, Pentax and Huawei RAW's

Linux - Debian 10.2
Version 3.0.0rc2-102

@upegelow
Copy link
Member

This is probably a side effect of #3666.

Question out of curiosity: why are people so obsessed with exact image dimensions like +/- 1 on a 4 Megapixel image?

@aurelienpierre
Copy link
Member

I'm sorry, what is the bug ? Integer rounding ?

@upegelow
Copy link
Member

#3666 always rounds down in order to never read past the original image dimensions. As a side effect even a minor float rounding error e.g. leading to 1439.999878 results in 1439. Previously we would round to the next integer with issues (see comments in #3666)

We could consider something like:

int i_width = floorf(f_width + eps)

where eps is some small delta to account for float errors (e.g. 1e-6).

@aurelienpierre
Copy link
Member

Why not simply int i_width = MAX(roundf(f_width), raw_width) ?

@upegelow
Copy link
Member

It's about this part of the code:

const int processed_width = floor(scale * pipe.processed_width);
const int processed_height = floor(scale * pipe.processed_height);

The task here is: adjust the output dimensions so that with all roi calculations we end up with the correct input dimensions.

Originally that code would add 0.5f and then convert to int, effectively rounding to the closest integer value.

The problem in #3666 has been triggered by very small output dimensions, namely thumbnails. Typically in the range of 150x100 and a scale factor of 1/40. If we select the output width/height even by +1 too high we may read dozens of pixels past the input image dimensions. That's the reason for moving to the floor function.

This code needs to deal with situations of different background:

  • thumbnails: process something reasonably close to the desired dimensions with a aspect ratio that roughly fits

  • most export cases: process with a scaling factor of 1. Output dimensions should match the original one or whatever crop is selected. A minor delta does not matter.

  • special export cases: process exactly with the dimensions chosen by the user when cropping. Here it might fail.

@juhavlintula
Copy link
Author

@upegelow the reason I need exact dimensions is that 1440×2560 is is the size of my mobile phone's screen and I like to use some of my images as wallpapers. Unfortunately Huawei's software requires exact dimension or otherwise it will recrop the image.

@upegelow
Copy link
Member

@juhavlintula: this sounds like a valid use case.

@upegelow
Copy link
Member

I had a close look once again at the code but unfortunately there is no simple solution.

The parameters in the export module define the maximum width and height of the output image, not the exact size. You might now argue that this should work nonetheless as the aspect ratio you request exactly matches the one of the cropped image. However, the size calculations are a mix of floating point and integer values with some rounding issues. In the end we need to make sure that the we never read past the dimensions of the input/raw image. Therefore we round down.

darktable 2.6.x handles this differently. It rounds to the next integer. This would normally give you the output image dimensions you desire but has some serious issues with artifacts and potential crashes. This needed to be fixed.

Currently the only option I could suggest is using an external tool like ImageMagick to rescale, or you export with a higher resolution and crop in Gimp. That's not optimal but changing this behavior in darktable will require significant changes.

@junkyardsparkle
Copy link
Contributor

junkyardsparkle commented Dec 15, 2019

What about using the existence of the cropping module in the history as a flag to use a different rounding behavior? Is the existence of croppiing not enough to mitigate the ROI issues? (and if it's not now, could it be made to be?)

upegelow added a commit to upegelow/darktable that referenced this issue Dec 18, 2019
this tries to compromise between darktable-org#3757 (make output dimensions exactly as user
requests) and darktable-org#3646 (do not read outside of input/raw image buffer).
@upegelow
Copy link
Member

What about using the existence of the cropping module in the history as a flag to use a different rounding behavior? Is the existence of croppiing not enough to mitigate the ROI issues? (and if it's not now, could it be made to be?)

This would not work well. What if the user has cropping activated in the perspective correction module? What if the frame module is active?

Please have a look at pull request #3808 for an alternative option.

@junkyardsparkle
Copy link
Contributor

@upegelow Yeah, I didn't really think much about exactly what the condition(s) should be, but that was the general idea... speaking of magical "fudge factors", what do you think about adding similar safety margin to the scaling calculation in the lens correction module? There have been some reports of black corner pixels, etc due to specific opencl drivers (#3349).

TurboGit pushed a commit that referenced this issue Dec 26, 2019
this tries to compromise between #3757 (make output dimensions exactly as user
requests) and #3646 (do not read outside of input/raw image buffer).
TurboGit pushed a commit that referenced this issue Dec 26, 2019
this tries to compromise between #3757 (make output dimensions exactly as user
requests) and #3646 (do not read outside of input/raw image buffer).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants