-
Notifications
You must be signed in to change notification settings - Fork 7.4k
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
Implemented CameraX and Data Binding in Object Detection App #341
base: master
Are you sure you want to change the base?
Conversation
@@ -14,151 +14,150 @@ | |||
* limitations under the License. | |||
*/ | |||
|
|||
package org.tensorflow.lite.examples.detection; | |||
|
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.
Please don't delete the test, but make it work. We need to test the change and make sure it works as intended.
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.
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.
Sure, will add this asap!
@@ -17,278 +17,307 @@ | |||
package org.tensorflow.lite.examples.detection; | |||
|
|||
import android.Manifest; |
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.
Imports should be sorted in alphabetic order.
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.
Noted!
final String labelFilename, | ||
final int inputSize, | ||
final boolean isQuantized) | ||
throws IOException { | ||
return new TFLiteObjectDetectionAPIModel(context, modelFilename); |
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.
Please format following 2 spaces indentation.
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.
Noted!
int cropSize = min(width, height); | ||
|
||
ImageProcessingOptions imageOptions = | ||
ImageProcessingOptions.builder() |
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.
Ditto (indent).
"" + cnt++, | ||
detection.getCategories().get(0).getLabel(), | ||
detection.getCategories().get(0).getScore(), | ||
detection.getBoundingBox())); |
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.
Ditto (indent).
final int dstWidth, | ||
final int dstHeight, | ||
final int applyRotation, | ||
final boolean maintainAspectRatio) { |
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.
Ditto (indent).
LOGGER.e(e, "Exception!"); | ||
return; | ||
detector = | ||
TFLiteObjectDetectionAPIModel.create( |
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.
No line break please.
e.printStackTrace(); | ||
LOGGER.e(e, "Exception initializing Detector!"); | ||
Toast toast = | ||
Toast.makeText( |
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.
Ditto (no line break)
switch (newState) { | ||
case BottomSheetBehavior.STATE_HIDDEN: | ||
break; | ||
case BottomSheetBehavior.STATE_EXPANDED: { |
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.
Please follow 2 spaces indentation.
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.
Thank you for the PR!
Since it contains UI change, can you show a screenshot of current running app? Thank you!
Yes sure I will attach a Screenshot in the upcoming Commit
sheetBehavior.setPeekHeight(height); | ||
} | ||
}); | ||
new ViewTreeObserver.OnGlobalLayoutListener() { |
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.
Ditto (indent).
|
||
if (hasPermission()) { | ||
setFragment(); | ||
//Start CameraX |
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.
Comment should start with a space, like // Start CameraX
. Please do the same for the other occurrences.
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.
Noted!
return yuvBytes[0]; | ||
} | ||
private void onStartCameraX(Size size, final int rotation) { | ||
final float textSizePx = |
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.
What does px
in textSizePx
mean? You can call it textSize
.
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.
Ok Sure
@@ -79,7 +79,7 @@ public void drawText(final Canvas canvas, final float posX, final float posY, fi | |||
} | |||
|
|||
public void drawText( | |||
final Canvas canvas, final float posX, final float posY, final String text, Paint bgPaint) { | |||
final Canvas canvas, final float posX, final float posY, final String text, Paint bgPaint) { |
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.
Please recover the original indent. (spaces = 2)
@@ -117,12 +117,12 @@ public void setAlpha(final int alpha) { | |||
} | |||
|
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.
Please recover the original indent. (spaces = 2)
return 0; | ||
} | ||
protected void setUseNNAPI(final boolean isChecked) { | ||
runInBackground( |
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.
Please format this. (spaces = 2)
@@ -341,13 +370,15 @@ protected synchronized void runInBackground(final Runnable r) { | |||
} | |||
} | |||
|
|||
|
|||
@Override |
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.
Please recover the original indent. (spaces = 2)
public void run() { | ||
ImageUtils.convertYUV420SPToARGB8888(bytes, previewWidth, previewHeight, rgbBytes); | ||
binding.trackingOverlay.addCallback( | ||
canvas -> { |
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.
Please format this with the right indentation. (spaces = 2)
camera.addCallbackBuffer(bytes); | ||
isProcessingFrame = false; | ||
|
||
if (!isProcessingFrame) { |
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.
Extra 2 spaces. The block doesn't align with the previous one.
image.close(); | ||
return; | ||
} catch (ExecutionException | InterruptedException e) { | ||
e.printStackTrace(); |
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.
Please log the error, instead of printing it.
Color.parseColor("#AA33AA"), | ||
Color.parseColor("#0D0068") | ||
Color.BLUE, | ||
Color.RED, |
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.
Please recover the original indentation. (spaces = 2)
@@ -144,13 +145,11 @@ public synchronized void draw(final Canvas canvas) { | |||
canvas.drawRoundRect(trackedPos, cornerSize, cornerSize, boxPaint); | |||
|
|||
final String labelString = | |||
!TextUtils.isEmpty(recognition.title) | |||
? String.format("%s %.2f", recognition.title, (100 * recognition.detectionConfidence)) |
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.
Please recover the original indentation. (spaces = 2)
borderedText.drawText( | ||
canvas, trackedPos.left + cornerSize, trackedPos.top, labelString + "%", boxPaint); |
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.
Please recover the original indentation. (spaces = 2)
@@ -170,7 +169,7 @@ private void processResults(final List<Recognition> results) { | |||
rgbFrameToScreen.mapRect(detectionScreenRect, detectionFrameRect); | |||
|
|||
logger.v( | |||
"Result! Frame: " + result.getLocation() + " mapped to screen:" + detectionScreenRect); | |||
"Result! Frame: " + result.getLocation() + " mapped to screen:" + detectionScreenRect); |
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.
Please recover the original indentation. (spaces = 2)
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.
Thank you for the PR!
Since it contains UI change, can you show a screenshot of current running app? Thank you!
return result; | ||
} | ||
} | ||
//package org.tensorflow.lite.examples.detection; |
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.
Please make the test passed rather than commenting it out.
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.
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.
Noted!
@@ -378,173 +409,59 @@ private void requestPermission() { | |||
CameraActivity.this, | |||
"Camera permission is required for this demo", | |||
Toast.LENGTH_LONG) | |||
.show(); | |||
.show(); |
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.
Please revert all changes that only change the indent like this. It make the PR very hard to review because there're a lot of changes that don't have any effect.
androidTestImplementation 'androidx.test:rules:1.4.0' | ||
|
||
// CameraX dependencies | ||
def camerax_version = "1.1.0-alpha05" |
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.
Please use latest stable version
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.
Noted!
// CameraX Lifecycle Library | ||
implementation "androidx.camera:camera-lifecycle:$camerax_version" | ||
// CameraX View class | ||
implementation "androidx.camera:camera-view:1.0.0-alpha25" |
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.
Do you really need androidx.camera:camera-view
? I don't see you use it anywhere in the repo.
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.
Using the androidx.camera:camera-view
To Connect the preview use case to the previewView
preview.setSurfaceProvider(binding.previewView.getSurfaceProvider());
lastProcessingTimeMs = SystemClock.uptimeMillis() - startTime; | ||
LOGGER.e("Degrees: %s", results); | ||
|
||
float minimumConfidence = MINIMUM_CONFIDENCE_TF_OD_API; |
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.
This chunk of code doesn't do anything.
float minimumConfidence = MINIMUM_CONFIDENCE_TF_OD_API;
if (MODE == DetectorMode.TF_OD_API) {
minimumConfidence = MINIMUM_CONFIDENCE_TF_OD_API;
}
private BottomSheetBehavior<LinearLayout> sheetBehavior; | ||
|
||
protected TextView frameValueTextView, cropValueTextView, inferenceTimeTextView; | ||
private enum DetectorMode { | ||
TF_OD_API; |
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.
What is this enum for? If it isn't used, remove it.
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.
It was previously there in the code so I felt like keeping it.
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.
Please delete as it doesn't do anything.
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.
Noted!
|
||
private static final int TF_OD_API_INPUT_SIZE = 300; | ||
private static final boolean TF_OD_API_IS_QUANTIZED = true; | ||
private static final String TF_OD_API_MODEL_FILE = "detect.tflite"; |
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.
Why do you prefix variable names with TF_OD_API? If there's no special reason for that, please remove.
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 have taken the reference from here, from the base repository I mean.
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.
Let's delete the prefix then because it's confusing.
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.
Noted!
int height = binding.bottomSheetLayout.gestureLayout.getMeasuredHeight(); | ||
sheetBehavior.setPeekHeight(height); | ||
} | ||
}); | ||
sheetBehavior.setHideable(false); | ||
|
||
sheetBehavior.setBottomSheetCallback( |
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.
This method was deprecated. Please update.
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.
Done!
private static final Logger LOGGER = new Logger(); | ||
|
||
private static final int PERMISSIONS_REQUEST = 1; | ||
|
||
private static final String PERMISSION_CAMERA = Manifest.permission.CAMERA; | ||
protected int previewWidth = 0; | ||
protected int previewHeight = 0; | ||
|
||
private boolean debug = false; |
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.
Add a comment to explain what this is for.
protected byte[] getLuminance() { | ||
return yuvBytes[0]; | ||
} | ||
private void onStartCameraX(Size size, final int rotation) { |
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.
size
is always DESIRED_ANALYSIS_SIZE so you can use the value directly inside the method rather than passing in a parameter.
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.
Noted!
Hi @lintian06 and @khanhlvg |
Hi @khanhlvg and @lintian06 I have added the test. @farmaker47 helped me. |
Hello!
Camera
andCamera2
API implementations with fragments withCameraX
.data binding
as well.Image
toBitmap
Conversion fortf_support
test
code for the app