From 8c67070f2dde196351246ec9f675657bcaecff9e Mon Sep 17 00:00:00 2001 From: Ian Benlolo <28714744+ianbenlolo@users.noreply.github.com> Date: Wed, 16 Oct 2024 05:01:17 -0400 Subject: [PATCH 1/5] Update README.md (#8544) --- utils/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/README.md b/utils/README.md index b873c200903..66fe535f62b 100644 --- a/utils/README.md +++ b/utils/README.md @@ -5,4 +5,4 @@ This folder contains some useful utilities for Computer Vision Annotation Tool (CVAT). To read about a certain utility please choose a link: -- [Command line interface for working with CVAT tasks](https://docs.cvat.ai/docs/manual/advanced/cli/) +- [Command line interface for working with CVAT tasks](https://docs.cvat.ai/docs/api_sdk/cli/) From c557f7090c03573bcca65d992707ed49402a16bf Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Wed, 16 Oct 2024 13:42:28 +0300 Subject: [PATCH 2/5] Updated migration (#8543) --- .../migrations/0084_honeypot_support.py | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/cvat/apps/engine/migrations/0084_honeypot_support.py b/cvat/apps/engine/migrations/0084_honeypot_support.py index 53b2be5e1e5..721d400ec38 100644 --- a/cvat/apps/engine/migrations/0084_honeypot_support.py +++ b/cvat/apps/engine/migrations/0084_honeypot_support.py @@ -1,9 +1,11 @@ # Generated by Django 4.2.15 on 2024-09-23 13:11 from typing import Collection +from collections import defaultdict import django.db.models.deletion from django.db import migrations, models +from django.db.models import Count, Q import cvat.apps.engine.models @@ -39,6 +41,28 @@ def get_segment_rel_frame_set(db_segment) -> Collection[int]: return sorted(get_rel_frame(abs_frame, db_data) for abs_frame in frame_set) +def delete_duplicate_ground_truth_jobs(apps, schema_editor): + Task = apps.get_model("engine", "Task") + Job = apps.get_model("engine", "Job") + + broken_tasks = Task.objects.annotate( + ground_truth_jobs_count=Count( + 'segment__job', filter=Q(segment__job__type='ground_truth') + ) + ).filter(ground_truth_jobs_count__gt=1) + + gt_jobs = Job.objects.filter( + segment__task__in=broken_tasks + ).filter(type='ground_truth').order_by('-updated_date').iterator(1000) + + groups = defaultdict(list) + for gt_job in gt_jobs: + assert gt_job.type == 'ground_truth' + groups[gt_job.segment.task.id].append(gt_job) + + for gt_jobs in groups.values(): + for gt_job in gt_jobs[1:]: + gt_job.delete() def init_validation_layout_in_tasks_with_gt_job(apps, schema_editor): Job = apps.get_model("engine", "Job") @@ -220,6 +244,10 @@ class Migration(migrations.Migration): ), ], ), + migrations.RunPython( + delete_duplicate_ground_truth_jobs, + reverse_code=migrations.RunPython.noop, + ), migrations.RunPython( init_validation_layout_in_tasks_with_gt_job, reverse_code=migrations.RunPython.noop, From 49ec1d175d9a9ce106158a7a7936e3c6622d3672 Mon Sep 17 00:00:00 2001 From: Maria Khrustaleva Date: Wed, 16 Oct 2024 16:26:28 +0200 Subject: [PATCH 3/5] Fix task creation with `gt_pool` validation and cloud storage data (#8539) --- ..._task_creation_with_gt_pool_and_cs_data.md | 4 + cvat/apps/engine/task.py | 28 +----- tests/python/rest_api/test_tasks.py | 95 +++++++++++++++++-- tests/python/shared/utils/helpers.py | 12 ++- utils/dataset_manifest/core.py | 15 +++ 5 files changed, 120 insertions(+), 34 deletions(-) create mode 100644 changelog.d/20241016_142620_maria_fix_task_creation_with_gt_pool_and_cs_data.md diff --git a/changelog.d/20241016_142620_maria_fix_task_creation_with_gt_pool_and_cs_data.md b/changelog.d/20241016_142620_maria_fix_task_creation_with_gt_pool_and_cs_data.md new file mode 100644 index 00000000000..8772b7f6713 --- /dev/null +++ b/changelog.d/20241016_142620_maria_fix_task_creation_with_gt_pool_and_cs_data.md @@ -0,0 +1,4 @@ +### Fixed + +- Task creation with cloud storage data and GT_POOL validation mode + () diff --git a/cvat/apps/engine/task.py b/cvat/apps/engine/task.py index 459a6ddd4b3..446cdabb082 100644 --- a/cvat/apps/engine/task.py +++ b/cvat/apps/engine/task.py @@ -1157,19 +1157,6 @@ def _update_status(msg: str) -> None: assert job_file_mapping[-1] == validation_params['frames'] job_file_mapping.pop(-1) - # Update manifest - manifest = ImageManifestManager(db_data.get_manifest_path()) - manifest.link( - sources=[extractor.get_path(image.frame) for image in images], - meta={ - k: {'related_images': related_images[k] } - for k in related_images - }, - data_dir=upload_dir, - DIM_3D=(db_task.dimension == models.DimensionType.DIM_3D), - ) - manifest.create() - db_data.update_validation_layout(models.ValidationLayout( mode=models.ValidationMode.GT_POOL, frames=list(frame_idx_map.values()), @@ -1324,24 +1311,15 @@ def _update_status(msg: str) -> None: assert image.is_placeholder image.real_frame = frame_id_map[image.real_frame] + # Update manifest + manifest.reorder([images[frame_idx_map[image.frame]].path for image in new_db_images]) + images = new_db_images db_data.size = len(images) db_data.start_frame = 0 db_data.stop_frame = 0 db_data.frame_filter = '' - # Update manifest - manifest = ImageManifestManager(db_data.get_manifest_path()) - manifest.link( - sources=[extractor.get_path(frame_idx_map[image.frame]) for image in images], - meta={ - k: {'related_images': related_images[k] } - for k in related_images - }, - data_dir=upload_dir, - DIM_3D=(db_task.dimension == models.DimensionType.DIM_3D), - ) - manifest.create() db_data.update_validation_layout(models.ValidationLayout( mode=models.ValidationMode.GT_POOL, diff --git a/tests/python/rest_api/test_tasks.py b/tests/python/rest_api/test_tasks.py index 408e74d8895..c57dec13f63 100644 --- a/tests/python/rest_api/test_tasks.py +++ b/tests/python/rest_api/test_tasks.py @@ -32,6 +32,7 @@ ClassVar, Dict, Generator, + Iterable, List, Optional, Sequence, @@ -1529,12 +1530,13 @@ def _create_task_with_cloud_data( server_files: List[str], use_cache: bool = True, sorting_method: str = "lexicographical", - spec: Optional[Dict[str, Any]] = None, data_type: str = "image", video_frame_count: int = 10, server_files_exclude: Optional[List[str]] = None, - org: Optional[str] = None, + org: str = "", filenames: Optional[List[str]] = None, + task_spec_kwargs: Optional[Dict[str, Any]] = None, + data_spec_kwargs: Optional[Dict[str, Any]] = None, ) -> Tuple[int, Any]: s3_client = s3.make_client(bucket=cloud_storage["resource"]) if data_type == "video": @@ -1551,7 +1553,9 @@ def _create_task_with_cloud_data( ) else: images = generate_image_files( - 3, **({"prefixes": ["img_"] * 3} if not filenames else {"filenames": filenames}) + 3, + sizes=[(100, 50) if i % 2 else (50, 100) for i in range(3)], + **({"prefixes": ["img_"] * 3} if not filenames else {"filenames": filenames}), ) for image in images: @@ -1598,6 +1602,7 @@ def _create_task_with_cloud_data( "name": "car", } ], + **(task_spec_kwargs or {}), } data_spec = { @@ -1608,9 +1613,8 @@ def _create_task_with_cloud_data( server_files if not use_manifest else server_files + ["test/manifest.jsonl"] ), "sorting_method": sorting_method, + **(data_spec_kwargs or {}), } - if spec is not None: - data_spec.update(spec) if server_files_exclude: data_spec["server_files_exclude"] = server_files_exclude @@ -1984,7 +1988,7 @@ def test_create_task_with_cloud_storage_and_check_retrieve_data_meta( use_cache=False, server_files=["test/video/video.avi"], org=org, - spec=data_spec, + data_spec_kwargs=data_spec, data_type="video", ) @@ -2550,6 +2554,85 @@ def test_can_create_task_with_gt_job_from_video( else: assert len(validation_frames) == validation_frames_count + @pytest.mark.with_external_services + @pytest.mark.parametrize("cloud_storage_id", [2]) + @pytest.mark.parametrize( + "validation_mode", + [ + models.ValidationMode("gt"), + models.ValidationMode("gt_pool"), + ], + ) + def test_can_create_task_with_validation_and_cloud_data( + self, + cloud_storage_id: int, + validation_mode: models.ValidationMode, + request: pytest.FixtureRequest, + admin_user: str, + cloud_storages: Iterable, + ): + cloud_storage = cloud_storages[cloud_storage_id] + server_files = [f"test/sub_0/img_{i}.jpeg" for i in range(3)] + validation_frames = ["test/sub_0/img_1.jpeg"] + + (task_id, _) = self._create_task_with_cloud_data( + request, + cloud_storage, + use_manifest=False, + server_files=server_files, + sorting_method=models.SortingMethod( + "random" + ), # only random sorting can be used with gt_pool + data_spec_kwargs={ + "validation_params": models.DataRequestValidationParams._from_openapi_data( + mode=validation_mode, + frames=validation_frames, + frame_selection_method=models.FrameSelectionMethod("manual"), + frames_per_job_count=1, + ) + }, + task_spec_kwargs={ + # in case of gt_pool: each regular job will contain 1 regular and 1 validation frames, + # (number of validation frames is not included into segment_size) + "segment_size": 1, + }, + ) + + with make_api_client(admin_user) as api_client: + # check that GT job was created + (paginated_jobs, _) = api_client.jobs_api.list(task_id=task_id, type="ground_truth") + assert 1 == len(paginated_jobs["results"]) + + (paginated_jobs, _) = api_client.jobs_api.list(task_id=task_id, type="annotation") + jobs_count = ( + len(server_files) - len(validation_frames) + if validation_mode == models.ValidationMode("gt_pool") + else len(server_files) + ) + assert jobs_count == len(paginated_jobs["results"]) + # check that the returned meta of images corresponds to the chunk data + # Note: meta is based on the order of images from database + # while chunk with CS data is based on the order of images in a manifest + for job in paginated_jobs["results"]: + (job_meta, _) = api_client.jobs_api.retrieve_data_meta(job["id"]) + (_, response) = api_client.jobs_api.retrieve_data( + job["id"], type="chunk", quality="compressed", index=0 + ) + chunk_file = io.BytesIO(response.data) + assert zipfile.is_zipfile(chunk_file) + + with zipfile.ZipFile(chunk_file, "r") as chunk_archive: + chunk_images = { + int(os.path.splitext(name)[0]): np.array( + Image.open(io.BytesIO(chunk_archive.read(name))) + ) + for name in chunk_archive.namelist() + } + chunk_images = dict(sorted(chunk_images.items(), key=lambda e: e[0])) + + for img, img_meta in zip(chunk_images.values(), job_meta.frames): + assert (img.shape[0], img.shape[1]) == (img_meta.height, img_meta.width) + class _SourceDataType(str, Enum): images = "images" diff --git a/tests/python/shared/utils/helpers.py b/tests/python/shared/utils/helpers.py index ac5948182d7..14015f4b2ad 100644 --- a/tests/python/shared/utils/helpers.py +++ b/tests/python/shared/utils/helpers.py @@ -5,7 +5,7 @@ import subprocess from contextlib import closing from io import BytesIO -from typing import Generator, List, Optional +from typing import Generator, List, Optional, Tuple import av import av.video.reformatter @@ -25,7 +25,11 @@ def generate_image_file(filename="image.png", size=(100, 50), color=(0, 0, 0)): def generate_image_files( - count, prefixes=None, *, filenames: Optional[List[str]] = None + count: int, + *, + prefixes: Optional[List[str]] = None, + filenames: Optional[List[str]] = None, + sizes: Optional[List[Tuple[int, int]]] = None, ) -> List[BytesIO]: assert not (prefixes and filenames), "prefixes cannot be used together with filenames" assert not prefixes or len(prefixes) == count @@ -35,7 +39,9 @@ def generate_image_files( for i in range(count): prefix = prefixes[i] if prefixes else "" filename = f"{prefix}{i}.jpeg" if not filenames else filenames[i] - image = generate_image_file(filename, color=(i, i, i)) + image = generate_image_file( + filename, color=(i, i, i), **({"size": sizes[i]}) if sizes else {} + ) images.append(image) return images diff --git a/utils/dataset_manifest/core.py b/utils/dataset_manifest/core.py index 8cf54c18ad9..6a7c9d92f0d 100644 --- a/utils/dataset_manifest/core.py +++ b/utils/dataset_manifest/core.py @@ -727,6 +727,21 @@ def emulate_hierarchical_structure( 'next': next_start_index, } + def reorder(self, reordered_images: List[str]) -> None: + """ + The method takes a list of image names and reorders its content based on this new list. + Due to the implementation of Honeypots, the reordered list of image names may contain duplicates. + """ + unique_images: Dict[str, Any] = {} + for _, image_details in self: + if image_details.full_name not in unique_images: + unique_images[image_details.full_name] = image_details + + try: + self.create(content=(unique_images[x] for x in reordered_images)) + except KeyError as ex: + raise InvalidManifestError(f"Previous manifest does not contain {ex} image") + class _BaseManifestValidator(ABC): def __init__(self, full_manifest_path): self._manifest = _Manifest(full_manifest_path) From ffe45fe2855c4b9d6d863148c2874941758de583 Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Thu, 17 Oct 2024 14:03:32 +0300 Subject: [PATCH 4/5] Fixed quality reports and immediate feedback on jobs with start frame and step (#8551) To address the identified issue, we likely need to store relative frame numbers in self._required_frames. Explanation: The only place where self._required_frames is used is in the method: ``` def _is_frame_required(self, frame): return self._required_frames is None or frame in self._required_frames ``` This method is called in: ``` def get_included_frames(self): return set( i for i in self.rel_range if not self._is_frame_deleted(i) and not self._is_frame_excluded(i) and self._is_frame_required(i) ) ``` Currently, the implementation tries to check whether a relative frame number exists in a set that contains absolute frame numbers, which doesn't work if start_frame or frame_step is defined when the task is created. This causes failures in both quality reports and immediate feedback, as well as possibly other functionalities that rely on get_included_frames. Another issue with the current code is a logical inconsistency: ``` abs_range = self.abs_range self._required_frames = set( self.abs_frame_id(frame) for frame in self._required_frames if frame in abs_range ) ``` Here, abs_frame_id expects a relative frame number and converts it to an absolute frame number, but we first check if the relative frame number from self._required_frames is already in the absolute range, which is incorrect. --- changelog.d/20241016_180804_sekachev.bs.md | 4 ++++ cvat/apps/dataset_manager/bindings.py | 7 ++----- cvat/apps/quality_control/quality_reports.py | 9 +++++---- 3 files changed, 11 insertions(+), 9 deletions(-) create mode 100644 changelog.d/20241016_180804_sekachev.bs.md diff --git a/changelog.d/20241016_180804_sekachev.bs.md b/changelog.d/20241016_180804_sekachev.bs.md new file mode 100644 index 00000000000..a16bb1a62f5 --- /dev/null +++ b/changelog.d/20241016_180804_sekachev.bs.md @@ -0,0 +1,4 @@ +### Fixed + +- Incorrect quality reports and immediate feedback with non default start frame or frame step + () diff --git a/cvat/apps/dataset_manager/bindings.py b/cvat/apps/dataset_manager/bindings.py index 172b54abf50..35d4b902a53 100644 --- a/cvat/apps/dataset_manager/bindings.py +++ b/cvat/apps/dataset_manager/bindings.py @@ -812,11 +812,8 @@ def _init_frame_info(self): self._excluded_frames.update(self.db_data.validation_layout.disabled_frames) if self._required_frames: - abs_range = self.abs_range - self._required_frames = set( - self.abs_frame_id(frame) for frame in self._required_frames - if frame in abs_range - ) + rel_range = self.rel_range + self._required_frames = set(frame for frame in self._required_frames if frame in rel_range) def __len__(self): segment = self._db_job.segment diff --git a/cvat/apps/quality_control/quality_reports.py b/cvat/apps/quality_control/quality_reports.py index be316ae73af..437cdb72615 100644 --- a/cvat/apps/quality_control/quality_reports.py +++ b/cvat/apps/quality_control/quality_reports.py @@ -2303,13 +2303,14 @@ def _compute_reports(self, task_id: int) -> int: if validation_layout.mode == ValidationMode.GT_POOL: task_frame_provider = TaskFrameProvider(task) active_validation_frames = set( - task_frame_provider.get_rel_frame_number(frame) - for frame, real_frame in ( + task_frame_provider.get_rel_frame_number(abs_frame) + for abs_frame, abs_real_frame in ( Image.objects.filter(data=task.data, is_placeholder=True) .values_list("frame", "real_frame") .iterator(chunk_size=10000) ) - if real_frame in active_validation_frames + if task_frame_provider.get_rel_frame_number(abs_real_frame) + in active_validation_frames ) jobs: List[Job] = [j for j in job_queryset if j.type == JobType.ANNOTATION] @@ -2317,7 +2318,7 @@ def _compute_reports(self, task_id: int) -> int: job.id: JobDataProvider( job.id, queryset=job_queryset, - included_frames=set(job.segment.frame_set) & active_validation_frames, + included_frames=active_validation_frames, ) for job in jobs } From 5be5d590407beec2747c6cac21a31b514c6ea1a0 Mon Sep 17 00:00:00 2001 From: Andrey Zhavoronkov Date: Fri, 18 Oct 2024 12:58:13 +0300 Subject: [PATCH 5/5] Workaround for av context closing issue when using AUTO thread_type (#8555) ### Motivation and context ### How has this been tested? ### Checklist - [x] I submit my changes into the `develop` branch - [ ] I have created a changelog fragment - [ ] I have updated the documentation accordingly - [ ] I have added tests to cover my changes - [ ] I have linked related issues (see [GitHub docs]( https://help.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword)) - [ ] I have increased versions of npm packages if it is necessary ([cvat-canvas](https://github.com/cvat-ai/cvat/tree/develop/cvat-canvas#versioning), [cvat-core](https://github.com/cvat-ai/cvat/tree/develop/cvat-core#versioning), [cvat-data](https://github.com/cvat-ai/cvat/tree/develop/cvat-data#versioning) and [cvat-ui](https://github.com/cvat-ai/cvat/tree/develop/cvat-ui#versioning)) ### License - [x] I submit _my code changes_ under the same [MIT License]( https://github.com/cvat-ai/cvat/blob/develop/LICENSE) that covers the project. Feel free to contact the maintainers if that's a concern. ## Summary by CodeRabbit - **Bug Fixes** - Improved video processing stability by disabling threading in video reading classes, addressing potential memory management issues. - **New Features** - Enhanced video handling capabilities with updated threading parameters for better performance during video processing. --- changelog.d/20241017_155815_andrey_fix_task_creating.md | 4 ++++ cvat/apps/engine/media_extractors.py | 8 +++++++- 2 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 changelog.d/20241017_155815_andrey_fix_task_creating.md diff --git a/changelog.d/20241017_155815_andrey_fix_task_creating.md b/changelog.d/20241017_155815_andrey_fix_task_creating.md new file mode 100644 index 00000000000..e3e8494c205 --- /dev/null +++ b/changelog.d/20241017_155815_andrey_fix_task_creating.md @@ -0,0 +1,4 @@ +### Fixed + +- av context closing issue when using AUTO thread_type + () diff --git a/cvat/apps/engine/media_extractors.py b/cvat/apps/engine/media_extractors.py index 9c1d2deca18..a64637359ff 100644 --- a/cvat/apps/engine/media_extractors.py +++ b/cvat/apps/engine/media_extractors.py @@ -552,6 +552,8 @@ def read_av_container(self, source: Union[str, io.BytesIO]) -> av.container.Inpu for stream in container.streams: context = stream.codec_context if context and context.is_open: + # Currently, context closing may get stuck on some videos for an unknown reason, + # so the thread_type == 'AUTO' setting is disabled for future investigation context.close() if container.open_files: @@ -583,7 +585,7 @@ def __init__( stop: Optional[int] = None, dimension: DimensionType = DimensionType.DIM_2D, *, - allow_threading: bool = True, + allow_threading: bool = False, ): super().__init__( source_path=source_path, @@ -635,6 +637,8 @@ def iterate_frames( if self.allow_threading: video_stream.thread_type = 'AUTO' + else: + video_stream.thread_type = 'NONE' frame_counter = itertools.count() with closing(self._decode_stream(container, video_stream)) as stream_decoder: @@ -795,6 +799,8 @@ def iterate_frames(self, *, frame_filter: Iterable[int]) -> Iterable[av.VideoFra video_stream = container.streams.video[0] if self.allow_threading: video_stream.thread_type = 'AUTO' + else: + video_stream.thread_type = 'NONE' container.seek(offset=start_decode_timestamp, stream=video_stream)