feat!: (object detection) RF-detr support, generic model support#826
feat!: (object detection) RF-detr support, generic model support#826
Conversation
4969b41 to
35c6056
Compare
| ObjectDetection::ObjectDetection( | ||
| const std::string &modelSource, std::vector<float> normMean, | ||
| std::vector<float> normStd, std::shared_ptr<react::CallInvoker> callInvoker) | ||
| : ObjectDetection(modelSource, callInvoker) { | ||
| if (normMean.size() == 3) { | ||
| normMean_ = std::move(normMean); | ||
| } else if (!normMean.empty()) { |
There was a problem hiding this comment.
I would add doxygen comments in here for these functions.
|
|
||
| ObjectDetection::ObjectDetection( | ||
| const std::string &modelSource, std::vector<float> normMean, | ||
| std::vector<float> normStd, std::shared_ptr<react::CallInvoker> callInvoker) |
There was a problem hiding this comment.
We can add test with passing normMean and normStd
msluszniak
left a comment
There was a problem hiding this comment.
It would be cool if someone else also check this PR.
packages/react-native-executorch/common/rnexecutorch/models/object_detection/Constants.h
Outdated
Show resolved
Hide resolved
| } | ||
| if (normStd.size() == 3) { | ||
| normStd_ = std::move(normStd); | ||
| } else if (!normStd.empty()) { |
There was a problem hiding this comment.
Don't we want to emit this warning for empty vectors as well?
NorbertKlockiewicz
left a comment
There was a problem hiding this comment.
In my opinion the computer vision example app should by default use the rf-detr model as a default (currently it's using ssdLite)
packages/react-native-executorch/common/rnexecutorch/models/object_detection/ObjectDetection.h
Show resolved
Hide resolved
| std::shared_ptr<react::CallInvoker> callInvoker); | ||
| [[nodiscard("Registered non-void function")]] std::vector<types::Detection> | ||
| generate(std::string imageSource, double detectionThreshold); | ||
| generate(std::string imageSource, double detectionThreshold, |
There was a problem hiding this comment.
Once we load the model the labelNames shouldn't change, thus shouldn't we pass them in constructor?
There was a problem hiding this comment.
Also you should update the tests to use new signature as currently it fails:
error: too few arguments to function call, expected 3, have 2
| image_processing::readImageToTensor(imageSource, getAllInputShapes()[0]); | ||
| ObjectDetection::generate(std::string imageSource, double detectionThreshold, | ||
| std::vector<std::string> labelNames) { | ||
| std::optional<cv::Scalar> mean = |
There was a problem hiding this comment.
I think we should do it once in the constructor and store std::optional<\cv::Scalar> mean/std as a class members instead of std::vector<\float>
…ject_detection/Constants.h Co-authored-by: Mateusz Sluszniak <56299341+msluszniak@users.noreply.github.com>
Description
Introduces a breaking change?
Type of change
Tested on
Testing instructions
Run demo app with RF-DETR model
Screenshots
Related issues
Checklist
Additional notes