Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't export outside annotations #1729

Merged
merged 11 commits into from
Jul 7, 2020
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed
- Some objects aren't shown on canvas sometimes. For example after propagation on of objects is invisible (<https://github.com/opencv/cvat/pull/1834>)
- `outside` annotations should not be in exported images (<https://github.com/opencv/cvat/issues/1620>)

### Security
-
Expand Down
6 changes: 5 additions & 1 deletion cvat/apps/dataset_manager/bindings.py
Original file line number Diff line number Diff line change
Expand Up @@ -442,9 +442,10 @@ def match_frame_fuzzy(self, path):
return None

class CvatTaskDataExtractor(datumaro.SourceExtractor):
def __init__(self, task_data, include_images=False):
def __init__(self, task_data, include_images=False, include_outside=False):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need the flag at all? When is it useful?

super().__init__()
self._categories = self._load_categories(task_data)
self._include_outside = include_outside

dm_items = []

Expand Down Expand Up @@ -541,6 +542,9 @@ def convert_attrs(label, cvat_attrs):
anno_attr['track_id'] = shape_obj.track_id
anno_attr['keyframe'] = shape_obj.keyframe

if not self._include_outside and shape_obj.outside:
continue

anno_points = shape_obj.points
if shape_obj.type == ShapeType.POINTS:
anno = datumaro.Points(anno_points,
Expand Down
11 changes: 8 additions & 3 deletions cvat/apps/dataset_manager/formats/mot.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def _import(src_file, task_data):
points=ann.points,
occluded=ann.attributes.get('occluded') == True,
outside=False,
keyframe=False,
keyframe=True,
z_order=ann.z_order,
frame=frame_number,
attributes=[],
Expand All @@ -78,6 +78,11 @@ def _import(src_file, task_data):
for track in tracks.values():
# MOT annotations do not require frames to be ordered
track.shapes.sort(key=lambda t: t.frame)
# Set outside=True for the last shape in a track to finish the track
track.shapes[-1] = track.shapes[-1]._replace(outside=True)
# Append a shape with outside=True to finish the track
last_shape = track.shapes[-1]
if last_shape.frame + task_data.frame_step <= \
int(task_data.meta['task']['stop_frame']):
track.shapes.append(last_shape._replace(outside=True,
frame=last_shape.frame + task_data.frame_step)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that here we need to add task_data.frame_step (probably +1 is correct). You can get a shape with frame > task_data.meta['task']['stop_frame']. @azhavoro , could you please look at the line and give your opinion?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With +1 I'm not sure about how filtered out frames would be handled in UI, e.g. in "go to the next keyframe" button.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zhiltsov-max , could you please check?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've checked. We can't just save the next frame (which is +1) without breaking the logic in many places, at least on the server, so we probably should stick to +frame_step.

)
task_data.add_track(track)
19 changes: 18 additions & 1 deletion cvat/apps/dataset_manager/tests/_test_formats.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ def _setUpModule():
_setUpModule()

from cvat.apps.dataset_manager.annotation import AnnotationIR
from cvat.apps.dataset_manager.bindings import TaskData
from cvat.apps.dataset_manager.bindings import TaskData, CvatTaskDataExtractor
from cvat.apps.dataset_manager.task import TaskAnnotation
from cvat.apps.engine.models import Task


Expand Down Expand Up @@ -406,6 +407,22 @@ def load_dataset(src):
self.assertEqual(len(dataset), task["size"])
self._test_export(check, task, format_name, save_images=False)

def test_can_skip_outside(self):
images = self._generate_task_images(3)
task = self._generate_task(images)
self._generate_annotations(task)
task_ann = TaskAnnotation(task["id"])
task_ann.init_from_db()
task_data = TaskData(task_ann.ir_data, Task.objects.get(pk=task["id"]))

extractor = CvatTaskDataExtractor(task_data, include_outside=False)
dm_dataset = datumaro.components.project.Dataset.from_extractors(extractor)
self.assertEqual(4, len(dm_dataset.get("image_1").annotations))

extractor = CvatTaskDataExtractor(task_data, include_outside=True)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we always should include_outside but different formats can use or ignore such shapes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, but there is no formats with such annotation property.

Copy link
Contributor Author

@zhiltsov-max zhiltsov-max Jun 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believed it too, so we've been there already. It means that all formats must expect (read as "support") such shapes. Much better is to filter them on the caller's side.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now "caller" knows about format implementation details (a format doesn't support "outside" flag). Is it OK? What is about attributes? Tracks? Cuboids? Are you going to filter that as?

Copy link
Contributor Author

@zhiltsov-max zhiltsov-max Jun 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, only CVAT knows about outside, so I treat it as a CVAT implementation detail. Tracks and cuboids are not exported too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I see that CvatTaskDataExtractor is used as an utility function by each format. Thus the knowledge encapsulated.

dm_dataset = datumaro.components.project.Dataset.from_extractors(extractor)
self.assertEqual(5, len(dm_dataset.get("image_1").annotations))

def test_cant_make_rel_frame_id_from_unknown(self):
images = self._generate_task_images(3)
images['frame_filter'] = 'step=2'
Expand Down
25 changes: 23 additions & 2 deletions cvat/apps/engine/tests/_test_rest_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -2965,6 +2965,19 @@ def _get_initial_annotation(annotation_format):
"points": [2.0, 2.1, 77.2, 36.22],
"type": "rectangle",
"occluded": True,
"outside": False,
"attributes": [
{
"spec_id": task["labels"][0]["attributes"][1]["id"],
"value": task["labels"][0]["attributes"][1]["default_value"]
}
]
},
{
"frame": 2,
"points": [2.0, 2.1, 77.2, 36.22],
"type": "rectangle",
"occluded": True,
"outside": True,
"attributes": [
{
Expand All @@ -2976,19 +2989,27 @@ def _get_initial_annotation(annotation_format):
]
}]
rectangle_tracks_wo_attrs = [{
"frame": 1,
"frame": 0,
"label_id": task["labels"][1]["id"],
"group": 0,
"attributes": [],
"shapes": [
{
"frame": 1,
"frame": 0,
"attributes": [],
"points": [1.0, 2.1, 50.2, 36.6],
"type": "rectangle",
"occluded": False,
"outside": False
},
{
"frame": 1,
"attributes": [],
"points": [1.0, 2.1, 51, 36.6],
"type": "rectangle",
"occluded": False,
"outside": False
},
{
"frame": 2,
"attributes": [],
Expand Down
22 changes: 14 additions & 8 deletions datumaro/datumaro/components/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ class GitWrapper:
def __init__(self, config=None):
self.repo = None

if config is not None and osp.isdir(config.project_dir):
if config is not None and config.project_dir:
self.init(config.project_dir)

@staticmethod
Expand All @@ -116,8 +116,12 @@ def spawn(cls, path):
spawn = not osp.isdir(cls._git_dir(path))
repo = git.Repo.init(path=path)
if spawn:
author = git.Actor("Nobody", "nobody@example.com")
repo.index.commit('Initial commit', author=author)
repo.config_writer().set_value("user", "name", "User") \
.set_value("user", "email", "user@nowhere.com") \
.release()
# gitpython does not support init, use git directly
repo.git.init()
repo.git.commit('-m', 'Initial commit', '--allow-empty')
return repo

def init(self, path):
Expand Down Expand Up @@ -377,9 +381,10 @@ def categories(self):
def get(self, item_id, subset=None, path=None):
if path:
raise KeyError("Requested dataset item path is not found")
if subset is None:
subset = ''
return self._subsets[subset].items[item_id]
item_id = str(item_id)
subset = subset or ''
subset = self._subsets[subset]
return subset.items[item_id]

def put(self, item, item_id=None, subset=None, path=None):
if path:
Expand Down Expand Up @@ -567,7 +572,7 @@ def get(self, item_id, subset=None, path=None):
rest_path = path[1:]
return self._sources[source].get(
item_id=item_id, subset=subset, path=rest_path)
return self._subsets[subset].items[item_id]
return super().get(item_id, subset)

def put(self, item, item_id=None, subset=None, path=None):
if path is None:
Expand Down Expand Up @@ -754,9 +759,10 @@ def save(self, save_dir=None):

@staticmethod
def generate(save_dir, config=None):
config = Config(config)
config.project_dir = save_dir
project = Project(config)
project.save(save_dir)
project.config.project_dir = save_dir
return project

@staticmethod
Expand Down