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

Data streaming using chunks #787

Closed
wants to merge 125 commits into from
Closed

Data streaming using chunks #787

wants to merge 125 commits into from

Conversation

ygnn123
Copy link

@ygnn123 ygnn123 commented Oct 21, 2019

Known issues in this PR:

  • There are no get frame REST API methods.

  • Crash browser tab on 4k video after several navigation events (Possible solution: change the size of the decoded frames buffer dynamically).

  • Some number of fast clicks (>10-20) on the navigation bar leads to the corresponding number of chunk request. Decoding begins after all requests have finished. Possible solution: implement the same logic as for cvat-data (process only the last request).

  • Build instruction for avc.wasm and Decoder.js as first step and dockerized build as final solution.

  • There are no access to original quality chunks and frames.

  • Need to check the creation task time for both case (images and video) and compare with current version.

  • Estimate network traffic for new and old version on the same data.

  • Add small-size preview for the navigation slider.

  • Video playback is not smooth - player freezes and skips some frames after that. Partially fixed. Need to implement a buffering on the player side to make playback more smoothly.

  • Player issue: now player is requiring frames every 1/25s. It may leads to the following case: the frame which requested later will be received by the player earlier. (If decoder works slow on the large frame resolution).

  • Decoder potentially may has performance problems and decoding speed may be not enough to play the video smoothly, especially on the high resolution videos. Solution: use downscaled images on the backend and upscale on the frontend. Implemented https://github.com/ygnn123/cvat/pull/7/files

  • Don't create decoder worker threads for each chunk, it leads to extra server requests, etc. (Checked: h264 decoder works fine).

  • Access to the original data for old tasks issue (data migration)

  • Cleanup code, fix codacy issues.

  • auto_segmentation, reid and datumaro are broken

@bsekachev
Copy link
Member

@ygnn123
Something wrong with tests on both client and server

@bsekachev
Copy link
Member

@ygnn123

Please, put minimized version of cvat-core.min.js to cvat/apps/engine/static/engine/js

cvat-core/src/frames.js Outdated Show resolved Hide resolved
cvat-core/src/frames.js Show resolved Hide resolved
cvat-core/src/session.js Show resolved Hide resolved
cvat-core/src/session.js Show resolved Hide resolved
cvat-core/src/session.js Show resolved Hide resolved
cvat-core/src/session.js Outdated Show resolved Hide resolved
cvat-core/src/session.js Show resolved Hide resolved
@bsekachev
Copy link
Member

@ygnn123

Files 3rdparty/buffer.js, 3rdparty/decoder.js, etc. look like have already been modified by us.
They must store exactly as is without any modifications from our side. Patch file is used in order to modify them automatically.

cvat-data/src/js/decode_video.js Outdated Show resolved Hide resolved
cvat-data/src/js/decode_video.js Outdated Show resolved Hide resolved
cvat-data/src/js/pttjpeg.js Outdated Show resolved Hide resolved
cvat-data/package.json Outdated Show resolved Hide resolved
cvat-data/src/js/decode_video.js Outdated Show resolved Hide resolved
cvat-data/package.json Outdated Show resolved Hide resolved
cvat-data/package.json Show resolved Hide resolved
@bsekachev
Copy link
Member

@ygnn123

You need to add cvat-data to a Dockerfile.
Now cvat-data isn't a part of a docker image, so unit tests cannot be performed because cvat-data package not found.

Dockerfile Outdated
@@ -142,6 +142,7 @@ COPY ssh ${HOME}/.ssh
COPY utils ${HOME}/utils
COPY cvat/ ${HOME}/cvat
COPY cvat-core/ ${HOME}/cvat-core
COPY cvat-core/ ${HOME}/cvat-data
Copy link
Member

Choose a reason for hiding this comment

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

Wrong source for copy

cvat-core/src/frames.js Outdated Show resolved Hide resolved
cvat-core/src/session.js Show resolved Hide resolved
cvat-core/src/session.js Show resolved Hide resolved
cvat/apps/engine/static/engine/js/cvat-core.min.js Outdated Show resolved Hide resolved
cvat-core/src/session.js Outdated Show resolved Hide resolved
throw new ArgumentError(
`Invalid mode is specified ${mode}`,
);
}

frameDataCache[taskID][frame] = new FrameData(size.width, size.height, taskID, frame);
Copy link
Member

Choose a reason for hiding this comment

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

Maybe is it better to cache these instances like before? It isn't exactly frames. They are only wrappers which don't have binary information about frames.

Copy link
Author

Choose a reason for hiding this comment

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

No!!! I realy need cache in cvat-data due to worker using.

@nmanovic
Copy link
Contributor

@azhavoro , please merge the latest develop branch. Need to have a working solution this week. Next week we need to start testing otherwise the feature will be delivered only next year and it is below our expectations.

@nmanovic nmanovic changed the title [WIP] Video streaming Video streaming Dec 24, 2019
@nmanovic nmanovic changed the title Video streaming Data streaming using chunks Dec 24, 2019
@@ -1,4 +1,5 @@
exclude_paths:
- '**/3rdparty/**'
- '**/engine/js/cvat-core.min.js'
- '**/engine/js/unzip_imgs.js'
Copy link
Contributor

Choose a reason for hiding this comment

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

What is a reason to exclude the file from scanning?

Copy link
Contributor

Choose a reason for hiding this comment

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

this is bundled and minified version of cvat-data/src/js/unzip_imgs.js which included to scanning.

from cvat.apps.engine.mime_types import mimetypes


class FrameProvider():
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a comment for future discussion. I generally like the class but prefer to change its interface:

  • QUALITY_ORIGINAL=100, QUALITY_COMPRESSED=0
  • get_preview(self)
  • get_chunk(self, n, quality=QUALITY_ORIGINAL)
  • get_frame(self, n, quality=QUALITY_ORIGINAL)
  • get_frames(self, quality=QUALITY_ORIGINAL)

The main idea behind the change is to keep the API stable if we generate compressed frames on the fly with a quality. Thus we will be able to call get_frame(self, n, 75).

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.

4 participants