MINIFICPP-989 - Motion detect for a set of captured frames#627
MINIFICPP-989 - Motion detect for a set of captured frames#627nghiaxlee wants to merge 1 commit intoapache:masterfrom
Conversation
extensions/opencv/FrameIO.h
Outdated
| if (ret != stream->getSize()) { | ||
| throw std::runtime_error("ImageReadCallback failed to fully read flow file input stream"); | ||
| } | ||
| *image_mat_ = cv::imdecode(image_buf_, -1); |
There was a problem hiding this comment.
Who is responsible for the lifetime of this resource?
There was a problem hiding this comment.
It will be released outside onTrigger scope. The pointer points to a cv::Mat object in onTrigger, and the CallBack also in onTrigger.
extensions/opencv/FrameIO.h
Outdated
| namespace apache { | ||
| namespace nifi { | ||
| namespace minifi { | ||
| namespace processors { |
There was a problem hiding this comment.
As these are only callbacks used for opencv, could you add them to a dirrent namepsace? Opencv is fine imho.
extensions/opencv/MotionDetector.cpp
Outdated
| namespace minifi { | ||
| namespace processors { | ||
|
|
||
| core::Property MotionDetector::ImageEncoding( // NOLINT |
There was a problem hiding this comment.
Please avoid nolint in such cases!
There is a core::propertybuilder class, used by a lot of MiNiFi processors, check those examples please!
There was a problem hiding this comment.
I know, I changed one but not all of them, will change all in the next commit.
extensions/opencv/MotionDetector.cpp
Outdated
| std::set<core::Relationship> relationships; | ||
| relationships.insert(Success); | ||
| relationships.insert(Failure); | ||
| setSupportedRelationships(std::move(relationships)); |
There was a problem hiding this comment.
setSupportedRelationships({Success, Failure});
Is a bit less complicated way of doing the same :)
extensions/opencv/MotionDetector.cpp
Outdated
| logger_->log_info("Background is missing, update and exit."); | ||
| cv::cvtColor(frame, bg_img_, cv::COLOR_BGR2GRAY); | ||
| cv::GaussianBlur(bg_img_, bg_img_, cv::Size(21, 21), 0, 0); | ||
| // cv::equalizeHist(bg_img_, bg_img_); |
extensions/opencv/MotionDetector.cpp
Outdated
| session->write(flow_file, &write_cb); | ||
| session->write(diff_file, &write_cb2); | ||
| session->transfer(flow_file, Success); | ||
| session->transfer(diff_file, Success); |
There was a problem hiding this comment.
I'm not sure if these two should be transferred to the same relationship. Do we need the original FF anyway?
nghiaxlee
force-pushed
the
MINIFICPP-989
branch
from
d4b7b09 to
b084f2a
Compare
arpadboda
left a comment
arpadboda
left a comment
There was a problem hiding this comment.
Some minor comments, mostly looks good.
extensions/opencv/MotionDetector.cpp
Outdated
|
|
||
| if (context->getProperty(BackgroundFrame.getName(), value) && !value.empty()) { | ||
| bg_img_ = cv::imread(value, cv::IMREAD_GRAYSCALE); | ||
| double scale = 500.0 / bg_img_.size().width; |
There was a problem hiding this comment.
Is this a value good for all scenarios we can imagine or shall we make it configurable?
There was a problem hiding this comment.
IMO, this should work for all scenarios, the main point is balance between computation time and accuracy
extensions/opencv/MotionDetector.h
Outdated
| bool update_bg_; | ||
|
|
||
| // Background & Foreground seperation. | ||
| // std::vector<cv::Mat> history; |
There was a problem hiding this comment.
Are these parts of some features not ready yet or just leftovers?
There was a problem hiding this comment.
They are for "Background & Foreground extraction" features that I wanted earlier. However, after second thought, I think I will create a new processor for that later, since it can be useful itself.
nghiaxlee
force-pushed
the
MINIFICPP-989
branch
from
b084f2a to
006db31
Compare
arpadboda
left a comment
arpadboda
left a comment
There was a problem hiding this comment.
Looks good, thanks! Will merge soon.
2 participants
Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
For all changes:
Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
Has your PR been rebased against the latest commit within the target branch (typically master)?
Is your initial contribution a single, squashed commit?
For code changes:
For documentation related changes:
Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.