-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Android Translate Showcase: Update to use CameraX Beta and PreviewView #14
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Except for an issue with how image cropping is done in the analyzer, all the camera code looks great to me.
widthCropPercent = WIDTH_CROP_PERCENT, | ||
heightCropPercent = HEIGHT_CROP_PERCENT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this might be problematic in some devices. When you request an aspect ratio, it's not guaranteed you'll get that exact ratio depending on what's available from the camera stack as well as what other use cases are running simultaneously. If you are going to manipulate the images, you should determine things like this after you know what size the images coming from the camera have.
Currently, the only way to know the size of the images that will come to the analyzer is to wait until the first image comes (which is less than ideal, unfortunately). So I would suggest moving any logic about passing manipulation to the inside of the analyzer function and instead pass something closer to your "intent" as a parameter. E.g. if you want a squared image, provide an aspect ratio and then compute the appropriate cropping necessary inside of the analyze function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'm not fully understanding something correctly, but I thought what you're describing is what we're currently doing: https://github.com/googlesamples/mlkit/blob/master/android/translate-showcase/app/src/main/java/com/google/mlkit/showcase/translate/analyzer/TextAnalyzer.kt#L67
Is this not correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The size of the image coming from the analyzer is not a guarantee. You are asking for a specific aspect ratio, but it's only a "request" -- you could get anything back from the camera, unfortunately. You are pre-selecting height and width crop percent, before you know what size / aspect ratio the image will have.
Let me pick a hypothetical (but absurd) example. If a device could only return images to the analyzer with resolution 100x900 (very, very tall and skinny images) then will your logic for cropping still work? If I understand your cropping logic correctly, I suspect there's something else you can do in cases where the image aspect ratio does not match what you requested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh, I think I see the problem now, will modify the logic. Thanks for explaining it again.
...ranslate-showcase/app/src/main/java/com/google/mlkit/showcase/translate/main/MainFragment.kt
Outdated
Show resolved
Hide resolved
...ranslate-showcase/app/src/main/java/com/google/mlkit/showcase/translate/main/MainFragment.kt
Outdated
Show resolved
Hide resolved
...ranslate-showcase/app/src/main/java/com/google/mlkit/showcase/translate/main/MainFragment.kt
Outdated
Show resolved
Hide resolved
* @return suitable aspect ratio | ||
*/ | ||
private fun aspectRatio(width: Int, height: Int): Int { | ||
val previewRatio = max(width, height).toDouble() / min(width, height) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost a nit, but comparing the difference between ratio values feels weird… I would probably take the logarithm of this value, and compare it to the logarithm of the standard aspect ratios?
...late-showcase/app/src/main/java/com/google/mlkit/showcase/translate/analyzer/TextAnalyzer.kt
Outdated
Show resolved
Hide resolved
* of preview ratio to one of the provided values. | ||
* | ||
* @param width - preview width | ||
* @param height - preview height | ||
* @return suitable aspect ratio | ||
*/ | ||
private fun aspectRatio(width: Int, height: Int): Int { | ||
val previewRatio = max(width, height).toDouble() / min(width, height) | ||
if (abs(previewRatio - RATIO_4_3_VALUE) <= abs(previewRatio - RATIO_16_9_VALUE)) { | ||
val previewRatio = log(max(width, height).toDouble() / min(width, height), 10.0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: It doesn't really matter which logarithm base you use, so I would use the default natural logarithm, because it's shorter.
Also planning on using this as the code for an ML Kit + CameraX codelab
cc @owahltinez