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

[V2] Correct Frame Processor loading between Objc/Swift #2025

Merged
merged 7 commits into from
Oct 18, 2023

Conversation

iBotPeaches
Copy link

@iBotPeaches iBotPeaches commented Oct 17, 2023

What

This PR fixes Frame Processors (Swift/Objc). I used the following for research:

Doing initialize appeared to be too early (or not consistent) as we might load code before reanimated is ready. When we move to "load" - that is no longer allowed on Swift due to the interop between objc. attribute constructor runs after load and seems the best place.

Changes

  • Corrects frame processor loading for objc/swift based processors.

Tested on

  • iPhone 13 Pro

Related issues

@vercel
Copy link

vercel bot commented Oct 17, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-native-vision-camera ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 17, 2023 4:05pm

@iBotPeaches iBotPeaches changed the title [Draft] [V2] Correct Frame Processor loading between Objc/Swift [V2] Correct Frame Processor loading between Objc/Swift Oct 17, 2023
@iBotPeaches
Copy link
Author

Okay, this works in the Example and resolves my issues on v2 with the frame processors now.

vision-camera.mp4

@iBotPeaches iBotPeaches marked this pull request as ready for review October 17, 2023 13:35
Copy link
Owner

@mrousavy mrousavy left a comment

Choose a reason for hiding this comment

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

Hey thanks for your PR!

Just left a few comments, otherwise we're good to go!

example/src/CameraPage.tsx Outdated Show resolved Hide resolved
@@ -33,7 +33,7 @@
*/
#define VISION_EXPORT_FRAME_PROCESSOR(frame_processor) \
\
+(void)initialize \
+(void)load \
Copy link
Owner

Choose a reason for hiding this comment

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

Great!

@@ -53,13 +53,11 @@ objc_name : NSObject<FrameProcessorPluginBase>
@end \
@implementation objc_name (FrameProcessorPlugin) \
\
+(void)initialize \
__attribute__((constructor)) static void VISION_CONCAT(initialize_, objc_name)() \
Copy link
Owner

Choose a reason for hiding this comment

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

Ahh, yea I forgot about that lol. I dont know why I reverted that

@cseluzha
Copy link

Thanks for this changes, you saved me! Unfortunately I can't use version 3 yet because I have to readjust some internal things in my code due to the change from ImageProxy to Frame, in custom frame processor.

I just added them locally, I will wait for them to merge in v2 to update my project.

@mrousavy mrousavy merged commit f5b6d7e into mrousavy:v2 Oct 18, 2023
3 of 5 checks passed
@mrousavy
Copy link
Owner

Let's go!

@iBotPeaches iBotPeaches deleted the fix-frame-processors branch October 18, 2023 11:58
@iBotPeaches
Copy link
Author

Thanks. Will await the new tag and update our plugin/project.

@mrousavy
Copy link
Owner

Will await the new tag and update our plugin/project

Just released an update on npm with this fix 💪

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.

3 participants