feat: style transfer with openCV(ios) #48

NorbertKlockiewicz · 2024-12-10T16:36:30Z

Description

This PR introduces implementation for style transfer module which is using opencv under the hood, it allows to load images from external sources, device storage and base64.

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update (improves or adds clarity to existing documentation)

Tested on

iOS
Android

Testing instructions

Screenshots

Related issues

Checklist

I have performed a self-review of my code
I have commented my code, particularly in hard-to-understand areas
I have updated the documentation accordingly
My changes generate no new warnings

Additional notes

## Description This pull request introduces example app for computer vision task, which is similar to llama example, for now it only has one screen implemented(Style Transfer) ### Type of change - [ ] Bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] Documentation update (improves or adds clarity to existing documentation) ### Tested on - [x] iOS - [x] Android ### Testing instructions  ### Screenshots  ### Related issues  ### Checklist - [x] I have performed a self-review of my code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have updated the documentation accordingly - [x] My changes generate no new warnings ### Additional notes

… project

…d image resource from base64, storage and external sources

## Description This pull request introduces example app for computer vision task, which is similar to llama example, for now it only has one screen implemented(Style Transfer) ### Type of change - [ ] Bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] Documentation update (improves or adds clarity to existing documentation) ### Tested on - [x] iOS - [x] Android ### Testing instructions  ### Screenshots  ### Related issues  ### Checklist - [x] I have performed a self-review of my code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have updated the documentation accordingly - [x] My changes generate no new warnings ### Additional notes

## Description Turbo module which was handling running of llama models was named RnExecutorchModule which wasn't really clear and conflicted with name of library, it's now changed to LLM, matching the style of StyleTransfer, ObjectDetection etc. ### Type of change - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] Documentation update (improves or adds clarity to existing documentation) ### Tested on - [x] iOS - [x] Android ### Testing instructions  ### Screenshots  ### Related issues  ### Checklist - [x] I have performed a self-review of my code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have updated the documentation accordingly - [ ] My changes generate no new warnings ### Additional notes

…ation

mkopcins · 2024-12-15T15:34:31Z

src/StyleTransfer.ts

+        setIsModelReady(false);
        await StyleTransfer.loadModule(path);
      } catch (e) {
        setError(getError(e));
      } finally {
-        setIsModelLoading(false);
+        setIsModelReady(true);


This looks a bit off to me, why do we set isModelReady to true even if an error occurs? I think it is more intuitive to treat isModelReady like a superset of isModelLoadedSuccessfully.
Also on this subject, change default value for isModelReady to false and only set it to true after loading succeeds.
Also perhaps we should move setIsModelReady(false) outside of the async block to avoid any possible weird race conditions when forward is called before isModelReady is set to false

mkopcins · 2024-12-15T15:37:53Z

src/StyleTransfer.ts

+    if (error) {
+      throw new Error(error);
+    }
+


This shouldn't be necessary if we keep invariant that: isModelReady is true only if error is null.

src/StyleTransfer.ts

mkopcins · 2024-12-15T15:47:20Z

ios/RnExecutorch/utils/ImageProcessor.mm

+    cv::Mat encodedData(1, [data length], CV_8UC1, (void *)data.bytes);
+    inputImage = cv::imdecode(encodedData, cv::IMREAD_COLOR);
+  }
+  if([[url scheme] isEqualToString: @"file"]){


Suggested change

if([[url scheme] isEqualToString: @"file"]){

else if([[url scheme] isEqualToString: @"file"]){

mkopcins · 2024-12-15T15:58:06Z

ios/RnExecutorch/models/StyleTransferModel.mm

-    [array addObject:number];
-  }
+  cv::Size modelImageSize = [self getModelImageSize];
+  cv::resize(input, input, modelImageSize);


This seems wrong, what if the input is smaller than modelImageSize?

never mind this, I thought initially this is an array pointer and not a cv::Mat object. Anyway since this is not performed 'inplace' due to the above reason I would consider creating another cv::Mat object output for better readability

mkopcins · 2024-12-15T16:01:33Z

ios/RnExecutorch/models/StyleTransferModel.mm

+- (cv::Mat)postprocess:(NSArray *)output {
+  cv::Size modelImageSize = [self getModelImageSize];
+  cv::Mat processedImage = [ImageProcessor arrayToMat: output width:modelImageSize.width height:modelImageSize.height];
+  cv::resize(processedImage, processedImage, originalSize);


Again, what if the originalSize is larger than modelImageSize?

mkopcins · 2024-12-15T18:33:58Z

src/StyleTransfer.ts

@@ -18,7 +18,7 @@ export const useStyleTransfer = ({
  modulePath,
 }: Props): StyleTransferModule => {
  const [error, setError] = useState<null | string>(null);
-  const [isModelLoading, setIsModelLoading] = useState(true);
+  const [isModelReady, setIsModelReady] = useState(true);


Suggested change

const [isModelReady, setIsModelReady] = useState(true);

const [isModelReady, setIsModelReady] = useState(false);

mkopcins · 2024-12-16T11:32:31Z

src/StyleTransfer.ts

@@ -30,33 +30,34 @@ export const useStyleTransfer = ({
      }

      try {
-        setIsModelLoading(true);
        await StyleTransfer.loadModule(path);


add setIsModelReady(false); here, in case modulePath prop changes

## Description This PR introduces implementation for style transfer module which is using opencv under the hood, it allows to load images from external sources, device storage and base64. ### Type of change - [ ] Bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] Documentation update (improves or adds clarity to existing documentation) ### Tested on - [x] iOS - [ ] Android ### Testing instructions  ### Screenshots  ### Related issues  ### Checklist - [x] I have performed a self-review of my code - [ ] I have commented my code, particularly in hard-to-understand areas - [x] I have updated the documentation accordingly - [x] My changes generate no new warnings ### Additional notes

NorbertKlockiewicz added 6 commits December 10, 2024 17:36

feat: provisional style transfer implementation with opencv(ios)

419d42e

performance improvements of style transfer, added ETError enum to ios…

0187160

… project

update naming so it matches RnExecutorchModule, added function to rea…

17e437a

…d image resource from base64, storage and external sources

test

9c38d14

remove computer vision example app as it's already on main

85fb088

NorbertKlockiewicz changed the title ~~Style Transfer Implementation with opencv~~ feat: style transfer with openCV(ios) Dec 12, 2024

NorbertKlockiewicz marked this pull request as ready for review December 12, 2024 10:45

NorbertKlockiewicz added 3 commits December 12, 2024 11:49

Merge branch 'main' into @norbertklockiewicz/style-transfer-implement…

5cd8e9e

…ation

NorbertKlockiewicz requested a review from mkopcins December 12, 2024 10:52

NorbertKlockiewicz added 4 commits December 12, 2024 12:37

fix: fixes to NS types and one logic bug

00a06c2

fix: add missing error throw

5963b15

feat: generated files have random names, remove unused log

fbaf2ad

chore: update ExecutorchLib binaries

5f1d008

mkopcins requested changes Dec 15, 2024

View reviewed changes

mkopcins reviewed Dec 15, 2024

View reviewed changes

fix: requested changes

6cd072f

NorbertKlockiewicz requested a review from mkopcins December 16, 2024 08:31

mkopcins reviewed Dec 16, 2024

View reviewed changes

mkopcins approved these changes Dec 16, 2024

View reviewed changes

fix: requested change

ea66b9e

NorbertKlockiewicz merged commit f556b8a into main Dec 16, 2024
2 checks passed

NorbertKlockiewicz deleted the @norbertklockiewicz/style-transfer-implementation branch December 16, 2024 12:55

mkopcins restored the @norbertklockiewicz/style-transfer-implementation branch December 16, 2024 13:22

mkopcins deleted the @norbertklockiewicz/style-transfer-implementation branch December 16, 2024 13:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: style transfer with openCV(ios) #48

feat: style transfer with openCV(ios) #48

NorbertKlockiewicz commented Dec 10, 2024 •

edited

Loading

mkopcins Dec 15, 2024

mkopcins Dec 15, 2024

mkopcins Dec 15, 2024

mkopcins Dec 15, 2024

mkopcins Dec 15, 2024

mkopcins Dec 15, 2024

mkopcins Dec 15, 2024

mkopcins Dec 16, 2024

	if([[url scheme] isEqualToString: @"file"]){
	else if([[url scheme] isEqualToString: @"file"]){

	const [isModelReady, setIsModelReady] = useState(true);
	const [isModelReady, setIsModelReady] = useState(false);

feat: style transfer with openCV(ios) #48

feat: style transfer with openCV(ios) #48

Conversation

NorbertKlockiewicz commented Dec 10, 2024 • edited Loading

Description

Type of change

Tested on

Testing instructions

Screenshots

Related issues

Checklist

Additional notes

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NorbertKlockiewicz commented Dec 10, 2024 •

edited

Loading