-
Notifications
You must be signed in to change notification settings - Fork 184
Enable IC background blur feature #564
base: main
Are you sure you want to change the base?
Conversation
talk/owt/sdk/ic/backgroundblur.cc
Outdated
bool BackgroundBlur::SetParameter(const std::string& key, | ||
const std::string& value) { | ||
if (key == "model_path") { | ||
return model.LoadModel(value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a blocking call? Do you expect caller do this on a task queue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is blocking. When the function returns, the model should be ready for inference. The caller do the task queue / threading if they don't want to wait.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to clearly call this out in your API doc
|
||
cv::Mat predict(const cv::Mat& frame); | ||
void predictAsync(const cv::Mat& frame); | ||
cv::Mat waitForFinished(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the point of public this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sync/Async prediction are options provided by openvino. I just pass on these to provides potential caller with free usage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't agree. If you want to provide async API, your API should include a callback param instead of asking developer to call another API just to wait for the result.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The waitForFinished
function behave like await Promise
, I believe this is another async call design except for callbacks. Anyway, we may remove the async api since the postprocessors use a sequential processing.
talk/owt/sdk/include/cpp/owt/base/localcamerastreamparameters.h
Outdated
Show resolved
Hide resolved
talk/owt/sdk/include/cpp/owt/ic/intelligentcollaborationparameters.h
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also add new build descriptions of building with --openvino_root in README.md
Create background blur post-processor for CameraVideoCapturer.