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

Video capture #69

Closed
wants to merge 18 commits into from
Closed

Video capture #69

wants to merge 18 commits into from

Conversation

YushraJewon
Copy link
Contributor

Allow plugin to capture video.

@YushraJewon YushraJewon self-assigned this Mar 21, 2024
@@ -24,8 +28,18 @@
import androidx.camera.core.ImageCaptureException;
import androidx.camera.core.Preview;
import androidx.camera.lifecycle.ProcessCameraProvider;
import androidx.camera.video.FileOutputOptions;
import androidx.camera.video.MediaStoreOutputOptions;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
import androidx.camera.video.MediaStoreOutputOptions;

return true;
}

fragment.captureVideo( useFlash, new VideoCallback() {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
fragment.captureVideo( useFlash, new VideoCallback() {
fragment.captureVideo(useFlash, new VideoCallback() {

Comment on lines 268 to 272
if (torchActivated) {
useFlash = true;
} else {
camera.getCameraControl().enableTorch(useFlash);
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (torchActivated) {
useFlash = true;
} else {
camera.getCameraControl().enableTorch(useFlash);
}
if (torchActivated) {
useFlash = true;
} else {
camera.getCameraControl().enableTorch(useFlash);
}

Some lines not indented properly xD

} catch (IllegalArgumentException e) {
// Error with result in capturing image with default resolution
e.printStackTrace();
this.getActivity().runOnUiThread(() -> {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
this.getActivity().runOnUiThread(() -> {
this.getActivity().runOnUiThread(() -> {

Why do we need to run this on UI thread?

.withAudioEnabled()
.start(ContextCompat.getMainExecutor(this.getContext()), videoRecordEvent -> {
if (videoRecordEvent instanceof VideoRecordEvent.Start) {
videoCallback.onStart(null,true, null);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
videoCallback.onStart(null,true, null);
videoCallback.onStart(null, true, null);

@@ -102,9 +124,9 @@ public View onCreateView(@NonNull LayoutInflater inflater, @Nullable ViewGroup c
return containerView;
}


Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

);
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

}
}

public void onStop(Exception err, Boolean started, String nativePath) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public void onStop(Exception err, Boolean started, String nativePath) {
public void onStop(Exception err, Boolean recording, String nativePath) {

The parameter is named recording in the interface. Should we keep the name as recording?

// Handle video saved
videoCallback.onStop(null, false, Uri.fromFile(videoFile).toString());
Uri savedUri = finalizeEvent.getOutputResults().getOutputUri();
Log.d(TAG, "Video saved to: " + savedUri);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Log.d(TAG, "Video saved to: " + savedUri);
Log.i(TAG, "Video saved to: " + savedUri);

Comment on lines 68 to 69
void onStart(Exception err,Boolean recording, String nativePath);
void onStop(Exception err, Boolean recording, String nativePath);
Copy link
Member

Choose a reason for hiding this comment

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

We are not using the parameter Exception err when defining the callbacks. Should we remove it?

Comment on lines 281 to 285
if (torchActivated) {
useFlash = true;
} else {
camera.getCameraControl().enableTorch(useFlash);
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (torchActivated) {
useFlash = true;
} else {
camera.getCameraControl().enableTorch(useFlash);
}
if (!torchActivated) {
camera.getCameraControl().enableTorch(useFlash);
}

This will work right?
In the if block, useFlash is being reassigned but we the argument is used only in the else block.

@@ -258,6 +277,54 @@ public void hasFlash(HasFlashCallback hasFlashCallback) {
hasFlashCallback.onResult(camera.getCameraInfo().hasFlashUnit());
}

public void captureVideo(VideoCallback videoCallback) {
if (recording != null) {
recording.stop();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
recording.stop();
recording.stop();
recording = null;

Comment on lines 288 to 289
ContentValues contentValues = new ContentValues();
contentValues.put(MediaStore.Video.Media.DISPLAY_NAME, filename);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ContentValues contentValues = new ContentValues();
contentValues.put(MediaStore.Video.Media.DISPLAY_NAME, filename);
ContentValues contentValues = new ContentValues();
contentValues.put(MediaStore.Video.Media.DISPLAY_NAME, filename);

not used

ActivityCompat.requestPermissions(this.getActivity(), new String[]{Manifest.permission.RECORD_AUDIO}, 200);
}
File videoFile = new File(
getContext().getApplicationContext().getFilesDir(),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
getContext().getApplicationContext().getFilesDir(),
getContext().getFilesDir(),

Log.e(TAG, "Video recording error: " + errorCode, errorCause);
} else {
// Handle video saved
videoCallback.onStop(false, Uri.fromFile(videoFile).toString());
Copy link
Member

Choose a reason for hiding this comment

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

Better to move this line after the log(line 317)

Copy link
Contributor

@parveshneedhoo parveshneedhoo left a comment

Choose a reason for hiding this comment

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

For audio permission deny part, we can treat it same as we do for image picker.

Before starting video capture :
1)we first request permission
2)then we check if it has permission
3) if it has, we start recording withAudioEnabled else start it without

videoCallback.onStop(false, Uri.fromFile(videoFile).toString());
Uri savedUri = finalizeEvent.getOutputResults().getOutputUri();
Log.i(TAG, "Video saved to: " + savedUri);
recording = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

move this on line 325? since we are not setting the recording to null if finalizeEvent.hasError()

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.

4 participants