nstudio / nativescript-camera-plus

MIT License
79 stars 50 forks source link

[WIP] NativeScript 7 Upgrade #137

Closed Codex- closed 3 years ago

Codex- commented 3 years ago

This PR is a work in progress, and has been created to give visibility on progress towards upgrading this package to support NativeScript 7.

Currently working on issues with the iOS implementation. But it launches now after much fighting :)

sublime392 commented 3 years ago

I am super excited about this, you have no idea. Is there anything I can do to help?

Codex- commented 3 years ago

I am super excited about this, you have no idea. Is there anything I can do to help?

Hey @sublime392

Glad you're looking forward to it, sorry it's taking a while! There's currently one issue i'm working on resolving, i'll commit what I can tonight/tomorrow.

The main issue i'm facing at the moment is that when you stop recording in iOS, there is a call made: UISaveVideoAtPathToSavedPhotosAlbum(recordingPath, this, 'videoDidFinishSavingWithErrorContextInfo', null);

Appears to crash, i'll post details later (work eating my time at the moment).

Codex- commented 3 years ago

Pushing most of what I have so far, currently when stopping a recording on iOS it crashes with the following:

***** Fatal JavaScript exception - application has been terminated. *****
NativeScript encountered a fatal error: Uncaught Error: MySwifty<0x1088e5800> does not respond to selector videoDidFinishSavingWithErrorContextInfo
at
module.exports.push.../../src/camera-plus.ts.MySwifty.recordingReady(file:///app/bundle.js:788:17)
at module.exports.push.../../src/camera-plus.ts.SwiftyDelegate.swiftyCamDidFinishProcessVideoAt(file:///app/bundle.js:569:27)
*** Terminating app due to uncaught exception 'NativeScript encountered a fatal error: Uncaught Error: MySwifty<0x1088e5800> does not respond to selector videoDidFinishSavingWithErrorContextInfo
at
module.exports.push.../../src/camera-plus.ts.MySwifty.recordingReady(file:///app/bundle.js:788:17)
at module.exports.push.../../src/camera-plus.ts.SwiftyDelegate.swiftyCamDidFinishProcessVideoAt(file:///app/bundle.js:569:27)
', reason: '(null)'

On the line: UISaveVideoAtPathToSavedPhotosAlbum(recordingPath, this, 'videoDidFinishSavingWithErrorContextInfo', null);

Codex- commented 3 years ago

A few more issues with iOS: The buttons like flash etc simply don't work or render on iOS but do get added as seen in logging.

_onLayoutChangeFn if the preview layer lines aren't commented, you get a hard crash: camera-plus.ios.ts:808

  private _onLayoutChangeFn(args: any) {
    const size = this.getActualSize();
    CLog(`xml width/height: ${size.width}x${size.height}`);
    const frame = this._swifty.view.frame;
    this._swifty.view.frame = CGRectMake(frame.origin.x, frame.origin.y, size.width, size.height);
    // this._swifty.previewLayer.frame = CGRectMake(frame.origin.x, frame.origin.y, size.width, size.height);
    this._swifty.view.setNeedsLayout();
    // this._swifty.previewLayer.setNeedsLayout();
  }

The crash:

CONSOLE LOG: NativeScript-CameraPlus --- [MySwifty viewDidAppear]
Received configuration update from daemon (initial)
CONSOLE LOG: NativeScript-CameraPlus --- [xml width/height: 414x342]
====== Assertion failed ======
Native stack trace:
1          0x10112e12c tns::Assert(bool, v8::Isolate*) + 128
2          0x1010ef0f8 tns::MetadataBuilder::RegisterStaticMethods(v8::Local<v8::Context>, v8::Local<v8::Function>, tns::BaseClassMeta const*, tns::KnownUnknownClassPair, robin_hood::detail::Table<true, 80ul, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, unsigned char, robin_hood::hash<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::equal_to<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > >&) + 476
3          0x1010ecf84 tns::MetadataBuilder::GetOrCreateConstructorFunctionTemplateInternal(v8::Local<v8::Context>, tns::BaseClassMeta const*, tns::KnownUnknownClassPair, robin_hood::detail::Table<true, 80ul, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, unsigned char, robin_hood::hash<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::equal_to<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > >&, robin_hood::detail::Table<true, 80ul, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, unsigned char, robin_hood::hash<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::equal_to<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > >&, std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::allocator<std::_<\M-b\M^@\M-&>
4          0x1010eb1d0 tns::MetadataBuilder::GetOrCreateConstructorFunctionTemplate(v8::Local<v8::Context>, tns::BaseClassMeta const*, tns::KnownUnknownClassPair, std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > > const&) + 88
5          0x1010959b0 std::__1::function<v8::Local<v8::FunctionTemplate> (v8::Local<v8::Context>, tns::BaseClassMeta const*, tns::KnownUnknownClassPair, std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > > const&)>::operator()(v8::Local<v8::Context>, tns::BaseClassMeta const*, tns::KnownUnknownClassPair, std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > > const&) const + 60
6          0x101092db4 tns::ArgConverter::CreateJsWrapper(v8::Local<v8::Context>, tns::BaseDataWrapper*, v8::Local<v8::Object>, bool, std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > > const&) + 1392
7          0x1011563e4 tns::Interop::GetResult(v8::Local<v8::Context>, tns::TypeEncoding const*, tns::BaseCall*, bool, std::__1::shared_ptr<v8::Persistent<v8::Value, v8::NonCopyablePersistentTraits<v8::Value> > >, bool, bool, bool, bool) + 3708
8          0x101152614 tns::Interop::CallFunctionInternal(tns::MethodCall&) + 496
9          0x101092224 tns::ArgConverter::Invoke(v8::Local<v8::Context>, objc_class*, v8::Local<v8::Object>, tns::V8Args&, tns::MethodMeta const*, bool) + 780
10         0x1010f0100 tns::MetadataBuilder::InvokeMethod(v8::Local<v8::Context>, tns::MethodMeta const*, v8::Local<v8::Object>, tns::V8Args&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, bool) + 88
11         0x1010efbf4 tns::MetadataBuilder::PropertyGetterCallback(v8::FunctionCallbackInfo<v8::Value> const&) + 252
12         0x10126be10 v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) + 536
13         0x10126b3dc v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) + 516
14         0x10126add0 v8::internal::Builtins::InvokeApiFunction(v8::internal::Isolate*, bool, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*, v8::internal::Handle<v8::internal::HeapObject>) + 492
15         0x101591214 v8::internal::Object::GetPropertyWithAccessor(v8::internal::LookupIterator*) + 372
16         0x101590a6c v8::internal::Object::GetProperty(v8::internal::LookupIterator*, bool) + 140
17         0x1014247c8 v8::internal::LoadIC::Load(v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Name>, bool) + 1428
18         0x10142cc64 v8::internal::Runtime_LoadNoFeedbackIC_Miss(int, unsigned long*, v8::internal::Isolate*) + 216
19         0x1019fcc4c Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_NoBuiltinExit + 108
20         0x101a6f778 Builtins_LdaNamedPropertyHandler + 4408
21         0x1019956b4 Builtins_InterpreterEntryTrampoline + 244
22         0x1019956b4 Builtins_InterpreterEntryTrampoline + 244
23         0x1019956b4 Builtins_InterpreterEntryTrampoline + 244
24         0x1019956b4 Builtins_InterpreterEntryTrampoline + 244
25         0x1019956b4 Builtins_InterpreterEntryTrampoline + 244
26         0x1019956b4 Builtins_InterpreterEntryTrampoline + 244
27         0x1019956b4 Builtins_InterpreterEntryTrampoline + 244
28         0x1019956b4 Builtins_InterpreterEntryTrampoline + 244
29         0x10198e504 Builtins_ArgumentsAdaptorTrampoline + 228
30         0x1019956b4 Builtins_InterpreterEntryTrampoline + 244
31         0x10198e504 Builtins_ArgumentsAdaptorTrampoline + 228
32         0x1019956b4 Builtins_InterpreterEntryTrampoline + 244
33         0x1019956b4 Builtins_InterpreterEntryTrampoline + 244
34         0x1019956b4 Builtins_InterpreterEntryTrampoline + 244
35         0x10198e504 Builtins_ArgumentsAdaptorTrampoline + 228
36         0x1019956b4 Builtins_InterpreterEntryTrampoline + 244
37         0x10198e504 Builtins_ArgumentsAdaptorTrampoline + 228
38         0x1019956b4 Builtins_InterpreterEntryTrampoline + 244
39         0x1019956b4 Builtins_InterpreterEntryTrampoline + 244
40         0x1019956b4 Builtins_InterpreterEntryTrampoline + 244
41         0x10198e504 Builtins_ArgumentsAdaptorTrampoline + 228
42         0x1019956b4 Builtins_InterpreterEntryTrampoline + 244
43         0x10198e504 Builtins_ArgumentsAdaptorTrampoline + 228
44         0x1019956b4 Builtins_InterpreterEntryTrampoline + 244
45         0x1019956b4 Builtins_InterpreterEntryTrampoline + 244
46         0x1019956b4 Builtins_InterpreterEntryTrampoline + 244
47         0x10198e504 Builtins_ArgumentsAdaptorTrampoline + 228
48         0x1019956b4 Builtins_InterpreterEntryTrampoline + 244
49         0x1019956b4 Builtins_InterpreterEntryTrampoline + 244
50         0x101992f64 Builtins_JSEntryTrampoline + 164
51         0x101992c08 Builtins_JSEntry + 168
52         0x143b0c000 52  ???                                 0x0000000143b0c000 0x0 + 5430624256
sublime392 commented 3 years ago

Hi @Codex- Although I am rather lost when it comes to iOS stuff, I pulled your branch and started mucking about. I got past the crash when stopping video, but I don't know if this helps you because it might be the wrong thing to do. I noticed that the UIImageWriteToSavedPhotosAlbum has the same type of signature as the UISaveVideoAtPathToSavedPhotosAlbum, and that null was being passed for most of the parameters of UIImageWriteToSavedPhotosAlbum. I did the same thing for the video save method (UISaveVideoAtPathToSavedPhotosAlbum(recordingPath, null, null, null);, and called this._owner.get().sendEvent(CameraPlus.videoRecordingReadyEvent, path); directly after. It works, but presumably there could be an undetected error while saving the video if doing it this way?

sublime392 commented 3 years ago

Also, I tried to dig deeper into the preview layer crash. I don't know if you noticed, but simply trying to reference the preview layer results in a crash. Trying to do something like const preview = this._swifty.previewLayer; results in crash. I tried getting it from the this._swifty.view.subviews with same result. I can log this._swifty.view.subviews, and see the preview layer as the first item in the list. Any attempt to access it results in crash. const view = this._swifty.view.subviews.objectAtIndex(0); crashes. Even trying to iterate over the subviews results in a crash unless you start at index 1 instead of zero.

Codex- commented 3 years ago

Perhaps @NathanWalker might be able to identify if the subviews problem is a NS level bug or not?

I'll keep digging when I get time soon!

sublime392 commented 3 years ago

btw, I have been using this in my project without the subview lines, and it is working well (camera only, I don't use the video recording feature).

NathanWalker commented 3 years ago

@Codex- Thank you for all this - We have a few improvements from v6 version we had been merging in over past couple weeks to port into this and will move this into our plugin monorepo here by the New Year and add you to the contributors listing: https://github.com/nstudio/nativescript-plugins

We can help look into the remaining issues you mentioned above.

Codex- commented 3 years ago

@NathanWalker Sounds great! If you'd like, once you've got master ready to port over I can rebase this branch to latest master and then open a PR with the changes over on the plugins repo? There are quite a few things in the repo i'd like to improve (mainly around typings and consistent API usage etc) but wanted to get a stable release done with the current implementation first :)

I actually had a look at this more and have only been hitting the same crash over and over, was wondering if it was in the NS layer but I haven't been able to consistently determine the issue

NathanWalker commented 3 years ago

@Codex- that'd be great - If you get a chance to rebase (or merge - we just rebase PR's into master but totally fine to just merge upstream/master into your fork) and resolve the listed conflicts below here to update this PR to latest master I can then drop the latest from this PR into the plugins monorepo where we can finish off the remaining items over there.

Also if you'd like to invite NathanWalker as collaborator with push rights to your fork I'd be happy to update your forked master to help resolve conflicts as well.

This repo can stand for v6 {N} compat going into the future and we'll manage all {N} 7 updates and beyond in the new monorepo.

Codex- commented 3 years ago

Working through a rebase at the moment.

Master currently crashes on Android 10 with the merged changes:

const nativeSizes: any = this._camera.getAvailablePictureSizes(ratio);
for (const size of nativeSizes) {
  sizes.push({ width: size.getWidth(), height: size.getHeight() });
}

and found the same for the ratios iterator too.

Also why is the upstream method for getting supported ratios getGet 😂

triniwiz commented 3 years ago

😂 Typo

Sent from Yahoo Mail for iPhone

On Saturday, December 26, 2020, 11:42 PM, Alex Miller notifications@github.com wrote:

Working through a rebase at the moment.

Master currently crashes on Android 10 with the merged changes: const nativeSizes: any = this._camera.getAvailablePictureSizes(ratio); for (const size of nativeSizes) { sizes.push({ width: size.getWidth(), height: size.getHeight() }); } and found the same for the ratios iterator too.

Also why is the upstream method for getting supported rations getGet 😂

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

Codex- commented 3 years ago

Also discovered a bug in the latest NS cli, if you have no dev team set in the build.xcconfig and you select a team you get a nice little Error is: Cannot read property 'id' of undefined.. Manually setting it works but a bit cumbersome.

Codex- commented 3 years ago

@NathanWalker Rebased and updated.

You still have to comment out these to get iOS to run though:

private _onLayoutChangeFn(args: any) {
  const size = this.getActualSize();
  CLog(`xml width/height: ${size.width}x${size.height}`);
  const frame = this._swifty.view.frame;
  this._swifty.view.frame = CGRectMake(frame.origin.x, frame.origin.y, size.width, size.height);
  // this._swifty.previewLayer.frame = CGRectMake(frame.origin.x, frame.origin.y, size.width, size.height);
  this._swifty.view.setNeedsLayout();
  // this._swifty.previewLayer.setNeedsLayout();
}
Codex- commented 3 years ago

Following the move to the monorepo i'd be keen to do a lot of things to improve the experience for this plugin for consumers, so just tag me when we're ready to get started

NathanWalker commented 3 years ago

Thanks @Codex- I'll plan on starting the move by Monday and will ping back when ready for us to continue collaborating there.

NathanWalker commented 3 years ago

Ok @Codex- I have it now here on master: https://github.com/nstudio/nativescript-plugins

npm run setup
npm start

And should be able to build it and/or demo it (as usual the camera needs to be tested on device so you can also cd apps/demo and just run on a device

Codex- commented 3 years ago

@NathanWalker great cheers!

Before I close this off, I was going to open some issues on the monorepo to track them as they are currently, does this make sense to you? That way we can close up the main remaining issues and can move on to some quality of life changes.

msn444 commented 3 years ago

Hi guys, thanks for your efforts on getting this done. When do you think it will be published to NPM?

Codex- commented 3 years ago

@msn444 you can target this branch directly via npm although there are pending issues still to do with recording on iOS.

msn444 commented 3 years ago

@msn444 you can target this branch directly via npm although there are pending issues still to do with recording on iOS.

Thanks @Codex- , but I get npm ERR! Can't install https://github.com/Codex-/nativescript-camera-plus/tarball/master: Missing package version when I do that.

Codex- commented 3 years ago

@msn444 hi, please attempt to use the latest version from npm, and open an issue for any issues you encounter on the new monorepo here: https://github.com/nstudio/nativescript-plugins/issues

Codex- commented 3 years ago

This PR and this project in general are now offered from the nativescript plugin monorepo here: https://github.com/nstudio/nativescript-plugins

Please open any issues encountered there, this PR has been used as the base for nativescript-camera-plus on that repository.