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

Implemented CameraX and Data Binding in Object Detection App #341

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

sayannath
Copy link

@sayannath sayannath commented Aug 7, 2021

Hello!

  • I have replaced the Camera and Camera2 API implementations with fragments with CameraX.
  • Implemented data binding as well.
  • Refactored the code according to that
  • Managed Image to Bitmap Conversion for tf_support
  • Modified test code for the app

@google-ml-butler google-ml-butler bot added size:XL CL Change Size: Extra Large awaiting review labels Aug 7, 2021
@google-cla google-cla bot added the cla: yes CLA has been signed label Aug 7, 2021
@sayannath sayannath changed the title Implemented CameraX and Data Binding Implemented CameraX and Data Binding in Object Detection App Aug 8, 2021
@@ -14,151 +14,150 @@
* limitations under the License.
*/

package org.tensorflow.lite.examples.detection;

Copy link
Contributor

@lintian06 lintian06 Aug 10, 2021

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.

Choose a reason for hiding this comment

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

Copy link
Author

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;
Copy link
Contributor

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.

Copy link
Author

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);
Copy link
Contributor

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.

Copy link
Author

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()
Copy link
Contributor

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()));
Copy link
Contributor

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) {
Copy link
Contributor

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(
Copy link
Contributor

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(
Copy link
Contributor

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: {
Copy link
Contributor

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.

Copy link
Author

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() {
Copy link
Contributor

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
Copy link
Contributor

@lintian06 lintian06 Aug 10, 2021

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.

Copy link
Author

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 =
Copy link
Contributor

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.

Copy link
Author

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) {
Copy link
Contributor

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) {
}

Copy link
Contributor

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(
Copy link
Contributor

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
Copy link
Contributor

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 -> {
Copy link
Contributor

@lintian06 lintian06 Aug 10, 2021

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) {
Copy link
Contributor

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();
Copy link
Contributor

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,
Copy link
Contributor

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))
Copy link
Contributor

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);
Copy link
Contributor

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);
Copy link
Contributor

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)

Copy link
Contributor

@lintian06 lintian06 left a 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;
Copy link
Member

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.

Choose a reason for hiding this comment

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

Copy link
Author

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();
Copy link
Member

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"
Copy link
Member

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

Copy link
Author

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"
Copy link
Member

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.

Copy link
Author

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;
Copy link
Member

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;
Copy link
Member

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.

Copy link
Author

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.

Copy link
Member

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.

Copy link
Author

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";
Copy link
Member

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.

Copy link
Author

@sayannath sayannath Aug 10, 2021

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.

Copy link
Member

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.

Copy link
Author

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(
Copy link
Member

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.

Copy link
Author

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;
Copy link
Member

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.

@khanhlvg
Copy link
Member

khanhlvg commented Aug 11, 2021

interpreter build variants doesn't build. Please fix the build error and all warnings.
image

protected byte[] getLuminance() {
return yuvBytes[0];
}
private void onStartCameraX(Size size, final int rotation) {
Copy link
Member

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.

Copy link
Author

Choose a reason for hiding this comment

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

Noted!

@sayannath
Copy link
Author

sayannath commented Aug 12, 2021

WhatsApp Image 2021-08-12 at 10 52 33 AM

Hi @lintian06 and @khanhlvg
I have attached the screenshot of the app.

@sayannath
Copy link
Author

sayannath commented Aug 13, 2021

Hi @khanhlvg and @lintian06 I have added the test. @farmaker47 helped me.

Screenshot 2021-08-14 at 12 24 41 AM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review cla: yes CLA has been signed size:XL CL Change Size: Extra Large
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants