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

android-examples-recording-plus #11920

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

oddityyyy
Copy link

@oddityyyy oddityyyy commented Jun 17, 2023

What is the purpose of the change

Solve #9989 #10310 #10007 #8551 #5446,and give a unified and complete solution.
Make changes on the basis of the https://github.com/IntelRealSense/librealsense/tree/master/wrappers/android/examples/recording example in the Android sample program, introduce the openCV library, realize pressing the snapshot button, save the aligned color map, pseudo-color depth map, and grayscale depth map locally on the Android device, and use the getData method to save The depth data is a .csv file.

Does this pull request potentially affect existing recording functions?

No.
Snapshot and recording are two independent processes and do not affect each other. Users can choose to press the snapshot button to save the frame data at a certain moment to the Android local or press the recording button to record a complete bag file for a certain period of time according to their own needs.

…ple program, introduce the openCV library, realize pressing the snapshot button, save the aligned color map, pseudo-color depth map, and grayscale depth map locally on the Android device, and use the getData method to save The depth data is a .csv file.
@mengyui
Copy link
Contributor

mengyui commented Jun 21, 2023

Hi @oddityyyy
This PR is so big!

  1. please rebase your code base to the current development branch.
  2. too many codes have been changed outside of the Android wrapper folder. for example, csharp/dlib/matlab/nodejs/open3d/opencv/openvino/pcl/pointcloud/python/unrealengine4... some of them have been changed/removed in previous release version. I think you should not modify them.
  3. is it possible that move opencvlibray out of this repo? or add opencv using .gitsubmodule?
  4. both "capture" and "recording" are changed. but in the description of this PR, only one modified example is "recording". do you want to change all of them?
  5. which version of Android Studio are you using?

and one more question, why do saving .jpg files instead of extracting pictures from the recorded .bag file?

Thx.

@Nir-Az
Copy link
Collaborator

Nir-Az commented Jun 21, 2023

Hi @oddityyyy This PR is so big!

  1. please rebase your code base to the current development branch.
  2. too many codes have been changed outside of the Android wrapper folder. for example, csharp/dlib/matlab/nodejs/open3d/opencv/openvino/pcl/pointcloud/python/unrealengine4... some of them have been changed/removed in previous release version. I think you should not modify them.
  3. is it possible that move opencvlibray out of this repo? or add opencv using .gitsubmodule?
  4. both "capture" and "recording" are changed. but in the description of this PR, only one modified example is "recording". do you want to change all of them?
  5. which version of Android Studio are you using?

and one more question, why do saving .jpg files instead of extracting pictures from the recorded .bag file?

Thx.

Small comment about 3, we prefer using ExternalProject_Add on Cmake on top of submodule.

Copy link

@AndrewBloom AndrewBloom left a comment

Choose a reason for hiding this comment

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

Just scrolled through the first lines it seems not updated to the latest master....

static {
if(!OpenCVLoader.initDebug())
{
Log.d("opencv","初始化失败");

Choose a reason for hiding this comment

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

Log in Chinese??

@@ -1,3 +1,4 @@
<resources>
<string name="app_name">RS Capture</string>
<string name="message1">拍照</string>

Choose a reason for hiding this comment

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

Message in chinese???

@@ -1,14 +1,14 @@
apply plugin: 'com.android.application'

android {
compileSdkVersion 30
compileSdkVersion 28

Choose a reason for hiding this comment

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

Compile sdk should be the latest not an old one

versionCode 1
versionName "1.0"
testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner"
testInstrumentationRunner "android.support.test.runner.AndroidJUnitRunner"

Choose a reason for hiding this comment

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

Why this? androidx is now the preferred way

@@ -20,11 +20,11 @@ android {

dependencies {
implementation fileTree(dir: 'libs', include: ['*.jar'])
implementation 'androidx.appcompat:appcompat:1.2.0'
implementation 'androidx.constraintlayout:constraintlayout:2.0.4'
implementation 'com.android.support:appcompat-v7:28.0.0'

Choose a reason for hiding this comment

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

here too androidX is the preferred way

@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
<androidx.constraintlayout.widget.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android"
<android.support.constraint.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android"

Choose a reason for hiding this comment

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

AndroidX is the preferred way

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.

None yet

4 participants