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

Record Video Functionality (Android Only) #587

Merged

Conversation

vipulsharma144
Copy link
Contributor

@vipulsharma144 vipulsharma144 commented Feb 12, 2020

Record Video
Save Video To cache
Stop Video Record and returns recorded video path

Android Only

@westonganger
Copy link
Collaborator

@vipulsharma144 this is looking pretty good. Can you resolve the conflicts?

@vipulsharma144
Copy link
Contributor Author

@westonganger Conflicts are resolved.

@westonganger
Copy link
Collaborator

This is a pretty large change. Do you have any additional comments before I start review?

@vipulsharma144
Copy link
Contributor Author

@westonganger not much to comment on this .Please help in the review .

Copy link
Collaborator

@westonganger westonganger left a comment

Choose a reason for hiding this comment

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

I have now added my comments.

Please ensure takeSnapshot functionality is still working with this PR. It seems your PR removed a bunch of the code for this.

@@ -1,4 +1,5 @@
# Cordova Plugin Camera Preview
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove all formatting that was applied to the README. Please include only add your additions.

README.md Outdated Show resolved Hide resolved
| FIXED | string | fixed | |
| AUTO | string | auto | |
| CONTINUOUS | string | continuous | IOS Only |
| Name | Type | Default | Note |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove the formatting on the tables. This new formatting makes it harder to edit the table.

plugin.xml Outdated Show resolved Hide resolved
plugin.xml Outdated Show resolved Hide resolved
src/ios/CameraSessionManager.m Outdated Show resolved Hide resolved
src/ios/CameraRenderController.m Outdated Show resolved Hide resolved
src/ios/CameraRenderController.m Outdated Show resolved Hide resolved
src/ios/CameraRenderController.m Outdated Show resolved Hide resolved
src/ios/CameraPreview.m Show resolved Hide resolved
plugin.xml Outdated Show resolved Hide resolved
console.log(characteristics);
});
```

### backgroundVideoStart(cb, [errorCallback])

_Currently this feature is for Android only._
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this actually android only? I am pretty sure the iOS is implemented too.


### backgroundVideoStop(cb, [errorCallback])

_Currently this feature is for Android only._
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this actually android only? I am pretty sure the iOS is implemented too.

Copy link

Choose a reason for hiding this comment

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

Was iOS ever confirmed?

Copy link
Collaborator

Choose a reason for hiding this comment

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

No it wasnt actually implemented. PR wanted for iOS support.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@westonganger
Copy link
Collaborator

@vipulsharma144 please follow up with my comments so we can keep the ball rolling on this PR.

@vipulsharma144
Copy link
Contributor Author

@westonganger whoa 🤩 !! Thanks for the detailed review I am on it . Will update asap.

@westonganger westonganger merged commit 4b0b780 into cordova-plugin-camera-preview:master Feb 18, 2020
@westonganger
Copy link
Collaborator

Alright this looks good enough now. Thanks @vipulsharma144 for your hard work on this PR.

@westonganger westonganger changed the title Record Video Functionality Record Video Functionality (Android Only) Feb 18, 2020
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