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

Prepare chunks on the fly #2007

Closed
wants to merge 26 commits into from
Closed

Prepare chunks on the fly #2007

wants to merge 26 commits into from

Conversation

Marishka17
Copy link
Contributor

@Marishka17 Marishka17 commented Aug 10, 2020

Motivation and context

Currently, when creating a task, original and compressed chunks are prepared.
This takes a significant amount of time and consumes a lot of disk space.
This PR contains a new way to create a task.
At the time of task creation minimum necessary information about the loaded data is collected.
(mapping of key frames and timestamps for video, dummy chunks for images).
Chunks are created "on the fly" when a request for needed chunk is received.
Created chunks are stored in a temporary cache of limited size.

How has this been tested?

Manually

Checklist

  • I submit my changes into the develop branch
  • I have added description of my changes into CHANGELOG file
  • I have added tests to cover my changes
  • I have increased versions of npm packages if it is necessary (cvat-canvas,
    cvat-core, cvat-data and cvat-ui)

License

  • 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 (see an example below)
# Copyright (C) 2020 Intel Corporation
#
# SPDX-License-Identifier: MIT

 * moved cache implementation
 * fixed start, stop, step for tasks that created on the fly
 * changed frame provider
@coveralls
Copy link

coveralls commented Aug 19, 2020

Pull Request Test Coverage Report for Build 7244

  • 242 of 266 (90.98%) changed or added relevant lines in 8 files are covered.
  • 185 unchanged lines in 6 files lost coverage.
  • Overall coverage increased (+0.2%) to 69.931%

Changes Missing Coverage Covered Lines Changed/Added Lines %
cvat/apps/engine/models.py 11 12 91.67%
cvat/apps/engine/media_extractors.py 7 10 70.0%
cvat/apps/engine/task.py 55 58 94.83%
cvat/apps/engine/frame_provider.py 16 20 80.0%
cvat/apps/engine/prepare.py 102 115 88.7%
Files with Coverage Reduction New Missed Lines %
src/annotations.js 5 48.63%
cvat/apps/authentication/serializers.py 6 56.52%
src/api-implementation.js 8 76.32%
src/annotations-history.js 14 30.77%
src/api.js 28 52.44%
src/session.js 124 53.61%
Totals Coverage Status
Change from base Build 7126: 0.2%
Covered Lines: 11982
Relevant Lines: 16670

💛 - Coveralls

@Marishka17 Marishka17 changed the title [WIP] Prepare chunks on the fly Prepare chunks on the fly Aug 19, 2020
cvat/apps/engine/cache.py Show resolved Hide resolved
cvat/apps/engine/cache.py Outdated Show resolved Hide resolved
cvat/apps/engine/media_extractors.py Outdated Show resolved Hide resolved
cvat/apps/engine/media_extractors.py Outdated Show resolved Hide resolved
cvat/apps/engine/migrations/0029_auto_20200814_2153.py Outdated Show resolved Hide resolved
@nmanovic
Copy link
Contributor

@Marishka17 , need to fix a couple of codacy issues.

bsekachev
bsekachev previously approved these changes Aug 21, 2020
Copy link
Member

@bsekachev bsekachev left a comment

Choose a reason for hiding this comment

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

Client part LGTM. I have only one question. Should we enable the option useCache by default if it doesn't work for all videos?

@Marishka17
Copy link
Contributor Author

@bsekachev , ok i will change default value.

@nmanovic
Copy link
Contributor

@Marishka17 , need to resolve conflicts.
@azhavoro , could you please comment or approve the PR?

cvat/apps/engine/media_extractors.py Outdated Show resolved Hide resolved
cvat/apps/engine/views.py Outdated Show resolved Hide resolved
cvat/apps/engine/prepare.py Outdated Show resolved Hide resolved
@azhavoro
Copy link
Contributor

LGTM, but need to fix a couple of minor comments and add some tests.

@chaoyifei
Copy link

HI,When does this pr merge

cvat/settings/base.py Outdated Show resolved Hide resolved
@nmanovic
Copy link
Contributor

nmanovic commented Sep 1, 2020

I have merged the PR with some minor modifications manually.

@nmanovic nmanovic closed this Sep 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants