CVAT-3D milestone6 #3234

manasars · 2021-05-25T16:26:03Z

Hi @bsekachev , @nmanovic , @zhiltsov-max

CVAT 3D Milestone 6 changes:

Added support for Dump annotations, Export Annotations and Upload annotations in PCD and Kitti formats.
The code changes are only in 4 files i.e bindings.py, registry.py and added new datatsets pointcloud.py and velodynepoint.py.

The rest of the files are from M5 base branch, was waiting for it to be merged but since comments are in progress created one for M6.

If you're unsure about any of these, don't hesitate to ask. We're here to help! -->

I submit my changes into the develop branch
We shall add the changes in CHANGELOG after M5 code is merged .
(https://github.com/opencv/cvat/blob/develop/CHANGELOG.md) file
Datumaro PR - CVAT-3D Milestone-6 PCD and Kitti formats openvinotoolkit/datumaro#245
I submit my code changes under the same MIT License that covers the project.
Feel free to contact the maintainers if that's a concern.
I have updated the license header for each file

…o cvat-3D-M5

…at-3D-M5

…o cvat-3D-M5

…at-3D-M5

manasars · 2021-07-13T11:13:27Z

@manasars , if the PR works as expected, all tests are passed, I'm OK to merge it. Could you please check that tests for 3D functionality works as expected to save time and accept the PR faster?

[Dependent] Server tests for CVAT-3D-M6 #3286

[Dependent] Cypress tests for CVAT-3D-M6. #3263

Hi @nmanovic ,

So in-order to check if only known 2D cases are failing and 3D test cases have passed, can we pull the latest cvat-3D-Milestone6 branch changes in the above mentioned PRs, can someone help us with that?

nmanovic · 2021-07-13T11:20:16Z

@manasars , I believe you always can merge both PRs into your branch. I don't think you need help from our side.

manasars · 2021-07-13T11:22:34Z

@manasars , I believe you always can merge both PRs into your branch. I don't think you need help from our side.

@nmanovic , ok sure we shall do that then and confirm once we test.

… cvat-3D-Milestone6

…t-3d-m6' into cvat-3D-Milestone6

jayraj-solanki · 2021-07-14T07:29:58Z

Hi @zhiltsov-max,
In the latest datumaro code, the exported annotation in "SLY point cloud format", the annotation files are exported as filename.pcd.pcd.json.Since filename contains .pcd twice, some unit tests are failing for point cloud (tests.test_rest_api_3D.Task3DTest)

So will this be changed in Datumaro or should we update the unit tests accordingly?

zhiltsov-max · 2021-07-14T11:23:34Z

Probably, you don't need to pass .pcd in the DatasetItem.id. We don't have such problem in the format tests. https://github.com/openvinotoolkit/datumaro/blob/develop/tests/test_sly_pointcloud_format.py

Updates related to the develop branch of Datumaro are in #3411

nmanovic · 2021-07-15T11:44:42Z

@manasars , do we have any other questions? Really need to complete the PR this week and merge.

manasars · 2021-07-15T12:43:18Z

@manasars , do we have any other questions? Really need to complete the PR this week and merge.

Hi @nmanovic , We do not have any further questions, as per Maxim's latest update we are modifying one portion of code and testing, we shall be pushing the changes in a while.

jayraj-solanki · 2021-07-15T18:27:35Z

Hi @nmanovic,

We have fixed the cypress tests and all the tests have passed.
Kindly review and merge the PR.

zhiltsov-max · 2021-07-16T08:24:18Z

cvat/apps/dataset_manager/bindings.py

+                        attributes["track_id"] = -1
+                        index += 1
+
+                dm_item = datumaro.DatasetItem(id=osp.split(frame_data.name)[-1].split('.')[0], image=None,


Suggested change

dm_item = datumaro.DatasetItem(id=osp.split(frame_data.name)[-1].split('.')[0], image=None,

dm_item = datumaro.DatasetItem(id=osp.splitext(frame_data.name)[0],

Hi @zhiltsov-max , the osp.splitext(frame_data.name)[0] returns the entire file path and not just the fileName.
We can leave it as above or change to :
id=osp.splitext(osp.split(frame_data.name)[-1])[0]

Why do you need to return only basename?

hi @zhiltsov-max,
As per the suggest changes we are getting the folder structure as shown below, "test_canvas3d/pointcloud/" is nested inside ann, pointcloud and related images folders before the actual data.

Hence we only require the basename

CVAT preserves relative image paths on export, so such behavoiur is probably expected for PCD too. But it can be a problem of the specific test, if "test_canvas3d/pointcloud/" is not supposed to be there.

@zhiltsov-max, If we make the suggested change id=osp.splitext(frame_data.name)[0] then the folder structure downloaded on click of dump annotations manually is as above which is incorrect as per SOW and not compatible with supervisely.

cvat/apps/dataset_manager/bindings.py

manasars · 2021-07-19T07:48:31Z

Hi @zhiltsov-max : I hope we are fine with all the review changes, kindly confirm.

nmanovic · 2021-07-19T09:03:29Z

@manasars , did I miss something? I cannot rotate, zoom in/out, move the primary view using mouse. Is it expected?

manasars · 2021-07-19T09:28:25Z

@manasars , did I miss something? I cannot rotate, zoom in/out, move the primary view using mouse. Is it expected?

Hi @nmanovic : We can rotate, zoom in/out etc when we select Move Controls in the left side bar similar to 2D, this was changed a part of one of the review comment changes.

nmanovic · 2021-07-19T09:37:55Z

@manasars , could you please point me on the review comment?

nmanovic · 2021-07-19T09:41:32Z

@manasars , I see a problem with projections of points. If you look at the screenshot below, you will see that side and front views are not informative at all. At the same time in the alternative annotation tools, it is easy to see boundaries of these objects on side and front views.
From my perspective, in the patch CVAT tries to project all points on these views. In reality it is a bad idea.

manasars · 2021-07-19T09:42:35Z

@manasars , could you please point me on the review comment?

Please find points 3 and 6 in the below link
#3234 (comment)

nmanovic · 2021-07-19T10:00:15Z

@manasars , could you please point me on the review comment?

Please find points 3 and 6 in the below link
#3234 (comment)

Do you mean the comment below?

If it isn't a request from our team, I'm not OK with such fix. In CVAT 2D and other annotation tools it is possible to interact with a frame using mouse by default. In the reference tool the mode is also activated by default. I like the ability to press buttons, but in reality it is much faster to interact with a scene using mouse. Do you agree?

nmanovic · 2021-07-19T10:03:00Z

@manasars , let me merge the PR, but I will wrote a separate mail with known problems which should be resolved.

nmanovic · 2021-07-19T12:58:47Z

@manasars , I summarized found issues here: #3438

It isn't all issues, but which I found during the final review today. I hope the list will not grow. Please review and comment.
To simplify next reviews and avoid the overhead on your side, I will merge the PR because all checks are passed. But I want to emphasize that it isn't an approval for milestone #6. The PR isn't completed and contains several critical issues which have to be fixed.

cdp and others added 30 commits April 9, 2021 18:14

CVAT 3D Milestone-5 changes

1bc71d9

Fixed Lint errors

592d71c

test

4739662

Fixed review comments

0b67a1a

Fixed Lint errors

7e58b65

Merge branch 'develop' of https://github.com/openvinotoolkit/cvat int…

9a7af78

…o cvat-3D-M5

M5 Box operations

48dd77e

Merge branch 'develop' of https://github.com/openvinotoolkit/cvat int…

cd8d4c5

…o cvat-3D-M5

M5- Bounding Box Operations

6735ba6

M5 Fixes

e94e39e

Merge branch 'develop' of https://github.com/openvinotoolkit/cvat int…

b6ea901

…o cvat-3D-M5

Merge branch 'develop' of https://github.com/openvinotoolkit/cvat int…

45f76ef

…o cvat-3D-M5

Cypress test added wait time

65e1233

Changed the scene color to white

8551ed2

Merge branch 'develop' of https://github.com/openvinotoolkit/cvat int…

129393c

…o cvat-3D-M5

Fixed year in header

67a46a3

CameraAction naming convention changes

d290c51

M5-Fixes

32148a8

Fixed test fail issue

81c23d5

Merge branch 'develop' of https://github.com/openvinotoolkit/cvat int…

913a162

…o cvat-3D-M5

M5- Major fixes

96f33e7

Trigger notification

01fe8dc

Minor Fixes M5

b7f78c4

Merge branch 'cvat-3D-M5' of https://github.com/manasars/cvat into cv…

d31a8d3

…at-3D-M5

M5 - Other open items

50d9a19

Merge branch 'develop' of https://github.com/openvinotoolkit/cvat int…

d3dad7b

…o cvat-3D-M5

Trigger notification

fc762f7

Error fix

642ead3

Merge branch 'cvat-3D-M5' of https://github.com/manasars/cvat into cv…

fd0c88c

…at-3D-M5

Trigger notification

593308f

cdp added 4 commits July 13, 2021 17:27

Merge remote-tracking branch 'MashaSS/cvat/ms/server_test_3D_M6' into…

4375c6d

… cvat-3D-Milestone6

Merge remote-tracking branch 'dvkruchinin/cvat/dkru/cypress-tests-cva…

d70da56

…t-3d-m6' into cvat-3D-Milestone6

Resolved existing error

f470e67

Naming convention change as per latest

0598294

jayraj-solanki added 3 commits July 15, 2021 23:08

Cypress test fixes

1712b45

Resolved merge conflicts

ac7e949

Reverted base.txt

0fef2fe

zhiltsov-max reviewed Jul 16, 2021

View reviewed changes

cvat/apps/dataset_manager/bindings.py Outdated Show resolved Hide resolved

zhiltsov-max reviewed Jul 16, 2021

View reviewed changes

cvat/apps/dataset_manager/bindings.py Outdated Show resolved Hide resolved

Review Fixes

0dab825

nmanovic approved these changes Jul 19, 2021

View reviewed changes

nmanovic merged commit c58e915 into cvat-ai:develop Jul 19, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CVAT-3D milestone6 #3234

CVAT-3D milestone6 #3234

manasars commented May 25, 2021 •

edited

manasars commented Jul 13, 2021

nmanovic commented Jul 13, 2021

manasars commented Jul 13, 2021

jayraj-solanki commented Jul 14, 2021

zhiltsov-max commented Jul 14, 2021 •

edited

nmanovic commented Jul 15, 2021

manasars commented Jul 15, 2021

jayraj-solanki commented Jul 15, 2021

zhiltsov-max Jul 16, 2021

manasars Jul 16, 2021 •

edited

zhiltsov-max Jul 16, 2021

jayraj-solanki Jul 16, 2021 •

edited

zhiltsov-max Jul 16, 2021 •

edited

jayraj-solanki Jul 16, 2021

manasars commented Jul 19, 2021

nmanovic commented Jul 19, 2021

manasars commented Jul 19, 2021

nmanovic commented Jul 19, 2021

nmanovic commented Jul 19, 2021

manasars commented Jul 19, 2021

nmanovic commented Jul 19, 2021

nmanovic commented Jul 19, 2021

nmanovic commented Jul 19, 2021

	dm_item = datumaro.DatasetItem(id=osp.split(frame_data.name)[-1].split('.')[0], image=None,
	dm_item = datumaro.DatasetItem(id=osp.splitext(frame_data.name)[0],

CVAT-3D milestone6 #3234

CVAT-3D milestone6 #3234

Conversation

manasars commented May 25, 2021 • edited

manasars commented Jul 13, 2021

nmanovic commented Jul 13, 2021

manasars commented Jul 13, 2021

jayraj-solanki commented Jul 14, 2021

zhiltsov-max commented Jul 14, 2021 • edited

nmanovic commented Jul 15, 2021

manasars commented Jul 15, 2021

jayraj-solanki commented Jul 15, 2021

zhiltsov-max Jul 16, 2021

Choose a reason for hiding this comment

manasars Jul 16, 2021 • edited

Choose a reason for hiding this comment

zhiltsov-max Jul 16, 2021

Choose a reason for hiding this comment

jayraj-solanki Jul 16, 2021 • edited

Choose a reason for hiding this comment

zhiltsov-max Jul 16, 2021 • edited

Choose a reason for hiding this comment

jayraj-solanki Jul 16, 2021

Choose a reason for hiding this comment

manasars commented Jul 19, 2021

nmanovic commented Jul 19, 2021

manasars commented Jul 19, 2021

nmanovic commented Jul 19, 2021

nmanovic commented Jul 19, 2021

manasars commented Jul 19, 2021

nmanovic commented Jul 19, 2021

nmanovic commented Jul 19, 2021

nmanovic commented Jul 19, 2021

manasars commented May 25, 2021 •

edited

zhiltsov-max commented Jul 14, 2021 •

edited

manasars Jul 16, 2021 •

edited

jayraj-solanki Jul 16, 2021 •

edited

zhiltsov-max Jul 16, 2021 •

edited