Custom handling of FaceRecognizer results #402

comdiv · 2015-10-14T03:30:11Z

It's code that can close #401 or can be used as suggestion to close this ticket

berak · 2015-10-14T05:47:40Z

modules/face/include/opencv2/face.hpp

+	To implement this method u just have to do same internal cycle as in predict(InputArray src, CV_OUT int &label, CV_OUT double &confidence) but
+	not try to get "best@ result, just resend it to caller side with given collector
+	*/
+	CV_WRAP virtual void predict_collect(InputArray src, PredictCollector collector) const = 0;


since the automated scripting generator (for python java,etc) can't handle callbacks (save c++11 things), this might have to go without the CV_WRAP tag

OK will fix it

comdiv · 2015-10-14T07:00:10Z

suggestions fixed

berak · 2015-10-14T07:12:46Z

(disclaimer: i haven't tried)

the linux/android builds might need enabling c++11, similar to this

mshabunin · 2015-10-15T09:07:08Z

@comdiv, why didn't you use object as a handler? Something similar to:

struct Collector {
    virtual bool collectOne(int, double, int) = 0;
}
...
virtual void predict_collect(InputArray src, Collector & collector, const int state = 0) const = 0;

I think it is not a good idea to have different interfaces for different C++ standards. If one have library compiled with C++03 compiler than all applications using this library will be restricted to use same standard, otherwise linking errors will occur.

comdiv · 2015-10-15T10:00:14Z

I think that really better is to use Iterator patter -

PredictItertator FaceRecognizer.open(...) ;
PredictRecord PredictIterator::next();
void PredictIterator::close();
struct PredictRecord  { int label, double dist } ; // or {int* label, double* dist}

As it used to be in .NET world - so client side decide itself how to deal with that.

While may be Collector class should be more usefull while we can predefine MaxCollector, AvgCollector, FirstByTreshholdCollector, Top10Collector and so on with ready to use aggregations.

Such set of classes could be Templated and be capable work with different collect_one notations.

The only thing is not good - while i'm using (bool*) and std::funciton it's just overload of predict, expressed in other C++ standard things. When we add Collector class family we grow entropy of opencv + opencv_contrib design. Where it could be defined? In face? In shared code for contrib? In opencv? Somwhere else?

comdiv · 2015-10-15T12:23:50Z

Think I'll rewrite on collector as class design and replace existed master overload of predict (while it's more generic).
The only implemented in core type of Collector will be MostCloseCollector that will be used internally by FaceRecognizer.
On public API level only abstract Collector will be provided.
So any brainf**ks about C++11 will be fixed.

comdiv · 2015-10-15T16:34:08Z

Have totally rewrite facerecognizer internals to use with PredicCollector class,
not breaking change - older API still in game
no c++ 11 dependency or other conditional compilation

comdiv · 2015-10-15T18:45:34Z

require suggestion - how to make it Pythonable?
When I make CV_WRAP onto method - it error that PredicCollector is unknown.
How to make it well?

mshabunin · 2015-10-16T08:55:47Z

Try to change to Ptr<PredictCollector>:

CV_WRAP virtual void predict(InputArray src, Ptr<PredictCollector> collector, const int state = 0) const = 0;

Also, users will need to create instances of PredictCollector in python, for this factory method should be implemented (static class method or separate function):

Ptr<MinDistancePredictCollector> MinDistancePredictCollector::create() {}

comdiv · 2015-10-16T14:48:45Z

All green but builds was renamed so warning in github still occures

mshabunin · 2015-10-21T13:12:15Z

@comdiv, please replace tabs with 4 spaces.

comdiv · 2015-10-22T01:55:39Z

done

comdiv · 2015-10-27T08:58:36Z

Is it usefull to add some custom PredictCollector implementations?
For example i have TopNPredictCollector.

mshabunin · 2015-12-07T12:12:41Z

@comdiv , please revert all unrelated changes e.g. code formatting.

comdiv · 2015-12-07T12:58:31Z

Have fixed

mshabunin · 2015-12-07T13:21:19Z

modules/face/src/predict_collector.cpp

+*     without specific prior written permission.
+*
+*   See <http://www.opensource.org/licenses/bsd-license>
+*/


I think this license is wrong. Please replace it with text from the main license.

mshabunin · 2015-12-07T13:41:16Z

I have no other comments.

comdiv · 2015-12-08T05:11:04Z

yes, copyed license from somewhere in opencv or opencv-contrib, will place your one

predict method rewritten to use with PredictCollector interface (abstract class) now face recognizer allow to perform different scenarios of result handling

comdiv · 2015-12-08T05:15:24Z

Have applyed valid license.
Another question - i can add some PredictCollector subclasses that can be usefull - is it pleased?

mshabunin · 2015-12-08T06:59:59Z

@comdiv , yes it could be useful. I think it is better to create new PR for this. Maybe we could also implement a simple example of using new interface.

👍

comdiv · 2015-12-08T07:04:44Z

What is better?

New ISSUE + new PR
Existed ISSUE + new PR
Continue this pair of ISSUE+PR

mshabunin · 2015-12-08T07:18:43Z

Just create a new PR with the proposed functionality, new issue is not necessary.

comdiv · 2015-12-08T09:22:02Z

Have question - can std::list be used with CV_WRAP , if not what is common type and conversion for such return values.

* Updated history_cuda.yml, sanitizer_cuda.yml and test_cuda.yml pipelines: 1) All cuda_plugin pipelines will be run if cuda_plugin fiolder was changed 2) Updated sanitizer_cuda.yml with proper CudaFuncTests's filters due to memory exhausting under cuda-memcheck * Renamed Range test to be handled by sanitizer_cuda * Update timeout for CUDAPlugin_Lin to 60 minutes Made separate Azure pipeline for CUDA

comdiv mentioned this pull request Oct 14, 2015

Custom handling of FaceRecognizer results #401

Closed

berak reviewed Oct 14, 2015
View reviewed changes

comdiv mentioned this pull request Oct 14, 2015

face::FaceRecognizer now returns a map associating labels with their confidence #359

Closed

comdiv force-pushed the comdiv branch 4 times, most recently from 3a23a9d to 0616037 Compare October 15, 2015 08:32

comdiv force-pushed the comdiv branch from 0616037 to c3524a4 Compare October 15, 2015 16:30

comdiv force-pushed the comdiv branch 5 times, most recently from 2016a0b to 30bb818 Compare October 15, 2015 17:47

comdiv force-pushed the comdiv branch 2 times, most recently from 1bbca8a to 093fc58 Compare October 16, 2015 07:44

comdiv force-pushed the comdiv branch 5 times, most recently from 5e10eff to 7b129a6 Compare October 16, 2015 11:43

mshabunin self-assigned this Oct 21, 2015

comdiv force-pushed the comdiv branch from 7b129a6 to c11967c Compare October 22, 2015 01:54

comdiv force-pushed the comdiv branch from c11967c to 71f9706 Compare October 27, 2015 08:23

comdiv force-pushed the comdiv branch from 71f9706 to 788743f Compare November 5, 2015 16:30

comdiv force-pushed the comdiv branch from 788743f to be21f45 Compare November 24, 2015 16:20

comdiv force-pushed the comdiv branch from be21f45 to 88b9ef0 Compare December 7, 2015 12:58

mshabunin reviewed Dec 7, 2015
View reviewed changes

Custom handling of FaceRecognizer results

9ff3192

predict method rewritten to use with PredictCollector interface (abstract class) now face recognizer allow to perform different scenarios of result handling

comdiv force-pushed the comdiv branch from 88b9ef0 to 9ff3192 Compare December 8, 2015 05:14

opencv-pushbot merged commit 9ff3192 into opencv:master Dec 8, 2015

mshabunin added a commit that referenced this pull request Dec 8, 2015

Merge pull request #402 from comdiv:comdiv

173512b

comdiv deleted the comdiv branch December 8, 2015 08:14

This was referenced Dec 8, 2015

revised PredictCollector with set of implementations #465

Merged

Using model->predict(imageMatrix) #435

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Custom handling of FaceRecognizer results #402

Custom handling of FaceRecognizer results #402

comdiv commented Oct 14, 2015

berak Oct 14, 2015

comdiv Oct 14, 2015

comdiv commented Oct 14, 2015

berak commented Oct 14, 2015

mshabunin commented Oct 15, 2015

comdiv commented Oct 15, 2015

comdiv commented Oct 15, 2015

comdiv commented Oct 15, 2015

comdiv commented Oct 15, 2015

mshabunin commented Oct 16, 2015

comdiv commented Oct 16, 2015

mshabunin commented Oct 21, 2015

comdiv commented Oct 22, 2015

comdiv commented Oct 27, 2015

mshabunin commented Dec 7, 2015

comdiv commented Dec 7, 2015

mshabunin Dec 7, 2015

mshabunin commented Dec 7, 2015

comdiv commented Dec 8, 2015

comdiv commented Dec 8, 2015

mshabunin commented Dec 8, 2015

comdiv commented Dec 8, 2015

mshabunin commented Dec 8, 2015

comdiv commented Dec 8, 2015

Custom handling of FaceRecognizer results #402

Custom handling of FaceRecognizer results #402

Conversation

comdiv commented Oct 14, 2015

berak Oct 14, 2015

Choose a reason for hiding this comment

comdiv Oct 14, 2015

Choose a reason for hiding this comment

comdiv commented Oct 14, 2015

berak commented Oct 14, 2015

mshabunin commented Oct 15, 2015

comdiv commented Oct 15, 2015

comdiv commented Oct 15, 2015

comdiv commented Oct 15, 2015

comdiv commented Oct 15, 2015

mshabunin commented Oct 16, 2015

comdiv commented Oct 16, 2015

mshabunin commented Oct 21, 2015

comdiv commented Oct 22, 2015

comdiv commented Oct 27, 2015

mshabunin commented Dec 7, 2015

comdiv commented Dec 7, 2015

mshabunin Dec 7, 2015

Choose a reason for hiding this comment

mshabunin commented Dec 7, 2015

comdiv commented Dec 8, 2015

comdiv commented Dec 8, 2015

mshabunin commented Dec 8, 2015

comdiv commented Dec 8, 2015

mshabunin commented Dec 8, 2015

comdiv commented Dec 8, 2015