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

reduce pixel_density problem severity: error -> warning, #339

Merged
merged 2 commits into from
Nov 5, 2019

Conversation

kba
Copy link
Member

@kba kba commented Nov 1, 2019

Comment on lines 157 to +158
if v is None or v <= 72:
self.report.add_error("Image %s: %s (%s pixels per %s) is too low" % (f.ID, k, v, exif.resolutionUnit))
self.report.add_warning("Image %s: %s (%s pixels per %s) is too low" % (f.ID, k, v, exif.resolutionUnit))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is the right thing to do here. This can be a significant problem for processors, so there should be a loud and clear error.

Moreover, core should at least provide a running definition of suspiciously low DPI – not just in the validator, but in OcrdExif too, and not just in isolation, but in relation to the pixel resolution as well.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO the best way to react in OcrdExif is to raise an error and set the density to 1 (as if nothing had been set) – all under the assumption of OCR-D/spec#129 that correct settings cannot be enforced in the first place.

@kba
Copy link
Member Author

kba commented Nov 5, 2019

I'll merge this now for the 2.0.0 release but as with OCR-D/spec#129 the discussion shall continue! @bertsky @wrznr feel free to open an issue here or in spec to discuss the rules of the heuristics and how we should handle bad input images.

@kba kba merged commit 8767e03 into OCR-D:master Nov 5, 2019
@kba kba deleted the relax-ppi branch November 15, 2019 15:27
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

Successfully merging this pull request may close these issues.

2 participants