Skip to content

feat: ocr(android)#96

Merged
NorbertKlockiewicz merged 2 commits into@norbertklockiewicz/ocr-implementationfrom
@nk/ocr-android
Feb 21, 2025
Merged

feat: ocr(android)#96
NorbertKlockiewicz merged 2 commits into@norbertklockiewicz/ocr-implementationfrom
@nk/ocr-android

Conversation

@NorbertKlockiewicz
Copy link
Copy Markdown
Contributor

@NorbertKlockiewicz NorbertKlockiewicz commented Feb 13, 2025

Description

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update (improves or adds clarity to existing documentation)

Tested on

  • iOS
  • Android

Testing instructions

Screenshots

Related issues

Checklist

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the documentation accordingly
  • My changes generate no new warnings

Additional notes

@NorbertKlockiewicz NorbertKlockiewicz marked this pull request as ready for review February 13, 2025 14:37
@NorbertKlockiewicz
Copy link
Copy Markdown
Contributor Author

I am wondering if I should add comments for functions similarly to iOS version

@NorbertKlockiewicz NorbertKlockiewicz changed the title feat: implementation of ocr for android feat: ocr(android) Feb 13, 2025
Comment on lines +47 to +49
} catch (e: Exception) {
onComplete(1, e)
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we tell from error which model failed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Without separate try catches for every model I don't think so

Comment on lines +90 to +93
if (rotatedConfidenceScore > confidenceScore) {
result = rotatedResult
confidenceScore = rotatedConfidenceScore
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Shouldn't we skip processing if confidence is still below Constants.LOW_CONFIDENCE_THRESHOLD?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sometimes the confidence score is still low and the result is correct, we are returning a confidence score to user so I think we should leave handling those cases for him


subArray.forEachIndexed { i, currentChar ->
if (i > 0) {
isNotRepeated.add(lastChar != currentChar)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why are we ignoring repeated characters? if we haev text like strawberry does this ignore double r's?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Simply the model is scanning the image column by column where column has width of I believe 1 pixel, so the same character will be scanned more than once, that's why we are ignoring repeated characters

@NorbertKlockiewicz NorbertKlockiewicz merged commit 42cc379 into @norbertklockiewicz/ocr-implementation Feb 21, 2025
@NorbertKlockiewicz NorbertKlockiewicz deleted the @nk/ocr-android branch February 21, 2025 12:31
NorbertKlockiewicz added a commit that referenced this pull request Feb 24, 2025
<!-- Provide a concise and descriptive summary of the changes
implemented in this PR. -->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] Documentation update (improves or adds clarity to existing
documentation)

- [ ] iOS
- [x] Android

<!-- Provide step-by-step instructions on how to test your changes.
Include setup details if necessary. -->

<!-- Add screenshots here, if applicable -->

<!-- Link related issues here using #issue-number -->

- [x] I have performed a self-review of my code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have updated the documentation accordingly
- [x] My changes generate no new warnings

<!-- Include any additional information, assumptions, or context that
reviewers might need to understand this PR. -->
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