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

Fixed bug: Error in Create project from backup for Standard 3D Annotation #4160

Merged
merged 13 commits into from
Jan 20, 2022
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Bug: canvas is busy when start playing, start resizing a shape and do not release the mouse cursor (<https://github.com/openvinotoolkit/cvat/pull/4151>)
- Fixed tus upload error over https (<https://github.com/openvinotoolkit/cvat/pull/4154>)
- Auth token key is not returned when registering without email verification (<https://github.com/openvinotoolkit/cvat/pull/4092>)

- Error in create project from backup for standard 3D annotation (<https://github.com/openvinotoolkit/cvat/pull/4160>)

### Security
- Updated ELK to 6.8.22 which uses log4j 2.17.0 (<https://github.com/openvinotoolkit/cvat/pull/4052>)
Expand Down
12 changes: 8 additions & 4 deletions cvat/apps/engine/media_extractors.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright (C) 2019-2020 Intel Corporation
# Copyright (C) 2019-2022 Intel Corporation
#
# SPDX-License-Identifier: MIT

Expand Down Expand Up @@ -136,6 +136,9 @@ def __iter__(self):
for i in range(self._start, self._stop, self._step):
yield (self.get_image(i), self.get_path(i), i)

def __contains__(self, media_file):
return media_file in self._source_path

def filter(self, callback):
source_path = list(filter(callback, self._source_path))
ImageListReader.__init__(
Expand Down Expand Up @@ -172,14 +175,14 @@ def get_image_size(self, i):
img = Image.open(self._source_path[i])
return img.width, img.height

def reconcile(self, source_files, step=1, start=0, stop=None, dimension=DimensionType.DIM_2D):
def reconcile(self, source_files, step=1, start=0, stop=None, dimension=DimensionType.DIM_2D, sorting_method=None):
# FIXME
ImageListReader.__init__(self,
source_path=source_files,
step=step,
start=start,
stop=stop,
sorting_method=self._sorting_method,
sorting_method=sorting_method if sorting_method else self._sorting_method,
)
self._dimension = dimension

Expand Down Expand Up @@ -328,13 +331,14 @@ def get_path(self, i):
else: # necessary for mime_type definition
return self._source_path[i]

def reconcile(self, source_files, step=1, start=0, stop=None, dimension=DimensionType.DIM_2D):
def reconcile(self, source_files, step=1, start=0, stop=None, dimension=DimensionType.DIM_2D, sorting_method=None):
super().reconcile(
source_files=source_files,
step=step,
start=start,
stop=stop,
dimension=dimension,
sorting_method=sorting_method
)

def extract(self):
Expand Down
58 changes: 38 additions & 20 deletions cvat/apps/engine/task.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

# Copyright (C) 2018-2021 Intel Corporation
# Copyright (C) 2018-2022 Intel Corporation
#
# SPDX-License-Identifier: MIT

Expand Down Expand Up @@ -326,22 +326,6 @@ def _create_thread(db_task, data, isBackupRestore=False, isDatasetImport=False):
db_data.start_frame = 0
data['stop_frame'] = None
db_data.frame_filter = ''
if isBackupRestore and media_type != 'video' and db_data.storage_method == models.StorageMethodChoice.CACHE:
# we should sort media_files according to the manifest content sequence
manifest = ImageManifestManager(db_data.get_manifest_path())
manifest.set_index()
sorted_media_files = []
for idx in range(len(media_files)):
properties = manifest[manifest_index(idx)]
image_name = properties.get('name', None)
image_extension = properties.get('extension', None)

full_image_path = f"{image_name}{image_extension}" if image_name and image_extension else None
if full_image_path and full_image_path in media_files:
sorted_media_files.append(full_image_path)
media_files = sorted_media_files.copy()
del sorted_media_files
data['sorting_method'] = models.SortingMethod.PREDEFINED
source_paths=[os.path.join(upload_dir, f) for f in media_files]
if manifest_file and not isBackupRestore and data['sorting_method'] in {models.SortingMethod.RANDOM, models.SortingMethod.PREDEFINED}:
raise Exception("It isn't supported to upload manifest file and use random sorting")
Expand All @@ -368,8 +352,8 @@ def _create_thread(db_task, data, isBackupRestore=False, isDatasetImport=False):
extractor.extract()

if db_data.storage == models.StorageChoice.LOCAL or \
(db_data.storage == models.StorageChoice.SHARE and \
isinstance(extractor, MEDIA_TYPES['zip']['extractor'])):
(db_data.storage == models.StorageChoice.SHARE and \
isinstance(extractor, MEDIA_TYPES['zip']['extractor'])):
validate_dimension.set_path(upload_dir)
validate_dimension.validate()

Expand All @@ -379,8 +363,15 @@ def _create_thread(db_task, data, isBackupRestore=False, isDatasetImport=False):
if validate_dimension.dimension == models.DimensionType.DIM_3D:
db_task.dimension = models.DimensionType.DIM_3D

keys_of_related_files = validate_dimension.related_files.keys()
absolute_keys_of_related_files = [os.path.join(upload_dir, f) for f in keys_of_related_files]
# When a task is created, the sorting method can be random and in this case, reinitialization will be with correct sorting
# but when a task is restored from a backup, a random sorting is changed to predefined and we need to manually sort files
# in the correct order.
source_files = absolute_keys_of_related_files if not isBackupRestore else \
[item for item in extractor.absolute_source_paths if item in absolute_keys_of_related_files]
extractor.reconcile(
source_files=[os.path.join(upload_dir, f) for f in validate_dimension.related_files.keys()],
source_files=source_files,
step=db_data.get_frame_step(),
start=db_data.start_frame,
stop=data['stop_frame'],
Expand All @@ -392,6 +383,33 @@ def _create_thread(db_task, data, isBackupRestore=False, isDatasetImport=False):
extractor.filter(lambda x: not re.search(r'(^|{0})related_images{0}'.format(os.sep), x))
related_images = detect_related_images(extractor.absolute_source_paths, upload_dir)

if isBackupRestore and not isinstance(extractor, MEDIA_TYPES['video']['extractor']) and db_data.storage_method == models.StorageMethodChoice.CACHE and \
db_data.sorting_method in {models.SortingMethod.RANDOM, models.SortingMethod.PREDEFINED} and validate_dimension.dimension != models.DimensionType.DIM_3D:
# we should sort media_files according to the manifest content sequence
# and we should do this in general after validation step for 3D data and after filtering from related_images
manifest = ImageManifestManager(db_data.get_manifest_path())
manifest.set_index()
sorted_media_files = []

for idx in range(len(extractor.absolute_source_paths)):
properties = manifest[idx]
image_name = properties.get('name', None)
image_extension = properties.get('extension', None)

full_image_path = os.path.join(upload_dir, f"{image_name}{image_extension}") if image_name and image_extension else None
if full_image_path and full_image_path in extractor:
sorted_media_files.append(full_image_path)
media_files = sorted_media_files.copy()
del sorted_media_files
data['sorting_method'] = models.SortingMethod.PREDEFINED
extractor.reconcile(
source_files=media_files,
step=db_data.get_frame_step(),
start=db_data.start_frame,
stop=data['stop_frame'],
sorting_method=data['sorting_method'],
)

db_task.mode = task_mode
db_data.compressed_chunk_type = models.DataChoice.VIDEO if task_mode == 'interpolation' and not data['use_zip_chunks'] else models.DataChoice.IMAGESET
db_data.original_chunk_type = models.DataChoice.VIDEO if task_mode == 'interpolation' else models.DataChoice.IMAGESET
Expand Down
14 changes: 12 additions & 2 deletions cvat/apps/engine/tests/test_rest_api.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright (C) 2020-2021 Intel Corporation
# Copyright (C) 2020-2022 Intel Corporation
#
# SPDX-License-Identifier: MIT

Expand Down Expand Up @@ -2553,6 +2553,16 @@ def setUpTestData(cls):
}
)

for sorting, _ in SortingMethod.choices():
cls.media_data.append(
{
"image_quality": 75,
"server_files[0]": filename,
'use_cache': True,
'sorting_method': sorting,
}
)

filename = os.path.join("videos", "test_video_1.mp4")
path = os.path.join(settings.SHARE_ROOT, filename)
os.makedirs(os.path.dirname(path))
Expand Down Expand Up @@ -2617,7 +2627,7 @@ def setUpTestData(cls):
**use_cache_data,
'sorting_method': SortingMethod.RANDOM,
},
# predefined: test_1.jpg, test_2.jpg, test_10.jpg, test_2.jpg
# predefined: test_1.jpg, test_2.jpg, test_10.jpg, test_3.jpg
{
**use_cache_data,
'sorting_method': SortingMethod.PREDEFINED,
Expand Down
21 changes: 12 additions & 9 deletions utils/dataset_manifest/core.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright (C) 2021 Intel Corporation
# Copyright (C) 2021-2022 Intel Corporation
#
# SPDX-License-Identifier: MIT

Expand Down Expand Up @@ -324,7 +324,7 @@ def partial_update(self, manifest, number):

def __getitem__(self, number):
assert 0 <= number < len(self), \
'A invalid index number: {}\nMax: {}'.format(number, len(self))
'Invalid index number: {}\nMax: {}'.format(number, len(self) - 1)
return self._index[number]

def __len__(self):
Expand Down Expand Up @@ -706,12 +706,15 @@ def _validate_first_item(_dict):
raise ValueError('Incorrect name field')
if not isinstance(_dict['extension'], str):
raise ValueError('Incorrect extension field')
# width and height are required for 2d data
# FIXME for 3d when manual preparation of the manifest will be implemented
if not isinstance(_dict['width'], int):
raise ValueError('Incorrect width field')
if not isinstance(_dict['height'], int):
raise ValueError('Incorrect height field')
# FIXME
Copy link
Contributor

Choose a reason for hiding this comment

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

@Marishka17 , the same question. What are we going to do with the comment? It looks like we disable two checks which can be important for 2D case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nmanovic, yes, but these checks are only relevant for the first element of the manifest file, and the reliability will drop a bit...

# Width and height are required for 2D data, but
# for 3D these parameters are not saved now.
# It is necessary to uncomment these restrictions when manual preparation for 3D data is implemented.

# if not isinstance(_dict['width'], int):
# raise ValueError('Incorrect width field')
# if not isinstance(_dict['height'], int):
# raise ValueError('Incorrect height field')

def is_manifest(full_manifest_path):
return _is_video_manifest(full_manifest_path) or \
Expand All @@ -723,4 +726,4 @@ def _is_video_manifest(full_manifest_path):

def _is_dataset_manifest(full_manifest_path):
validator = _DatasetManifestStructureValidator(full_manifest_path)
return validator.validate()
return validator.validate()