Ios video logic #86

parveshneedhoo · 2024-09-05T08:32:21Z

No description provided.

…plugin-simple-camera-preview into ios-video-logic

…mple-camera-preview into ios-video-logic

parveshneedhoo · 2024-09-12T07:05:46Z

src/ios/SimpleCameraPreview.m

+        [self.sessionManager startRecordingToOutputFileURL:fileURL recordingDelegate:self];
+    } else {
+       CDVPluginResult *pluginResult = [CDVPluginResult resultWithStatus:CDVCommandStatus_ERROR messageAsString:@"Session not initialized or already recording"];
+       [self.commandDelegate sendPluginResult:pluginResult callbackId:command.callbackId];


setKeepCallback true

same in the other methods

we keep it same logic as with Androids and the single callback

src/ios/SimpleCameraPreview.m

parveshneedhoo · 2024-09-12T07:10:31Z

src/ios/SimpleCameraPreview.m

+    CMTime time = CMTimeMakeWithSeconds(1.0, 600);
+    NSError *error = nil;
+    CMTime actualTime;
+        CGImageRef imageRef = [imageGenerator copyCGImageAtTime:time actualTime:&actualTime error:&error];


Suggested change

CGImageRef imageRef = [imageGenerator copyCGImageAtTime:time actualTime:&actualTime error:&error];

CGImageRef imageRef = [imageGenerator copyCGImageAtTime:time actualTime:&actualTime error:&error];

src/ios/SimpleCameraPreview.m

parveshneedhoo · 2024-09-12T07:17:38Z

src/ios/SimpleCameraPreview.m

+    }
+
+    UIImage *thumbnail = [[UIImage alloc] initWithCGImage:imageRef];
+        CGImageRelease(imageRef);


Suggested change

CGImageRelease(imageRef);

CGImageRelease(imageRef);

src/ios/SimpleCameraPreview.m

parveshneedhoo · 2024-09-12T07:22:37Z

src/ios/SimpleCameraPreview.m

+    } else {
+        NSString *thumbnail = [self generateThumbnailForVideoAtURL:outputFileURL];
+        NSString *filePath = [outputFileURL path];
+        NSDictionary *result = @{@"nativePath": filePath, @"thumbnail": thumbnail};


check if we actually have the thumbnail generated before adding it to the dictionary?

we can return null if we get errors from the generateThumbnailForVideoAtURL method

Co-authored-by: Parvesh Needhoo <116141944+parveshneedhoo@users.noreply.github.com>

…plugin-simple-camera-preview into ios-video-logic

sc-nick

good for me

HashirRajah

I have a few comments but it can all be treated as improvements/refactoring for later.

Everything seems to work except

when I start recording on iOS and put the app in background, it does not stop recording and when opening the app again timer stops at 0 but stays there.

HashirRajah · 2024-09-27T05:36:11Z

src/ios/CameraSessionManager.h

@@ -12,6 +12,8 @@
 - (BOOL) deviceHasUltraWideCamera;
 - (void) deallocSession;
 - (void) updateOrientation:(AVCaptureVideoOrientation)orientation;
+- (void) startRecordingToOutputFileURL:(NSURL *)fileURL recordingDelegate:(id<AVCaptureFileOutputRecordingDelegate>)recordingDelegate;


Suggested change

- (void) startRecordingToOutputFileURL:(NSURL *)fileURL recordingDelegate:(id<AVCaptureFileOutputRecordingDelegate>)recordingDelegate;

- (void) startRecording:(NSURL *)fileURL recordingDelegate:(id<AVCaptureFileOutputRecordingDelegate>)recordingDelegate;

Can this method be renamed to startRecording ?

HashirRajah · 2024-09-27T05:44:25Z

src/ios/CameraSessionManager.m

+
+                AVCaptureDevice *audioDevice = [AVCaptureDevice defaultDeviceWithMediaType:AVMediaTypeAudio];
+                NSError *audioError = nil;
+                AVCaptureDeviceInput *audioInput = [AVCaptureDeviceInput deviceInputWithDevice:audioDevice error:&audioError];


Should audioInput be declared in the header file like videoDeviceInput?

HashirRajah · 2024-09-27T05:47:07Z

src/ios/CameraSessionManager.m

+                AVCaptureDevice *audioDevice = [AVCaptureDevice defaultDeviceWithMediaType:AVMediaTypeAudio];
+                NSError *audioError = nil;
+                AVCaptureDeviceInput *audioInput = [AVCaptureDeviceInput deviceInputWithDevice:audioDevice error:&audioError];
+                if (audioInput && [self.session canAddInput:audioInput]) {


Suggested change

if (audioInput && [self.session canAddInput:audioInput]) {

if ([self.session canAddInput:audioInput]) {

Will this be enough? When looking at similar code blocks, only this check is done. For example on line 93:

if ([self.session canAddInput:videoDeviceInput]) { [self.session addInput:videoDeviceInput]; self.videoDeviceInput = videoDeviceInput; }

HashirRajah · 2024-09-27T05:51:48Z

src/ios/CameraSessionManager.m

@@ -10,6 +10,7 @@ - (CameraSessionManager *)init {
            [self.session setSessionPreset:AVCaptureSessionPresetPhoto];
        }
        self.filterLock = [[NSLock alloc] init];
+        self.movieFileOutput = [[AVCaptureMovieFileOutput alloc] init];


Is there a reason why this line is not done in the setupSession function instead?

Im taking the example of imageOutput. The allocation and checks are all done in setupSession on line 98:

AVCapturePhotoOutput *imageOutput = [[AVCapturePhotoOutput alloc] init]; if ([self.session canAddOutput:imageOutput]) { [self.session addOutput:imageOutput]; self.imageOutput = imageOutput; }

HashirRajah · 2024-09-27T05:57:53Z

src/ios/CameraSessionManager.m

+    if (!self.movieFileOutput.isRecording) {
+        AVCaptureConnection *connection = [self.movieFileOutput connectionWithMediaType:AVMediaTypeVideo];
+        if ([connection isVideoOrientationSupported]) {
+            connection.videoOrientation = [self getCurrentOrientation];
+        }
+        [self.movieFileOutput startRecordingToOutputFileURL:fileURL recordingDelegate:recordingDelegate];


Can we do an early return?

if (self.movieFileOutput.isRecording) return; // logic to start recording

HashirRajah · 2024-09-27T06:18:23Z

src/ios/SimpleCameraPreview.m

+}
+
+- (void)startVideoCapture:(CDVInvokedUrlCommand*)command {
+    if (self.sessionManager != nil && !self.sessionManager.movieFileOutput.isRecording) {


I think we can invert the condition and make an early return instead of having an if and an else block

if (self.sessionManager == nil || self.sessionManager.movieFileOutput.isRecording) { CDVPluginResult *pluginResult = [CDVPluginResult resultWithStatus:CDVCommandStatus_ERROR messageAsString:@"Session not initialized or already recording"]; [self.commandDelegate sendPluginResult:pluginResult callbackId:command.callbackId]; return; } // other logic

The condition should be an || condition when reversing.

HashirRajah · 2024-09-27T06:20:47Z

src/ios/SimpleCameraPreview.m

+}
+
+- (void)stopVideoCapture:(CDVInvokedUrlCommand*)command {
+    if (self.sessionManager != nil && self.sessionManager.movieFileOutput.isRecording) {


Same comments for the if part as above.

HashirRajah · 2024-09-27T06:22:24Z

src/ios/SimpleCameraPreview.m

+    if (error) {
+        CDVPluginResult *pluginResult = [CDVPluginResult resultWithStatus:CDVCommandStatus_ERROR messageAsString:error.localizedDescription];
+        [self.commandDelegate sendPluginResult:pluginResult callbackId:self.videoCallbackContext.callbackId];
+    } else {


Can add a return in the if instead of an else block.

HashirRajah · 2024-09-27T06:27:29Z

src/ios/SimpleCameraPreview.m

+    } else {
+        NSString *thumbnail = [self generateThumbnailForVideoAtURL:outputFileURL];
+        NSString *filePath = [outputFileURL path];
+        NSDictionary *result = @{@"nativePath": filePath, @"thumbnail": thumbnail};


Maybe we can remove recording in the onStop for Android too.

ios video logic

9541f35

parveshneedhoo self-assigned this Sep 5, 2024

YushraJewon changed the base branch from master to video-ios-poc September 5, 2024 09:47

use correct directory

d7341ae

parveshneedhoo changed the base branch from video-ios-poc to master September 5, 2024 10:23

parveshneedhoo and others added 6 commits September 5, 2024 14:36

remove logs

279743a

Merge branch 'ios-video-logic' of github.com:spoonconsulting/cordova-…

57af3b3

…plugin-simple-camera-preview into ios-video-logic

add audio device

32dd7cb

generate thumbnail

7ac1815

Merge branch 'ios-video-logic' of github.com:spoonconsulting/cordova-…

d37631a

…plugin-simple-camera-preview into ios-video-logic

Merge branch 'master' of github.com:spoonconsulting/cordova-plugin-si…

933b35d

…mple-camera-preview into ios-video-logic

parveshneedhoo commented Sep 12, 2024

View reviewed changes

parveshneedhoo assigned YushraJewon Sep 12, 2024

YushraJewon and others added 4 commits September 12, 2024 14:31

return null if there is thumbnail error

cfc7057

Apply suggestions from code review

cf831ce

Co-authored-by: Parvesh Needhoo <116141944+parveshneedhoo@users.noreply.github.com>

change null to nil

5b9aa6e

Merge branch 'ios-video-logic' of github.com:spoonconsulting/cordova-…

25eac64

…plugin-simple-camera-preview into ios-video-logic

YushraJewon requested review from sc-nick and HashirRajah September 24, 2024 06:57

sc-nick reviewed Sep 26, 2024

View reviewed changes

HashirRajah reviewed Sep 27, 2024

View reviewed changes

YushraJewon added 3 commits October 1, 2024 10:30

stop recording if app go in background

a4e07db

check ig sessionManager is running before stopRecording

6a864f4

bump version 2.0.28

c360b81

YushraJewon merged commit 1e24fd9 into master Oct 3, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ios video logic #86

Ios video logic #86

parveshneedhoo commented Sep 5, 2024

parveshneedhoo Sep 12, 2024

YushraJewon Sep 12, 2024

parveshneedhoo Sep 12, 2024

parveshneedhoo Sep 12, 2024

parveshneedhoo Sep 12, 2024

sc-nick left a comment

HashirRajah left a comment

HashirRajah Sep 27, 2024

HashirRajah Sep 27, 2024 •

edited

Loading

HashirRajah Sep 27, 2024

HashirRajah Sep 27, 2024

HashirRajah Sep 27, 2024

HashirRajah Sep 27, 2024 •

edited

Loading

HashirRajah Sep 27, 2024 •

edited

Loading

HashirRajah Sep 27, 2024

HashirRajah Sep 27, 2024

HashirRajah Sep 27, 2024

	CGImageRef imageRef = [imageGenerator copyCGImageAtTime:time actualTime:&actualTime error:&error];
	CGImageRef imageRef = [imageGenerator copyCGImageAtTime:time actualTime:&actualTime error:&error];

	- (void) startRecordingToOutputFileURL:(NSURL *)fileURL recordingDelegate:(id<AVCaptureFileOutputRecordingDelegate>)recordingDelegate;
	- (void) startRecording:(NSURL *)fileURL recordingDelegate:(id<AVCaptureFileOutputRecordingDelegate>)recordingDelegate;

	if (audioInput && [self.session canAddInput:audioInput]) {
	if ([self.session canAddInput:audioInput]) {

Ios video logic #86

Ios video logic #86

Conversation

parveshneedhoo commented Sep 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sc-nick left a comment

Choose a reason for hiding this comment

HashirRajah left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HashirRajah Sep 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HashirRajah Sep 27, 2024 • edited Loading

Choose a reason for hiding this comment

HashirRajah Sep 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HashirRajah Sep 27, 2024 •

edited

Loading

HashirRajah Sep 27, 2024 •

edited

Loading

HashirRajah Sep 27, 2024 •

edited

Loading