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.

Sign up for GitHub

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

Jump to bottom

Support image width and height larger than 32767 #3435

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Support image width and height larger than 32767 #3435

wants to merge 1 commit into from

Conversation

stweil
Copy link
Contributor

@stweil stweil commented May 19, 2021

Signed-off-by: Stefan Weil sw@weilnetz.de

@stweil stweil marked this pull request as draft May 19, 2021 15:20
@stweil
Copy link
Contributor Author

stweil commented May 19, 2021

Please don't merge until these changes were better tested.

@stweil
Copy link
Contributor Author

stweil commented May 19, 2021

The modifications here allow processing of large images which fixes issue #3184. I only tested the example in that issue and did not run performance tests or measure the increased memory usage.

@stweil stweil mentioned this pull request May 19, 2021
Open
@stweil
Copy link
Contributor Author

stweil commented May 19, 2021

Known issues: code setting x / y / width / height to INT16_MAX or -INT16_MAX needs modifications, too.

@amitdo
Copy link
Collaborator

amitdo commented May 19, 2021

Please don't merge until these changes were better tested.

This pull request is still a work in progress
Draft pull requests cannot be merged.

@amitdo
Copy link
Collaborator

amitdo commented May 19, 2021

I wonder about the impact on regular size images.

How much more memory (in percents) will be consumed with this patch?

@stweil
Copy link
Contributor Author

stweil commented May 19, 2021

How much more memory (in percents) will be consumed with this patch?

I don't have numbers up to now.

@lgtm-com
Copy link

lgtm-com bot commented May 19, 2021

This pull request introduces 3 alerts when merging eb8f13b into 93348a8 - view on LGTM.com

new alerts:

  • 3 for Comparison of narrow type with wide type in loop condition

@egorpugin
Copy link
Contributor

egorpugin commented Jun 28, 2021 •
edited

You changed int16->int32 a lot.
That must be a typedef (using) for possible future changes?

@stweil
Copy link
Contributor Author

stweil commented Jun 29, 2021

That must be a typedef (using) for possible future changes?

Yes, the current Tesseract code uses int16_t which I replaced by int32_t for image dimensions.

I considered using a typedef, but don't think that there will be a future change for which a typedef would help. What kind of possible change do you think of?

@egorpugin
Copy link
Contributor

egorpugin commented Jun 29, 2021 •
edited

What kind of possible change do you think of?

I agree, this may be subtle.
But from programmer point of view seeing coord_t (or something like that) instead of just int32_t annotate code much better.
You may note that in tess we have a lot of int vars which are hard to understand when navigating code here and there.

@stweil stweil force-pushed the imagesize branch 2 times, most recently from 9fc3685 to 5dbd736 Compare July 25, 2021 06:31
@stweil
Copy link
Contributor Author

stweil commented Jul 25, 2021

But from programmer point of view seeing coord_t (or something like that) instead of just int32_t annotate code much better.

coord_t would violate the standards which reserve types ending with "_t".
I now use TDimension.

@egorpugin
Copy link
Contributor

violate the standards

What standards?

@stweil
Copy link
Contributor Author

stweil commented Jul 25, 2021

What standards?

ISO C, see https://www.gnu.org/software/libc/manual/html_node/Reserved-Names.html.

@stweil
Copy link
Contributor Author

stweil commented Sep 9, 2021

Should we make support for large images an optional feature in release 5.0.0 by adding a configure option, for example --enable-large-images?

@egorpugin
Copy link
Contributor

Or just ignore them.
If something big does not fit into 5.0.0 soon enough, just postpone.

@amitdo
Copy link
Collaborator

amitdo commented Sep 9, 2021 •
edited

If you choose to do it for 5.0.0, I suggest to add to the help message that explains this option:

Experimental feature (use it only for testing purpose)

and in a comment in the code itself:

For package builders: We recommend not to enable this option because the feature is unstable/untested.

@stweil stweil force-pushed the imagesize branch 2 times, most recently from 5c5d98f to 2e508b3 Compare October 27, 2021 06:49
@stweil
Support image width and height larger than 32767
0b681f9 
Builds now can define LARGE_IMAGES to use 32 bit for
image dimensions instead of 16 bit.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
@stweil
Copy link
Contributor Author

stweil commented Oct 27, 2021

Most parts of the initial pull requests are now in main, so support for larger images is prepared.

@amitdo amitdo mentioned this pull request Aug 4, 2022
Open
@monotone
Copy link

@stweil Hi, Is this feat merged to main branched? Why i just see this not changed at the tag 5.3.0 source code :
image

@stweil
Copy link
Contributor Author

stweil commented Feb 16, 2023

No, it isn't merged. This pull request is still a draft.

@monotone
Copy link

it seems some years already, what‘s the reason?

GerHobbelt added a commit to GerHobbelt/tesseract that referenced this pull request Feb 27, 2023
@GerHobbelt
Introducing new APIs to assist with detecting and reporting overlarge…
7d2e851 
… input images. Available to both userland and tesseract internal code, these can be used to report & early fail images which are too large to fit in memory.

Some very lenient defaults are used for the memory pressure allowance (1.5 GByte for 32bit builds, 64GByte for 64bit builds) but this can be tweaked to your liking and local machine shop via Tesseract Global Variable `allowed_image_memory_capacity` (DOUBLE type).

NOTE: the allowance limit can be effectively removed by setting this variable to an 'insane' value, e.g. `1.0e30`.
HOWEVER, the CheckAndReportIfImageTooLarge() API will still fire for images with either width or high dimension >= TDIMENSION_MAX, which in the default built is the classic INT16_MAX (32767px); when compiled with defined(LARGE_IMAGES), then the width/height limit is raised to 24bit i.e. ~ 16.7 Mpx, which would then tolerate images smaller than 16777216 x 16777216px. (This latter part is a work-in-progress.)

Related:

- tesseract-ocr#3184
- tesseract-ocr#3885
- tesseract-ocr#3435 (pullreq by @stweil -- WIP)

# Conflicts:
#	src/api/baseapi.cpp
#	src/ccmain/tesseractclass.h
#	src/ccmain/thresholder.cpp
#	src/ccutil/params.h
#	src/textord/tordmain.cpp
GerHobbelt added a commit to GerHobbelt/tesseract that referenced this pull request Feb 27, 2023
@GerHobbelt
Introducing new APIs to assist with detecting and reporting overlarge…
eb6d523 
… input images. Available to both userland and tesseract internal code, these can be used to report & early fail images which are too large to fit in memory.

Some very lenient defaults are used for the memory pressure allowance (1.5 GByte for 32bit builds, 64GByte for 64bit builds) but this can be tweaked to your liking and local machine shop via Tesseract Global Variable `allowed_image_memory_capacity` (DOUBLE type).

NOTE: the allowance limit can be effectively removed by setting this variable to an 'insane' value, e.g. `1.0e30`.
HOWEVER, the CheckAndReportIfImageTooLarge() API will still fire for images with either width or high dimension >= TDIMENSION_MAX, which in the default built is the classic INT16_MAX (32767px); when compiled with defined(LARGE_IMAGES), then the width/height limit is raised to 24bit i.e. ~ 16.7 Mpx, which would then tolerate images smaller than 16777216 x 16777216px. (This latter part is a work-in-progress.)

Related:

- tesseract-ocr#3184
- tesseract-ocr#3885
- tesseract-ocr#3435 (pullreq by @stweil -- WIP)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@stweil @amitdo @egorpugin @monotone