Bounding Box API by crstrong · Pull Request #55 · IntelLabs/vdms

crstrong · 2018-11-28T20:59:37Z

Support for the Bounding Box API for rectangles, along with tests (both in C++ and Python).

luisremis · 2018-12-03T20:10:37Z

Something off with the tests on my setup:

[----------] 1 test from FindImage
[ RUN ] FindImage.simpleFind
[Exception] ObjectEmpty at src/ImageData.cc:70
db/images/png/4c328cc66771034.png could not be read, object is empty
tests/json_queries.cc:138: Failure
Value of: "0"
Expected: json_response[0]["FindImage"]["status"].asString()
Which is: "-1"
[ FAILED ] FindImage.simpleFind (10 ms)
[----------] 1 test from FindImage (10 ms total)

[----------] 1 test from UpdateEntity
[ RUN ] UpdateEntity.simpleAddUpdate
[ OK ] UpdateEntity.simpleAddUpdate (21 ms)
[----------] 1 test from UpdateEntity (21 ms total)

[----------] 1 test from QueryHandler
[ RUN ] QueryHandler.AddAndFind
[ OK ] QueryHandler.AddAndFind (36 ms)
[----------] 1 test from QueryHandler (36 ms total)

[----------] 5 tests from BoundingBox
[ RUN ] BoundingBox.simpleAdd
[ OK ] BoundingBox.simpleAdd (15 ms)
[ RUN ] BoundingBox.addAndFind
[ OK ] BoundingBox.addAndFind (13 ms)
[ RUN ] BoundingBox.addWithImage
[ OK ] BoundingBox.addWithImage (15 ms)
[ RUN ] BoundingBox.addAndFindCoordinates
[ OK ] BoundingBox.addAndFindCoordinates (13 ms)
[ RUN ] BoundingBox.findBBWithBlob
[Exception] ObjectEmpty at src/ImageData.cc:158
Image object is empty
[Exception] ObjectEmpty at src/ImageData.cc:158
Image object is empty
[ OK ] BoundingBox.findBBWithBlob (12 ms)
[----------] 5 tests from BoundingBox (68 ms total)

crstrong · 2018-12-03T20:29:59Z

You may have to add a db/images/png folder to the test folder. I believe I needed to.

luisremis · 2018-12-03T20:33:06Z

cool, will make a note to add that on the scripts as part of the PR.

luisremis

Looks good, some notes here:
Add the necessary creation of folders to the tests scripts.
Also, both commits have modification on the functionality files. It is better to have all the functionality added in the first commit and all the testing in the second, so it is better to rework the commits.

src/ImageCommand.cc

src/BoundingBoxCommand.cc

tests/python/TestBoundingBox.py

tests/json_queries.cc

vishakha041 · 2018-12-11T23:34:09Z

Should I review this PR or wait for updates from what Luis has requested?

crstrong · 2018-12-11T23:42:56Z

I believe all of his comments have been addressed, so please do review the PR

vishakha041

This looks almost ready to go with a few simple changes.

tests/cleandbs.sh

tests/json_queries.cc

utils/src/api_schema/api_schema.json

src/BoundingBoxCommand.cc

src/QueryHandler.cc

vishakha041 · 2018-12-18T23:28:00Z

src/defines.h

+
+// Regions
+
+#define VDMS_ROI_TAG		"VD::ROI"


Minor: Is there a reason there are two colons here and 1 everywhere else?

Probably muscle memory from typing classes ;)

this was not fixed in the final version, will fix it.

vishakha041

Looks good. I see that you have updated the wiki pages too. Cool.

luisremis · 2019-01-01T02:12:11Z

src/defines.h

+#define VDMS_ROI_IMAGE_EDGE "VD:ROIIMGLINK"
+#define VDMS_ROI_SHAPE_PROP "VD:ROISHAPE"
+
+#define ROI_COORD_X "_x1"


this are inconsistent with the rest of defines and property names.

luisremis · 2019-01-01T02:12:25Z

src/defines.h

+
+// Regions
+
+#define VDMS_ROI_TAG		"VD::ROI"


this was not fixed in the final version, will fix it.

luisremis · 2019-01-01T02:14:11Z

src/BoundingBoxCommand.h

+
+        BoundingBoxCommand(const std::string &cmd_name);
+
+        std::string get_img_path(PMGDQuery &query_tx, Json::Value link);


this functions is not defined or used, will remove it.

luisremis · 2019-01-01T02:18:15Z

src/BoundingBoxCommand.h

+
+// Helper classes for handling various JSON commands.
+
+    class BoundingBoxCommand : public RSCommand


this class is not needed, the command can inherit from RSCommand directly. there is nothing specialized and shared between the bondingboxes commands.

src/defines.h

luisremis · 2019-01-01T03:47:24Z

src/defines.h

+#define VDMS_ROI_TAG		"VD::ROI"
+#define VDMS_ROI_EDGE_TAG   "VD:ROILINK"
+#define VDMS_ROI_IMAGE_EDGE "VD:ROIIMGLINK"
+#define VDMS_ROI_SHAPE_PROP "VD:ROISHAPE"


this is not used.

luisremis · 2019-01-01T03:52:18Z

src/BoundingBoxCommand.cc

+                        get_value<int>(coords, "w"),
+                        get_value<int>(coords, "h")));
+
+                    VCL::ImageFormat format = VCL::PNG;


if the image was originally jpg, we will force a change of format. we need to use the format of the original file by default.

luisremis · 2019-01-01T03:54:42Z

src/BoundingBoxCommand.cc

+                        else if (requested_format == "jpg") {
+                            format = VCL::JPG;
+                        }
+                        else {


this is checked in the json schema, no need to check here again

brain.png that is referenced in distributed/helpers.h uses image from test folder. This file is unused in this directory. Missed during review.

crstrong requested review from Ragaad, luisremis and vishakha041 November 28, 2018 20:59

luisremis suggested changes Dec 3, 2018

View reviewed changes

crstrong force-pushed the feature_boundingbox branch from 45778ba to 28c2b1b Compare December 5, 2018 00:49

vishakha041 suggested changes Dec 13, 2018

View reviewed changes

crstrong force-pushed the feature_boundingbox branch from 28c2b1b to 921cb08 Compare December 18, 2018 19:33

vishakha041 reviewed Dec 18, 2018

View reviewed changes

vishakha041 previously approved these changes Dec 18, 2018

View reviewed changes

crstrong added 2 commits December 31, 2018 13:12

Add support for Bounding Boxes

e916fe8

Add unit tests for bounding boxes

340207a

luisremis dismissed vishakha041’s stale review via 7236c68 January 1, 2019 04:13

luisremis force-pushed the feature_boundingbox branch from 921cb08 to 7236c68 Compare January 1, 2019 04:13

luisremis reviewed Jan 1, 2019

View reviewed changes

luisremis force-pushed the feature_boundingbox branch from 7236c68 to 340207a Compare January 1, 2019 04:27

luisremis approved these changes Jan 1, 2019

View reviewed changes

luisremis merged commit fa6d85b into develop Jan 1, 2019

luisremis deleted the feature_boundingbox branch January 30, 2019 18:11

Ragaad pushed a commit to Ragaad/vdms that referenced this pull request Jun 16, 2022

Delete distributed/examples folder (IntelLabs#55)

dbcea8f

brain.png that is referenced in distributed/helpers.h uses image from test folder. This file is unused in this directory. Missed during review.


		BoundingBoxCommand(const std::string &cmd_name);

		std::string get_img_path(PMGDQuery &query_tx, Json::Value link);


		// Helper classes for handling various JSON commands.

		class BoundingBoxCommand : public RSCommand

Conversation

crstrong commented Nov 28, 2018

Uh oh!

luisremis commented Dec 3, 2018

Uh oh!

crstrong commented Dec 3, 2018

Uh oh!

luisremis commented Dec 3, 2018

Uh oh!

luisremis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vishakha041 commented Dec 11, 2018

Uh oh!

crstrong commented Dec 11, 2018

Uh oh!

vishakha041 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vishakha041 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants