Add robust local optical flow (RLOF) implementations #1940

tsenst · 2018-12-09T16:44:50Z

The RLOF method is an improved pyramidal iterative Lucas-Kanade approach. This implementations contains interfaces for sparse optical flow for feature tracking and dense optical flow based on sparse-to-dense interpolation schemes.

Add performance and accuracy tests have been implementation as well as documentation with the related publications

force_builders=Custom,Linux x64 Debug,Docs,linux,Mac,Win64,Win32
CPU_BASELINE:Custom=AVX2
docker_image:Custom=fedora:28
buildworker:Custom=linux-1,linux-2,linux-4
**WIP**

…oved pyramidal iterative Lucas-Kanade approach. This implementations contains interfaces for sparse optical flow for feature tracking and dense optical flow based on sparse-to-dense interpolation schemes. Add performance and accuracy tests have been implementation as well as documentation with the related publications

alalek

Thank you for the contribution!

Below are comments from high level review.
Public interfaces look good.

More detailed review will be later (please make builds green).

alalek · 2018-12-09T17:50:09Z

modules/optflow/include/opencv2/optflow/rlofflow.hpp

+
+						  License Agreement
+			   For Open Source Computer Vision Library
+					   (3-clause BSD License)


Consider using of short form of license header:

// This file is part of OpenCV project. // It is subject to the license terms in the LICENSE file found in the top-level directory // of this distribution and at http://opencv.org/license.html.

or avoid using of tabs

alalek · 2018-12-09T17:50:38Z

modules/optflow/doc/optflow.bib

+	title = {Robust Local Optical Flow: Dense Motion Vector Field Interpolation},
+	booktitle = {Picture Coding Symposium},
+	year = {2016},
+	pages = {1--5},


Please fix indentation (replace tabs)

alalek · 2018-12-09T17:51:15Z

modules/optflow/include/opencv2/optflow/rlofflow.hpp

+    namespace optflow
+    {
+		//! @addtogroup optflow
+		//! @{


Please avoid extra indentation in "namespaces" - it is not necessary.

alalek · 2018-12-09T17:53:00Z

modules/optflow/include/opencv2/optflow_o.hpp

@@ -0,0 +1,309 @@
+/*


Do we need this file? Looks like it is a copy of existed one.

You were right. The file has been removed:

- fix optflow.bib indentation - remove optflow_o.hpp - change RLOFOpticalFlowParameter interfaces to Ptr<RLOFOpticalFlowParameter> to remove error on building. Fix warnings

fix perf and accuracy tests

…r to enable python wrapper interfaces

alalek

I pushed massive but minor updates (spaces, includes, static, etc). Please take a look.

alalek · 2018-12-14T15:22:38Z

modules/optflow/src/rlof/rlof_localflow.cpp

+
+    // Initialize point grid
+    int stepr = prevPyramids[0]->m_Image.rows / 30;
+    int stepc = prevPyramids[0]->m_Image.rows / 40;


m_Image.cols

Thanks that was a bug

alalek · 2018-12-14T15:28:01Z

modules/optflow/src/rlof/berlof_invoker.hpp

+            for( y = 0; y < winSize.height; y++ )
+            {
+                const uchar* src = (const uchar*)I.data + (y + iprevPt.y)*step + iprevPt.x*cn;
+                const short* dsrc = (const short*)derivI.data + (y + iprevPt.y)*dstep + iprevPt.x*cn2;


Please avoid using "data" and "step" calculations. Use .ptr<mat_type>(row, col) instead. Remove using of "steps" in other places too.

const uchar* src = I.ptr<uchar>(y + iprevPt.y, iprevPt.x); const short* dsrc = derivI.ptr<short>(y + iprevPt.y, iprevPt.x);

alalek · 2018-12-14T15:34:55Z

modules/optflow/src/rlof/berlof_invoker.hpp

+                            _mm_loadl_epi64((const __m128i*)(Jptr + x)), z);
+                        __m128i v01 = _mm_unpacklo_epi8(_mm_loadl_epi64((const __m128i*)(Jptr + x + cn)), z);
+                        __m128i v10 = _mm_unpacklo_epi8(_mm_loadl_epi64((const __m128i*)(Jptr + x + step)), z);
+                        __m128i v11 = _mm_unpacklo_epi8(_mm_loadl_epi64((const __m128i*)(Jptr + x + step + cn)), z);


Jptr + step => Jptr1

where:

const uchar* Jptr = J.ptr<uchar>(y + inextPt.y, inextPt.x); CV_DbgCheckLT(y + inextPt.y + 1, J.rows, ""); // or use std::min(J.rows - 1, y + inextPt.y + 1) below const uchar* Jptr1 = J.ptr<uchar>(y + inextPt.y + 1, inextPt.x);

The usage of step has been removed with the current pull

alalek · 2018-12-14T18:43:19Z

modules/optflow/src/rlof/geo_interpolation.cpp

+    return ret;
+/*
+    for (int y = 0; y < in.rows; y++) {
+        for (int x = 0; x < in.cols; x++) {


Do we still need this commented code?

This snippet will be removed

alalek · 2018-12-16T22:42:32Z

I'm sorry. I though that patch is almost complete.

Merge branch

Please don't do this again.

This is error prone. Conflicts has been resolves incorrectly in the most cases. This case is not an exception.
Performing merge commit is a time waste without proper strategy...
It is almost impossible to verify performed changes in merges. Provide clean commit-after-commit patch history. No merges!
Use git "fetch" and "rebase" instead. If you don't know that is this, then it is better to ignore external changes and just force push (git push --force ...) your local changes. I will re-apply my changes then patch will complete.

…ert2016

…ol) instead.

alalek · 2018-12-16T23:42:53Z

This is why we asking to use .ptr():

(gdb) bt
#0  0x00007ffff7b36d80 in cv::error(int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, char const*, char const*, int)@plt ()
    at /home/alalek/projects/opencv/build/opencv/lib/libopencv_optflow.so.4.0
#1  0x00007ffff7b71bcc in cv::Mat::ptr<unsigned char>(int, int) const (this=0x7fffffffacf0, i0=-5, i1=0)
    at /home/alalek/projects/opencv/dev/modules/core/include/opencv2/core/mat.inl.hpp:1033
#2  0x00007ffff7b68554 in cv::optflow::plk::radial::TrackerInvoker::operator()(cv::Range const&) const (this=0x7fffffffb210, range=...)
    at /home/alalek/projects/opencv/contrib/modules/optflow/src/rlof/plk_invoker.hpp:172
#3  0x00007ffff2948479 in parallel_for_impl(cv::Range const&, cv::ParallelLoopBody const&, double) (range=..., body=..., nstripes=-1)
    at /home/alalek/projects/opencv/dev/modules/core/src/parallel.cpp:602
#4  0x00007ffff2948263 in cv::parallel_for_(cv::Range const&, cv::ParallelLoopBody const&, double) (range=..., body=..., nstripes=-1)
    at /home/alalek/projects/opencv/dev/modules/core/src/parallel.cpp:518
#5  0x00007ffff7b6ec0a in cv::optflow::calcLocalOpticalFlowCore(cv::Ptr<cv::optflow::CImageBuffer>*, cv::Ptr<cv::optflow::CImageBuffer>*, cv::_InputArray const&, cv::_InputOutputArray const&, cv::optflow::RLOFOpticalFlowParameter const&) (prevPyramids=0x747120, currPyramids=0x747140, _prevPts=..., _nextPts=..., param=...)
    at /home/alalek/projects/opencv/contrib/modules/optflow/src/rlof/rlof_localflow.cpp:467
...
(gdb) f 2
#2  0x00007ffff7b68554 in cv::optflow::plk::radial::TrackerInvoker::operator() (this=0x7fffffffb210, range=...)
    at /home/alalek/projects/opencv/contrib/modules/optflow/src/rlof/plk_invoker.hpp:172
172	                const uchar* src = I.ptr<uchar>(y + iprevPt.y, 0) + iprevPt.x*cn;
(gdb) p iprevPt
$1 = {x = -5, y = -5}
(gdb) p y
$2 = 0
(gdb) p I.cols
$3 = 37
(gdb) p I.rows
$4 = 25

Code tries to access pixels out of the buffer:

Debug build
breakpoint on "cv::error"
disabled threading (to get stable code flow)

./bin/opencv_test_optflow --gtest_filter=SparseOpticalFlow.ReferenceAccuracy:DenseOpticalFlow_RLOF.ReferenceAccuracy \
    --test_threads=1

Perhaps "out of range" (lost) points should be filtered out.

alalek · 2018-12-16T23:59:10Z

Rebased commits without "merges" are here: https://github.com/alalek/opencv_contrib/commits/pr1940

tsenst · 2018-12-17T10:54:06Z

I am sorry for the circumstances. To not cause any new conflicts who should I proceed to integrate the bugfixes?

alalek · 2018-12-17T10:59:57Z

It would be nice if you can grab changes from https://github.com/alalek/opencv_contrib/commits/pr1940
If not then just work on this patch as usual. I will all extra minor changes before merge then patch is ready.

alalek · 2018-12-18T04:54:00Z

modules/optflow/src/rlof/rlof_invokerbase.hpp

+    int step = static_cast<int>(img.step1());
+    const cv::Point3_<uchar> * tval = img.ptr<cv::Point3_<uchar>>();
+    tval += c;
+    tval += r * step / 3;


One more manual pointer arithmetic:

-int step = static_cast<int>(img.step1()); -const cv::Point3_<uchar> * tval = img.ptr<cv::Point3_<uchar>>(); -tval += c; -tval += r * step / 3; +CV_DbgAssert(img.type() == CV_8UC3); +const cv::Point3_<uchar> * tval = img.ptr< cv::Point3_<uchar> >(r, c);

"if" check above should be updated too.

I removed this function because it is not used anymore. Instead computation is done by the HorizontalCrossSegmentation class

alalek · 2018-12-19T18:33:49Z

@tsenst Do you plan adding more commits into this PR?

tsenst · 2018-12-19T19:24:26Z

I have done some tests on Optical Flow datasets to compare the accuracy of this implementation with the origin one. The results were satisfying. So from my side I am not planning any new commits.

prevImage, currImage and derivI as well as changing the offset of the points in the invoker classes. add some static_cast to avoid warning remove 50 grid size sample from perf test. This grid size is to sparse for the epic interpolation remove notSameColor function since it is not used anymore

alalek

Well done! Thank you 👍

alalek reviewed Dec 10, 2018

View reviewed changes

senst and others added 16 commits December 10, 2018 20:50

- exchange tabs with spaces

69ef784

- fix optflow.bib indentation - remove optflow_o.hpp - change RLOFOpticalFlowParameter interfaces to Ptr<RLOFOpticalFlowParameter> to remove error on building. Fix warnings

introducing precompiler flag RLOD_SSE

9b11b49

remove header that could not be found

2b41b05

remove whitespaces

4956de9

fix perf and accuracy tests

remove x86intrin.h header

a6cc9bb

fix ios and arm by removing last sse commands

c367b8c

fix warnings for windows compilation

085e3c0

fix documentation RLOFOpticalFlowParameter

3f759af

integrate cast to remove last warnings

fb31337

* add create method and function inferfaces to RLOFOpticalFlowParamte…

d1440c0

…r to enable python wrapper interfaces

white space fixes / coding style

912c1dd

fix perf test

2b4eb0b

other changes: precomp.hpp / static

8ccab0b

use Matx44f and Vec4f instead of Mat

70f1116

normSigmas into constants

3b794f2

replace ceil() calls

5acff7b

alalek reviewed Dec 14, 2018

View reviewed changes

maximum level is set to 5 so that it is similar value used in the papers

aa3cc4c

senst and others added 6 commits December 16, 2018 23:24

implement paralellized horizontal cross segmentation as used in Geist…

725e296

…ert2016

drop dead code

c1c94a2

Avoid using "data" and "step" calculations. Use .ptr<mat_type>(row, c…

b95339c

…ol) instead.

Avoid using "data" and "step" calculations. Use .ptr<mat_type>(row, c…

873e03a

…ol) instead.

bugfix on BEPLK with ica and adapt the accuracy tests

0046f54

more 'static' functions

221b861

alalek reviewed Dec 18, 2018

View reviewed changes

alalek approved these changes Dec 30, 2018

View reviewed changes

alalek merged commit 1c9e237 into opencv:master Dec 30, 2018

sotsuka-fixstars mentioned this pull request Jan 7, 2019

Add cudastereo semi global matching (SGM) implementation fixstars/opencv_contrib#1

Merged

9 tasks

alalek mentioned this pull request Oct 27, 2020

optflow(rlof): fix uninitialized variable #2728

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add robust local optical flow (RLOF) implementations #1940

Add robust local optical flow (RLOF) implementations #1940

tsenst commented Dec 9, 2018 •

edited by alalek

Loading

alalek left a comment

alalek Dec 9, 2018

alalek Dec 9, 2018

alalek Dec 9, 2018

alalek Dec 9, 2018

tsenst Dec 11, 2018

alalek left a comment

alalek Dec 14, 2018

tsenst Dec 14, 2018

alalek Dec 14, 2018

alalek Dec 14, 2018

tsenst Dec 15, 2018

alalek Dec 14, 2018

tsenst Dec 14, 2018

alalek commented Dec 16, 2018

alalek commented Dec 16, 2018 •

edited

Loading

alalek commented Dec 16, 2018

tsenst commented Dec 17, 2018

alalek commented Dec 17, 2018

alalek Dec 18, 2018 •

edited

Loading

tsenst Dec 18, 2018

alalek commented Dec 19, 2018

tsenst commented Dec 19, 2018

alalek left a comment

Add robust local optical flow (RLOF) implementations #1940

Add robust local optical flow (RLOF) implementations #1940

Conversation

tsenst commented Dec 9, 2018 • edited by alalek Loading

alalek left a 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

alalek left a 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

Choose a reason for hiding this comment

alalek commented Dec 16, 2018

alalek commented Dec 16, 2018 • edited Loading

alalek commented Dec 16, 2018

tsenst commented Dec 17, 2018

alalek commented Dec 17, 2018

alalek Dec 18, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alalek commented Dec 19, 2018

tsenst commented Dec 19, 2018

alalek left a comment

Choose a reason for hiding this comment

tsenst commented Dec 9, 2018 •

edited by alalek

Loading

alalek commented Dec 16, 2018 •

edited

Loading

alalek Dec 18, 2018 •

edited

Loading